Retain input ordering in loadscope (Rebase code for #1098)
#1217
Conversation
* Optionally retain input ordering in loadscope for tests where relative ordering matters. i.e. guarantee that, given [input_1, input_2], input_2 never runs before input_1. On any given worker, either input_ has ran before input_2, or input_1 has never and will never run on this worker.
for more information, see https://pre-commit.ci
darwintree
changed the title
loadscope (Rebase code for #1098)
| @@ -127,6 +127,17 @@ def pytest_addoption(parser: pytest.Parser) -> None: | |||
| "(default) no: Run tests inprocess, don't distribute." | |||
| ), | |||
| ) | |||
| group.addoption( | |||
| "--no-loadscope-reorder", | |||
There was a problem hiding this comment.
If particularly find negative options like this confusing. How about we change it to --loadscope-reorder and default it to True?
There was a problem hiding this comment.
I think the original specification might be more canonical.
I found some discussions on the flag name: https://stackoverflow.com/a/15008806/14223776.
However the dest variable naming could be improved. Will modify later
changelog/1083.trivial
Outdated
| @@ -0,0 +1 @@ | |||
| With `--no-loadscope-reorder`, retain input ordering in loadscope for tests where relative ordering matters. i.e. guarantee that, given [input_1, input_2],input_2 never runs before input_1. On any given worker, either input_1 has ran before input_2, or input_1 has never and will never run on this worker. This only applies when using `loadscope`. | |||
There was a problem hiding this comment.
The entry attempts to explain the feature in detail, which is great, however I find the explanation confusing: what are input_1 and input_2 exactly? Note that using multiple paragraphs is OK.
Also, it should use double backticks to denote code (this is RST).
While the change is simple, it is a new feature (it adds a new command-line flag), so it should be renamed to 1083.improvement.rst. 👍
|
Im not quite sure how to take this I haven't been able to review the original pr which is part of why the original pr hasn't been rebased I won't magically be able to review a rebase |
|
@RonnyPfannschmidt, that's a fair point. From my perspective, the PR is relatively small. Do you have any concerns about the functionality or implementation that make you want to take more time to evaluate it further? |
|
Im perhaps paranoid about order details The code is dealing with the wrong id types |
Would you specify what does "wrong id types" mean? Would be glad to resolve the issue |
This is a pull request coming from #1098 to enable the possibility to merge.
This pull request rebase the original changes to the latest master branch commit.
I'm not the original author but would be willing to help improve the code if there is any comments
Closes #1098
Thanks for submitting a PR, your contribution is really appreciated!
Here's a quick checklist that should be present in PRs:
Make sure to include reasonable tests for your change if necessary
We use towncrier for changelog management, so please add a news file into the
changelogfolder following these guidelines:Name it
$issue_id.$typefor example588.bugfix;If you don't have an issue_id change it to the PR id after creating it
Ensure type is one of
removal,feature,bugfix,vendor,docortrivialMake sure to use full sentences with correct case and punctuation, for example: