Skip to content

Conversation

@alexGNX
Copy link
Contributor

@alexGNX alexGNX commented Mar 4, 2025

This is a fix about performance issues we encountered running testsuites with many User Defined Executors (UDE). Issue 826 was opened regarding this problem.

UDEs

Previous behavior

  • Each time a test step uses an UDE, every UDE YAML definition files are parsed and interpolated 2 times, due to multiple calls to:
func (v *Venom) registerUserExecutors
  • All UDEs are loaded at the moment an UDE is executed. This causes confusing behaviors: e. g. if you have 2 UDEs in the lib folder (let's call them createUser and createProduct), but createUser.yaml is not valid YAML. When running a test suite that only uses the createProduct UDE, you get the following error message:
$ venom run mytestsuite.yml
executor "createProduct" is not implemented

even though createProduct is valid. This occurs because the parsing of createUserfailed (even if it is not used in the test suite).

  • No UDE duplication check, i. e. you have 2 UDEs with the same name in 2 separate files you get no error.
  • if you set the libDirconfiguration option to the same value as the default one (lib/), each file of the directory will be loaded 2 times at each call of registerUserExecutors()
  • There are nearly no logs regarding UDEs when they fail, which make debugging them extremely tedious. They are not included in the tests reports either.

New behaviour

  • All UDEs are loaded during the parsing step (pre-run). During this step we check that the keys executor and steps are present, we load the raw file in memory and we perform a duplication check:
$ venom run mytestsuite.yml

unable to register user executors: unable to register user executor "createUser" from file "lib/createUser.yml": executor "createUser" already exists (from file "lib/createUser copy.yml")
  • the interpolation and parsing of the UDE is performed only when the UDE is called
  • when an UDE fails, we output the interpolated file to make debugging easy, e. g.:
 venom run headers.yml 
 • test maps behavior (headers.yml)
        • headers-UDE FAIL
                • headers
                  Testcase "headers", step #0-0: 1 error(s) decoding:

* 'Headers' expected a map, got 'string' (headers.yml:10)
                  Testcase "headers UDE", step #0-0: executor "headers" failed - raw interpolated:
executor: headers
input:
  headers: {}
steps:
- type: http
  method: GET
  url: https://httpbin.org/headers
  headers: "{{ .input.header }}"
  assertions:
    - result.statuscode ShouldEqual 200
    - result.bodyjson.headers.Host ShouldEqual "httpbin.org"
    - result.bodyjson.headers.Foo ShouldEqual "bar"
  vars:
    hdrs:
      from: result.bodyjson.headers
output:
  systemout: "{{.hdrs}}" (headers.yml:22)
Writing file out/test_results_headers.xml
final status: FAIL

Improvement measurement

I tested this fix on a collection of test suites totalling around 700 test cases and 120 UDEs, with some of them layered (UDE calling another UDE). Runtime has been divided by a factor of 3 (from around 35min down to around 12min)

Additional performance enhancement

  • Optimize the way the exec executor reads stdout and stderr. For exemple when loading a file into a variable with exec:
- name: getFile
  steps:
  - type: exec
    script: cat dat/bigfile.json
    vars:
      content:
        from: result.systemout

With 100k lines in the file, this test step used to take more than a minute. This lowers it down to only a couple ms.

  • use maps.Clone() to clone a map

Copy link

@BenHaramboure BenHaramboure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, test cases using user-defined executors execution time decreased by about 65% (mostly due to parsing improvements)

@ovh-cds
Copy link
Collaborator

ovh-cds commented Apr 30, 2025

CDS Report build-venom-a#253.0 ✘

  • Build
    • Build ✔
    • Unit Tests ✔
  • Tests
    • Acceptance Tests ✘

@ovh-cds
Copy link
Collaborator

ovh-cds commented Apr 30, 2025

CDS Report build-venom-a#253.1 ✘

  • Build
    • Build ✔
    • Unit Tests ✔
  • Tests
    • Acceptance Tests ✘

Copy link
Member

@yesnault yesnault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @alexGNX Thank you for this pr.

Can you check the user_executor_many.yml test please, it's currently failing:

❯ venom run -vv user_executor_many.yml
          [trac] writing venom.0.log
 • Many user executor (user_executor_many.yml)
        • info
                • exec PASS
                  [info] value foo vars foo_from_vars
                  [trac] writing user_executor_many.info.testcase.1.step.1.0.dump.json
        • mytc
                • userExecutorA FAIL
                  Testcase "userExecutorB", step #2-0: Assertion "result.systemout ShouldEqual input.foo foo_from_vars_on_b; fooFromEnv foo_from_vars" failed. expected: input.foo foo_from_vars_on_b; fooFromEnv foo_from_vars  got: input.foo defaultValueA_on_b; fooFromEnv foo_from_vars (user_executor_many.yml:0)
                  Testcase "userExecutorA", step #2-0: executor "userExecutorB" failed (user_executor_many.yml:0)
                  Testcase "mytc", step #1-0: executor "userExecutorA" failed (user_executor_many.yml:0)
final status: FAIL

Signed-off-by: alexGNX <alexandre.gagneux12@gmail.com>
@alexGNX alexGNX force-pushed the UDE-optimization branch from 34cb5cc to 5ccf633 Compare May 5, 2025 09:08
@alexGNX
Copy link
Contributor Author

alexGNX commented May 5, 2025

Hi @alexGNX Thank you for this pr.

Can you check the user_executor_many.yml test please, it's currently failing:

❯ venom run -vv user_executor_many.yml
          [trac] writing venom.0.log
 • Many user executor (user_executor_many.yml)
        • info
                • exec PASS
                  [info] value foo vars foo_from_vars
                  [trac] writing user_executor_many.info.testcase.1.step.1.0.dump.json
        • mytc
                • userExecutorA FAIL
                  Testcase "userExecutorB", step #2-0: Assertion "result.systemout ShouldEqual input.foo foo_from_vars_on_b; fooFromEnv foo_from_vars" failed. expected: input.foo foo_from_vars_on_b; fooFromEnv foo_from_vars  got: input.foo defaultValueA_on_b; fooFromEnv foo_from_vars (user_executor_many.yml:0)
                  Testcase "userExecutorA", step #2-0: executor "userExecutorB" failed (user_executor_many.yml:0)
                  Testcase "mytc", step #1-0: executor "userExecutorA" failed (user_executor_many.yml:0)
final status: FAIL

Hi @yesnault, thanks for the review. It is fixed.

Copy link
Member

@yesnault yesnault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yesnault yesnault merged commit c3aabf9 into ovh:master May 5, 2025
2 checks passed
peczenyj pushed a commit to peczenyj/venom that referenced this pull request May 7, 2025
Signed-off-by: alexGNX <alexandre.gagneux12@gmail.com>
Signed-off-by: Tiago Peczenyj <tpeczenyj@weborama.com>
ceriath pushed a commit to ceriath/venom that referenced this pull request Jul 11, 2025
Signed-off-by: alexGNX <alexandre.gagneux12@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants