Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/1083.trivial
Original file line number Diff line number Diff line change
@@ -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`.
11 changes: 11 additions & 0 deletions src/xdist/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,17 @@ def pytest_addoption(parser: pytest.Parser) -> None:
"(default) no: Run tests inprocess, don't distribute."
),
)
group.addoption(
"--no-loadscope-reorder",
Copy link
Member

Choose a reason for hiding this comment

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

If particularly find negative options like this confusing. How about we change it to --loadscope-reorder and default it to True?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

--load-scope and --no-load-scope seems good to me. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--load-scope and --no-load-scope seems good to me. 👍

--load-scope might be confusing. As this feature is only enabled on --dist loadscope and it just affects the loadscope order behaviour. if --loadscope-reorder would be ok, I would add another option --loadscope-reorder (whereas, --loadscope-reorder and --no-loadscope-reorder will be finally added)

Copy link
Member

Choose a reason for hiding this comment

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

whereas, --loadscope-reorder and --no-loadscope-reorder will be finally added

Ahh sorry, yes, that is what I meant actually. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

action="store_true",
dest="noloadscopenoreorder",
default=False,
help=(
"Reorders tests when used in conjunction with loadscope.\n"
"Will order tests by number of tests per scope as a best-effort"
" attempt to evenly distribute scopes across all workers."
),
)
group.addoption(
"--tx",
dest="tx",
Expand Down
14 changes: 9 additions & 5 deletions src/xdist/scheduler/loadscope.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,11 +371,15 @@ def schedule(self) -> None:
work_unit = unsorted_workqueue.setdefault(scope, {})
work_unit[nodeid] = False

# Insert tests scopes into work queue ordered by number of tests.
for scope, nodeids in sorted(
unsorted_workqueue.items(), key=lambda item: -len(item[1])
):
self.workqueue[scope] = nodeids
if self.config.option.noloadscopenoreorder:
for scope, nodeids in unsorted_workqueue.items():
self.workqueue[scope] = nodeids
else:
# Insert tests scopes into work queue ordered by number of tests.
for scope, nodeids in sorted(
unsorted_workqueue.items(), key=lambda item: -len(item[1])
):
self.workqueue[scope] = nodeids

# Avoid having more workers than work
extra_nodes = len(self.nodes) - len(self.workqueue)
Expand Down
18 changes: 18 additions & 0 deletions testing/acceptance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1254,6 +1254,24 @@ def test(i):
"test_b.py::test", result.outlines
) == {"gw0": 20}

def test_workqueue_ordered_by_input(self, pytester: pytest.Pytester) -> None:
test_file = """
import pytest
@pytest.mark.parametrize('i', range({}))
def test(i):
pass
"""
pytester.makepyfile(test_a=test_file.format(10), test_b=test_file.format(20))
result = pytester.runpytest(
"-n2", "--dist=loadscope", "--no-loadscope-reorder", "-v"
)
assert get_workers_and_test_count_by_prefix(
"test_a.py::test", result.outlines
) == {"gw0": 10}
assert get_workers_and_test_count_by_prefix(
"test_b.py::test", result.outlines
) == {"gw1": 20}

def test_module_single_start(self, pytester: pytest.Pytester) -> None:
"""Fix test suite never finishing in case all workers start with a single test (#277)."""
test_file1 = """
Expand Down