pytest-dev / pytest-xdist

pytest plugin for distributed testing and loop-on-failures testing modes.
https://pytest-xdist.readthedocs.io
MIT License
1.43k stars 227 forks source link

Allow one test per node #894

Open robertschweizer opened 1 year ago

robertschweizer commented 1 year ago

The new --maxschedchunk option is great for running test suites with few slow tests.

However, it has a limitation in that it sends two tests to each worker at the start even if --maxschedchunk=1. If tests are sorted by how long they take, the two slowest tests will end up on the same node.

https://github.com/pytest-dev/pytest-xdist/issues/876 sounds like this is not easy to change.

Use-Case

I have a test suite with roughly this kind of runtime distribution:

15 min: 2 tests 5 min: 5 tests 0.5 min: 30 tests

The test fixtures are extremely cheap, much cheaper than the tests themselves. We know pretty well how long each test takes, and sort tests accordingly.

As a workaround, we can take into account xdist's scheduling details in the sorting of tests, but this is confusing and it would be nicer to fix it here.

kurt-cb commented 1 year ago

I implemented code in my branch to address this issue, however I have not prepared it for a PR. historically, xdist (inefficiently so) puts two tests on each node to start with and adds one more as each completes. This was required because get_next_item in pytest needs to tear down fixtures based on the upcoming test during cleanup of the current test. Since pytest doesn't really need to know ahead of time, but at test cleanup, I tricked pytest into delaying the decision. This allows the scheduler to put a single item on each worker, then decide when the test is complete (in the cleanup) what work is next.

It is a quite complicated change, as it effects the scheduler and some internal xdist flow. The "DummyNext" item is sent back to pytest, and then when pytest really needs the next item, the DummyNext proxies the new work. Although it would be better to fix pytest, this change requires no corresponding change to pytest.

Here is the change I made that introduces the fix.

This is the change in my fork

kurt-cb commented 1 year ago

Since it appears that the moderators have little to no interest in responding to my PR, I doubt that I will waste my time making this into a PR any time soon.

RonnyPfannschmidt commented 1 year ago

If you create something that's intentionally hard to track it's guaranteed to fall trough the cracks in a project

Your choice, you consequences

kurt-cb commented 1 year ago

I don't know what "hard to track" refers to. There are currently 9 PRs outstanding on xdist and my PR is the only one that passes tests. It also refers to the bug that it addresses #402. If repo owners are uninterested in reviewing PRs, then the project is dead.

As for the fork, it includes many unrelated changes and I have no intention of creating a PR for it. I created an Affinity scheduler that would bind workers based a named affinity (usually a type of cpu or resource only available to that worker).

Again, due to my experience with my one PR (as well as comments from others), it is a waste of my time to clean it up for a PR. It is not my consequences, as only the community suffers, not me.