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

Run tests in a parallel matrix #1031

Closed
wants to merge 14 commits into from
Closed

Run tests in a parallel matrix #1031

wants to merge 14 commits into from

Conversation

bonjourmauko
Copy link
Member

@bonjourmauko bonjourmauko commented Aug 20, 2021

Hello!

(Working) prototype for solving #1025 and #1035, poke #1027, please chime in! 😄

⚠ This is not a proposal but a demonstration of the different tools, so we can discuss and choose what make more sense in terms both of performance and readability to the community.

Trying it out

$ make test.matrix
...

real	2m13.152s
user	10m12.086s
sys	1m48.311s

New features

  • Run openfisca test tests in parallel with make, nox, and invoke
  • Run tests against Python 3.7+
  • Run tests against NumPy 1.18+

Notes

  • We don't have control over the way subprocesses are launched and run, so reporting could be a bit confusing
  • Performance gains are relative to the number of cores in your machine
  • OpenFisca is not threadsafe, so each subprocess will run a single thread
  • If one day we solve concurrency problems, ... that could go faster.

TODO

  • Run each test suite with invoke
  • Run the matrix of tests with nox
  • Parallelize with make
  • Parallelize with pytest-parallel
  • Parallelize with pytest-split
  • Run .py tests in parallel
  • Run .yaml tests in parallel
  • Provide decent reporting
  • What to do in CI Switch from CircleCI to GitHub Actions #1030
  • Benchmark

@bonjourmauko bonjourmauko added the kind:ci Continuous ops, integration & deployment label Aug 20, 2021
@bonjourmauko bonjourmauko requested a review from a team August 20, 2021 06:23
@MattiSG
Copy link
Member

MattiSG commented Aug 24, 2021

Thanks @maukoquiroga! The promise of such a speed improvement is exciting, and I would be very appreciative of a refactor / overhaul of the current test runner 😃

However, I must say that the changeset is pretty daunting, and seems to have impacts way beyond the test runner, with some core functions having their visibility changed… Could we maybe convene of a video discussion with other interested @openfisca/international-contrib contributors, in order to understand the goals, side effects and side benefits? 🙂

In terms of pure performance and parallelisation, it might also be that #1030 allows us to use https://github.com/nektos/act as suggested by @benoit-cty, and we could have local parallelisation with the exact same setup as CI, which I would be very excited about 😃

@bonjourmauko
Copy link
Member Author

bonjourmauko commented Aug 24, 2021

However, I must say that the changeset is pretty daunting, and seems to have impacts way beyond the test runner, with some core functions having their visibility changed…

Some of them were absolutely required, in which case I've tried to avoid a breaking change by exposing changes through __init__.py files, some maybe just because I just was rushing to make it work (hence the draft status).

Indeed a partial refactoring of the test runner was imposed in order to make this work, which could definitely go in a PR on its own, with proper deprecations, and agreed upon breaking changes.

In terms of pure performance and parallelisation, it might also be that #1030 allows us to use https://github.com/nektos/act as suggested by @benoit-cty, and we could have local parallelisation with the exact same setup as CI, which I would be very excited about 😃

❤️

Could we maybe convene of a video discussion with other interested @openfisca/international-contrib contributors, in order to understand the goals, side effects and side benefits? 🙂

Definitely.

@bonjourmauko bonjourmauko added the kind:discovery Issue requires discovery: value, ux and tech label Aug 24, 2021
@bonjourmauko bonjourmauko changed the title Run tests in parallel Run tests in a parallel matrix Sep 2, 2021
@bonjourmauko bonjourmauko mentioned this pull request Sep 2, 2021
@bonjourmauko bonjourmauko added kind:roadmap A group of issues, constituting a delivery roadmap and removed kind:discovery Issue requires discovery: value, ux and tech labels Sep 29, 2021
@bonjourmauko bonjourmauko added this to the Improved testing milestone Sep 29, 2021
@bonjourmauko
Copy link
Member Author

Closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:ci Continuous ops, integration & deployment kind:roadmap A group of issues, constituting a delivery roadmap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants