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

chore: serialize engine integration tests #2245

Merged
merged 4 commits into from
Mar 12, 2024

Conversation

eakmanrq
Copy link
Contributor

@eakmanrq eakmanrq commented Mar 11, 2024

pytest-xdist will now start evenly distributing the tests across the N workers but make sure that tests with a common xdist_group end up on the same worker and therefore run sequentially. The only tests with this grouping applied are these integration tests.

Only negative with this change is we lose the "worksteal" functionality where idle workers are greedy and steal work from other workers. This is a minor negative though.

Edit: impact of not having "worksteal" is bigger then I thought. It appears "worksteal" still tried to group modules together but did not provide guarantees of this. Therefore some module functions couldn't be parallelized but we never saw this because worksteal would group them together and therefore run them sequentially. Added grouping for those and we might heave some flaky tests for a bit until we get everything grouped together correctly. pytest-dev/pytest-xdist#890

@eakmanrq eakmanrq requested review from treysp and izeigerman March 11, 2024 19:21
Copy link
Member

@izeigerman izeigerman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once green

@eakmanrq eakmanrq force-pushed the eakmanrq/serialize_engine_integration_tests branch 3 times, most recently from 98ed9a8 to 9d3578c Compare March 11, 2024 23:34
@eakmanrq eakmanrq force-pushed the eakmanrq/serialize_engine_integration_tests branch from 9d3578c to 89d0e20 Compare March 11, 2024 23:56
@eakmanrq eakmanrq enabled auto-merge (squash) March 12, 2024 00:00
@eakmanrq eakmanrq merged commit e6d952c into main Mar 12, 2024
9 of 10 checks passed
@eakmanrq eakmanrq deleted the eakmanrq/serialize_engine_integration_tests branch March 12, 2024 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants