enable parallel tests with pytest-xdist and pytest-split

[ytausch]

Enabling pytest-xdist (which is already in the list of our dependencies) brings down test time from 12 minutes to around 8:30, which saves us 3:30 min per test run.

For the model tests, test runtime is roughly equal but I still enabled it for consistency.

As you can see in the commit history, I also evaluated using pytest-split for splitting tests across multiple GitHub Runners but could not see a significant performance gain because of the overhead involved with building the Docker container. I found a naive approach (building the Docker container 4 times in parallel) that brings us down another minute to 7:20, saving 40 % wait time in total.

Are there any tests on the MongoDB that could break with this? @beckermr

• Pydantic model updated or no update needed

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.97%. Comparing base (7ecb031) to head (18387e6).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2874      +/-   ##
==========================================
- Coverage   76.98%   76.97%   -0.02%     
==========================================
  Files         112      112              
  Lines       11975    11977       +2     
==========================================
  Hits         9219     9219              
- Misses       2756     2758       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ytausch]

@0xbe7a Thoughts about this?

[ytausch]

There seems to be something wrong with the coverage. Converting to draft.

[beckermr]

The tests used to break in general with this which is why I turned it off. It's a good idea to try again.

[beckermr]

Pytest split is really cool!

The mongodb tests happen against a test instance in a docker container in the GitHub runner, so that should work fine.

[beckermr]

We'll need to adjust the required tests for this pr. I will do that once we merge it.

[ytausch]

Note: I am wildly pushing here to evaluate different approaches.

I just found out that running pytest-xdist (parallelization within runners) disables the MongoDB tests because the sensitive environment variable MONGODB_CONNECTION_STRING is not visible for the worker processes, as they are hidden by the sensitive_env logic. Might be that my original runtime measures are incorrect because of this.

[ytausch]

Okay, so here is a summary of what I found out:

• enabling pytest-xdist requires us force the MongoDB tests to run serially as they write to the same collections and could interfere with each other - I think @beckermr you might not have considered that in your reply? I implemented such a serialization approach here: ytausch@a41b902

  • Edit: pytest.mark.xdist_group might even be better (docs)

• enabling pytest-xdist disables the MongoDB tests (see my comment above) - a solution for that would need to be developed

In the diff of this PR (or here, you find my analysis of the runners we already use in this project. From the analysis, I figure out that a splitting the tests across 4 runners is too much. Requesting more runners than available does lead to test times to double, which we should avoid.

• For that reason, I tried splitting the tests across 2 runners only - which lead to the tests split unequally across runners, yielding a runtime of 11:10, which is only an improvement of 50 seconds.
• This can be improved by measuring test execution times, which is natively supported by pytest-split. Caching the Test Durations as GitHub Cache Artifacts leads to a test time of 9:30.

It is possible to combine pytest-xdist and pytest-split.

Other approaches for speeding up test times could include:

• Optimizing the Docker build process (see Docker Image includes .git folder #2876)

[ytausch]

Open issues:

• the coverage decreases unexpectedly
• deleting from the GitHub actions cache currently does not work -> this might just because I am an external collaborator to this repo and could go away after merging
• enabling python-xdist would require a solution for the MongoDB connecting string env var -> I might do this in a follow-up PR

[beckermr]

My opinion is that we should not let the perfect be the enemy of the good and we should go ahead.

Are there tests not being run now and that is causing the coverage decrease?

[beckermr]

I was thinking through this, and I think we should do the test timings files like the coverage ones.

Specifically,

1. We upload the test durations as artifacts
2. download all of them at once with a glob like we do the for the coverage files
3. combine them with jq
4. update the cache

For the cache, instead of a delete and then save, which might cause a race condition for another job, we can use a key with say the branch name or github ref in it. Then we can restore adaptive using restore keys and save new caches easily when there is a miss.