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

Reduce execution time of tests #429

Merged
merged 83 commits into from
Oct 30, 2023

Conversation

AnesBenmerzoug
Copy link
Collaborator

@AnesBenmerzoug AnesBenmerzoug commented Sep 13, 2023

Description

This PR closes #312 #291

Changes

  • Separate tests and notebook tests dispatch workflows.
  • Add CI step to upload coverage report to codecov.
  • Use --failed-first pytest flag to start testing from the last failed tests.
  • Generate coverage report as xml file.
  • Increase number of slowest test durations to show to 30.
  • Use pytest-split to automatically split the tests in 4 groups based on their measured duration.
  • Commit .test_durations file for use in CI.
  • Use pytest-cases to separate the test case definition from the tests in the influences tests.
  • Refactor influence tests to reduce their runtime.
  • Define new pytest marker pytest.mark.slow to mark slow tests and skip them by default.
  • Define new custom pytest flag --slow-tests to enable running slow tests.
  • Combine tox test environments into tests environment.
  • Remove unused report tox environment.
  • Update contributing docs.
  • Pin mypy to version 1.5.1.
  • Fix type hints.
  • Cache mypy cache directory on CI.
  • Add pytest-sugar as a dev dependency.
  • Define pytest cache class and fixture that uses cloudpickle to accelerate heavy fixtures.
  • Update and fix some of the notebooks.
  • Use nbmake to test notebooks instead of using custom script.
  • Fix links to documentation in README.

Checklist

  • Wrote Unit tests (if necessary)
  • Updated Documentation (if necessary)
  • Updated Changelog
  • If notebooks were added/changed, added boilerplate cells are tagged with "tags": ["hide"] or "tags": ["hide-input"]

@AnesBenmerzoug AnesBenmerzoug self-assigned this Sep 13, 2023
This was generating too many warnings
@codecov
Copy link

codecov bot commented Oct 27, 2023

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

@mdbenito mdbenito linked an issue Oct 27, 2023 that may be closed by this pull request
2 tasks
mdbenito
mdbenito previously approved these changes Oct 27, 2023
Copy link
Collaborator

@mdbenito mdbenito left a comment

Choose a reason for hiding this comment

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

This is awesome!

I'd just wish that pytest-split could replace xdist entirely, but there are two issues:

  • Crucially: Is pytest-split supported by pycharm?
  • We would need some automated way of starting the maximum number of split jobs possible given the amount of CPUs. Or maybe it'd be enough to just have a config variable (env?) setting this quantity.

@mdbenito mdbenito self-requested a review October 27, 2023 14:52
@AnesBenmerzoug
Copy link
Collaborator Author

@mdbenito

  • What do you mean by "supported by pycharm"?
  • Do you mean starting multiple groups in parallel? pytest-split doesn't support that. The best we can do is to either increase or decrease the number of groups.

@mdbenito
Copy link
Collaborator

  • What do you mean by "supported by pycharm"?

I mean, if I run tox as a run configuration, will I get IDE support for the tests, in particular with links to the right files (as opposed to useless copies in the tox environment)

  • Do you mean starting multiple groups in parallel? pytest-split doesn't support that. The best we can do is to either increase or decrease the number of groups.

I mean running the n commands in one go, and computing the number n automatically.

But never mind. xdist manages to fully use the machine and split works wonders on CI, so let's leave it like that.

@mdbenito
Copy link
Collaborator

@AnesBenmerzoug One more thing: I think it would make sense to deactivate coverage reports in local tests. It's pointless, generates tons of temp files and surely slows things down.

@mdbenito
Copy link
Collaborator

mdbenito commented Oct 27, 2023

@AnesBenmerzoug See #454 in case you want to look into it for this PR, otherwise we can merge this as-is.

@AnesBenmerzoug
Copy link
Collaborator Author

@mdbenito

  • I will document the custom cache fixture as well as when to rerun pytest with --store-durations.
  • Not sure about the PyCharm supports. I will test it and tell if it's possible to do what you want.
  • There is no way currently to determine the number of splits automatically and to run them all. I think it's fine as it is for now.
  • Regarding issue LiSSA tests timeout #454, it doesn't happen locally for me or on CI so I don't how to help with that. Perhaps @schroedk can help.

Copy link
Collaborator

@mdbenito mdbenito left a comment

Choose a reason for hiding this comment

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

Thanks for the added docs. I think this is great. I think we can merge, and then release a patch version soon

@mdbenito
Copy link
Collaborator

mdbenito commented Oct 29, 2023

@AnesBenmerzoug There seems to be some problem with "Run base tests"?

@mdbenito
Copy link
Collaborator

mdbenito commented Oct 29, 2023

  • Not sure about the PyCharm supports. I will test it and tell if it's possible to do what you want.
  • There is no way currently to determine the number of splits automatically and to run them all. I think it's fine as it is for now.

I don't think this is necessary if the situation is as I described in this comment, namely:

  • xdist is used locally
  • split is used on CI only (or locally if manually started)

In this case, we get max core usage locally, and IDE built-in support to launch and track the parallel tests.

@AnesBenmerzoug
Copy link
Collaborator Author

@mdbenito Run base tests shouldn't exist anymore. It was replaced by Run Tests - Python <version> - Group <group> in this PR.

@AnesBenmerzoug AnesBenmerzoug merged commit 25fc8fc into develop Oct 30, 2023
20 checks passed
@mdbenito mdbenito deleted the fix/reduce-execution-time-of-tests branch October 30, 2023 08:38
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.

Reduce execution time of tests Use codecov
2 participants