Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify tests #353

Closed
nsheff opened this issue Mar 23, 2023 · 5 comments
Closed

Simplify tests #353

nsheff opened this issue Mar 23, 2023 · 5 comments

Comments

@nsheff
Copy link
Contributor

nsheff commented Mar 23, 2023

Right now running pytest on the dev branch takes 90 sec on my local computer

I'm open to other opinions, but in my opinion, if the tests take more than 15 seconds or so, then I don't run them as often and they lose some utility.

Given that looper doesn't do that much complicated stuff I'm wondering if we can pare down the tests to complete more quickly.

@nsheff nsheff added this to the 1.5.0 milestone Apr 24, 2023
@nsheff nsheff added the tests label Apr 24, 2023
@donaldcampbelljr
Copy link
Contributor

The slowest tests exist under smoketests. Typically, a single test runs a subprocess of looper via subprocess.Popen, i.e. subprocess.Popen(looper run config), captures any stderr/stdout and asserts there is no error. Unfortunately, many of these simple tests take more than 1 second to complete:

================================================== slowest durations ===================================================
2.20s call     tests/smoketests/test_run.py::TestLooperRunPreSubmissionHooks::test_looper_other_plugins[looper.write_sample_yaml_cwl-cwl.yaml]
1.99s call     tests/smoketests/test_run.py::TestLooperConfig::test_init_config_file[run]
1.81s call     tests/smoketests/test_run.py::TestLooperBothRuns::test_cmd_extra_cli[arg0-run]
1.63s call     tests/smoketests/test_run.py::TestLooperRunPreSubmissionHooks::test_looper_other_plugins[looper.write_submission_yaml-submission.yaml]
1.57s call     tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_multi_pipeline
1.56s call     tests/smoketests/test_run.py::TestLooperRunPreSubmissionHooks::test_looper_command_templates_hooks[touch {looper.output_dir}/submission/{sample.sample_name}_test.txt; {%raw%}echo {}{%endraw%}]
1.53s call     tests/smoketests/test_run.py::TestLooperCompute::test_cli_yaml_settings_passes_settings[run]
1.53s call     tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_override_sample[ --sjhsjd 212]
1.53s call     tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_run_basic
1.52s call     tests/smoketests/test_run.py::TestLooperBothRuns::test_cmd_extra_cli[arg1-run]
1.51s call     tests/smoketests/test_run.py::TestLooperCompute::test_nonexistent_yaml_settings_disregarded[run]
1.51s call     tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_sample[ --sjhsjd 212]
1.51s call     tests/smoketests/test_run.py::TestLooperRunPreSubmissionHooks::test_looper_other_plugins[looper.write_sample_yaml_prj-prj.yaml]
1.51s call     tests/smoketests/test_run.py::TestLooperRunSubmissionScript::test_looper_lumping
1.49s call     tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_sample[ --string]
1.48s call     tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_sample[string]
1.47s call     tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_override_sample[7867#$@#$cc@@]
1.46s call     tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_override_sample[ --string]
1.46s call     tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_sample_attr_missing
  1. We could remove some of these tests (e.g. do we need both test_cmd_extra_sample conditions?).
  2. We could also consider using pytest-xdist which allows pytest to distribute and run the tests in parallel over multiple CPUs.

On my machine this drops the execution time from 114 secs to 24 secs.

@donaldcampbelljr
Copy link
Contributor

Discussed:

  • see markmeld pytesting where main(test_args) is used to test CLI functionality instead of subprocess.Popen(looper run config) which should achieve the same results but also cut down on overall executiong time.

@donaldcampbelljr
Copy link
Contributor

Some tests were changed to call main directly instead of using subprocess.Popen.
This change allows for decreasing test time from 78 secs to 43 secs.
Some tests still need to use the subprocess.Popen because they must assert that certain values exist in stdout. The current implementation appears to be best practice and other methods to retrieve and decode stdout proved difficult to implement.

Therefore, I am marking this as likely solved with the PR #386.

@donaldcampbelljr
Copy link
Contributor

Re-opening for now. Currently doing another pass to remove the remaining subprocess calls and instead use a returned dict that contains important messages from the looper run. See commit a29b7b8 as an example.

@donaldcampbelljr
Copy link
Contributor

Closed via #400

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants