Skip to content

improvement(tests): Use pytest-xdist to run tests #11726

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pehala
Copy link
Contributor

@pehala pehala commented Aug 12, 2025

  • Shortens time for unittests from 13 to 7 minutes (CI run for this PR) with 2 workers
    • Enabled by recent unit tests improvements
  • Shortens time for integration tests from 40minutes to 14minutes with 4 workers

Testing

  • locally
  • CI

PR pre-checks (self review)

  • I added the relevant backport labels
  • I didn't leave commented-out/debugging code

Reminders

  • Add New configuration option and document them (in sdcm/sct_config.py)
  • Add unit tests to cover my changes (under unit-test/ folder)
  • Update the Readme/doc folder relevant to this change (if needed)

@pehala pehala added backport/none Backport is not required New Hydra Version PR# introduces new Hydra version labels Aug 12, 2025
@pehala pehala removed the New Hydra Version PR# introduces new Hydra version label Aug 12, 2025
@pehala
Copy link
Contributor Author

pehala commented Aug 12, 2025

Reliably fails on CI due to:
Cannot contact i-0ec1cf98cc5aaf828: java.lang.InterruptedException

Probably overload?

@fruch
Copy link
Contributor

fruch commented Aug 12, 2025

keep in mind the machine we are using for CI isn't not like you laptop.
it's only 4 cpu

and as you can see it's making thing slower... and not faster.

@pehala
Copy link
Contributor Author

pehala commented Aug 12, 2025

keep in mind the machine we are using for CI isn't not like you laptop. it's only 4 cpu

and as you can see it's making thing slower... and not faster.

Its making them not work at all (for now). Can I see what is the problem, I see it is interrupted but I would like to know why. (Is it memory? is it CPU? is it some other limit being broken?)

Also I do not think 4 workers should overload 4 CPU system

@fruch
Copy link
Contributor

fruch commented Aug 12, 2025

keep in mind the machine we are using for CI isn't not like you laptop. it's only 4 cpu

and as you can see it's making thing slower... and not faster.

Its making them not work at all (for now). Can I see what is the problem, I see it is interrupted but I would like to know why. (Is it memory? is it CPU? is it some other limit being broken?)

We don't have any machinery like those for those tests

You can connect to a running builder, or spin your own SCT runner with the same size, and experiment with it.

Also I do not think 4 workers should overload 4 CPU system

I don't know if it overload, bit having 12 cores running in 3 min, I would expect them to be slower when using 1/3 of the cores

@pehala pehala added the test-integration Enable running the integration tests suite label Aug 18, 2025
@pehala
Copy link
Contributor Author

pehala commented Aug 19, 2025

v3:

  • Lower default factor to 2 for unit tests
  • Add option to hydra to customize factor
  • Add paralelization to integration tests
    • With 4 workers the time is now 16 minutes

It seems that since integration tests provision their own machine, paralelization is not a problem. I will do last few tests with unittests and if they do not still reliably work I will open another PR just for integration tests

EDIT: It seems to work with two workers, which roughly halves the execution time (13 -> 7min), still valuable improvement in my opinion.
Integration tests are a huge improvement from 40 to 14 min

@pehala pehala marked this pull request as ready for review August 19, 2025 14:07
@pehala pehala changed the title improvement(unit_tests): Use pytest-xdist to run unit-tests improvement(tests): Use pytest-xdist to run tests Aug 19, 2025
@pehala pehala requested review from fruch and a team August 19, 2025 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/none Backport is not required test-integration Enable running the integration tests suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants