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

Split deterministically regardless of test order Fix #23 #52

Merged
merged 1 commit into from
Apr 22, 2022

Conversation

bullfest
Copy link
Contributor

@bullfest bullfest commented Apr 22, 2022

Description

Hi, I believe this should fix the issues with running this together with test-suite-randomization tools such as pytest-randomly or pytest-random-order when using the least_duration algorithm, I don't think the problem is possible to solve for duration_based_chunks.
The idea here is that if the tests always are in the same order when splitting (by sorting by str-representation) then the split should always be done the same way.

Opening this before fixing documentation + changelog in case that I've missed something obvious and this being rejected. 😅

Checklist

  • Tests covering the new functionality have been added
  • Documentation has been updated OR the changes are too minor to be documented
  • The Changes are listed in the CHANGELOG.md OR the changes are insignificant

@@ -141,8 +141,8 @@ class TestSplitToSuites:
["test_1", "test_2", "test_3", "test_4", "test_5", "test_6", "test_7"],
),
(2, 2, "duration_based_chunks", ["test_8", "test_9", "test_10"]),
(2, 1, "least_duration", ["test_3", "test_5", "test_6", "test_8", "test_10"]),
(2, 2, "least_duration", ["test_1", "test_2", "test_4", "test_7", "test_9"]),
(2, 1, "least_duration", ["test_3", "test_5", "test_7", "test_9", "test_10"]),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The split changing here is a bit unfortunate, but happens because "test_10" < "test_2" which shuffles everything around a bit, but the contract for least_duration should still be upheld.

@bullfest
Copy link
Contributor Author

Sorry about that failing test, I had changed around the code a bit when adding types and missed re-running it 🙃

@bullfest
Copy link
Contributor Author

I've now run this on a "real" test-suite together with pytest-randomly with a mix of unit/integration tests and it seems to split in a nice way, with no duplicates between splits and the same total number of tests run. 😄

@jerry-git
Copy link
Owner

Great! Could you also add a line to the changelog 🙏 And does this make any difference wrt the mention about pytest-random-order in readme?

@bullfest
Copy link
Contributor Author

Done! Would it be possible to do a release relatively soon? 😃

@jerry-git
Copy link
Owner

Gimme 15 minutes :)

@jerry-git jerry-git merged commit 43f6005 into jerry-git:master Apr 22, 2022
@jerry-git
Copy link
Owner

Needed only 6 minutes, it's available in 0.8.0. Thanks for the contribution! 🏅

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.

None yet

2 participants