Try out parallel testing [full build] #92
Conversation
|
Of course, we can only get the best estimate using a full build, but if the tests all work correctly and pass in this parallel context, it should be a sign that there shouldn't be any (many?) problems. |
|
Based on https://github.com/pyodide/pyodide-recipes/actions/runs/15079325254/job/42394053702?pr=92, this seems to work! 409 tests executed in 1 minute and 51 seconds, while in the logs for an older job, I see that 380 tests ran in 3 minutes. The time savings will show up better and will be more useful for full builds. |
|
Previously we had xdist on but our ci was pretty flakey and @rth turned it off because he thought it was oversubscribed. Because the browser uses several threads. Maybe start with xdist on node? |
|
Oh, I didn't know! I'll see if I can restrict this to just Node through a global-level marker and let the rest of the tests run in serial, but I don't immediately see it in the matrix. I thought that we were testing only with Chrome at first, but that doesn't seem to be the case. I'll check. |
Package Build ResultsTotal packages built: 292 Package Build Times (click to expand)
Longest build: opencv-python (37m 12s) |
|
Okay, I notice that we aren't running tests on Node.js in CI at the moment, it's just that the Thanks! |
|
I'm fine with merging it and seeing what happens. Maybe don't set the concurrency very high? We should also add node tests ideally. |
Great! Yes, since this is on GitHub Actions runners, and we've set it to |
Yes, and also Safari and Firefox, as we just need to download the artifacts in a matrix of jobs. I can take it up in a follow-up. |
|
Let's wait for #91 before we go ahead with this; I'll rebase after that is done. |
Just worth double checking that this is creating the number of jobs you expect. "auto" may not work properly in some virtualized environnement including CI pytest-dev/pytest-xdist#1103. as they are not handling some of the edge cases that say joblib.cpu_count includes... |
|
Thanks. Yeah, I think it was flaky before, but it was a long time ago, so we can try it again. We can revert it anytime if it doesn't work. |
Thanks for that link, @rth. I tried to find the CPU specifications for the Linux runners, but I don't see any, unfortunately. If these are Intel runners, then we need to check for hyper-threading, and the CPU affinity might be a better value to use. If they are AMD CPUs, we have SMT which isn't well defined either as we know nothing about the make/model/architecture. It looks like there are now 4 cores across all runners, bumped up from the previous 2 (except M-series macOS runners, which have 3): https://docs.github.com/en/actions/using-github-hosted-runners/using-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories. The page doesn't say anything about the number of physical/logical/virtual cores. In the meantime, we can change to |
agriyakhetarpal
changed the title
|
I've triggered a full build and full test suite run, let's see if we have any issues. |
|
Thanks! The failing tests are related to cython so I think they are okay. The output looks a little bit noisy than before (two lines per test?). It would be nice to fix that, but it is not a big problem, otherwise looks good to me. |
|
Yes, I'm not really sure why that is happening. I think we can try again; I'm happy to wait a little. It could be a bug with how pytest-xdist interacts with the terminal and reports test successes/failures. |
|
Based on multiple outputs in the logs, here's an example: What I think is happening here is that it uses multiple lines because it adds the |
|
Okay, after a bit of experimentation, I think this is expected. I ran the [gw0] [ 59%] PASSED pyodide_build/tests/test_venv.py::test_venv_cli_args[options2-expected_calls2]
pyodide_build/tests/test_venv.py::test_venv_cli_args[options4-expected_calls4]
pyodide_build/tests/test_venv.py::test_venv_cli_args[options5-expected_calls5]
pyodide_build/tests/test_venv.py::test_supported_virtualenv_options
pyodide_build/tests/test_venv.py::test_venv_creation[default]This means that there is something more fishy going on, and it's not a problem with this repository alone – I checked the logs for other tests and this is everywhere. Perhaps pytest-dev/pytest-xdist#605? A worker Hence we should be fine to merge this, as it's easily a close-to-2x improvement (31 minutes ➡️ 16 minutes). Thanks for the reviews! |
4 participants
In this PR, I'm trying to investigate if we can shorten the time taken to run the test suite using
pytest-xdist, which can take upwards of ~25 minutes in the case of a full build.