diff --git a/subprojects/robotpy-wpilib/tests/requirements.txt b/subprojects/robotpy-wpilib/tests/requirements.txt index e079f8a60..114768e8b 100644 --- a/subprojects/robotpy-wpilib/tests/requirements.txt +++ b/subprojects/robotpy-wpilib/tests/requirements.txt @@ -1 +1,2 @@ pytest +pytest-order diff --git a/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py b/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py index 40326e880..04b153ce4 100644 --- a/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py +++ b/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py @@ -432,3 +432,616 @@ def test_state_transitions(robot, control): ) result.assert_outcomes(passed=1) + + +# Seconds a chain's sentinel writer sleeps before writing, when the test that reads that +# sentinel runs in an isolated subprocess. +# +# Why this is needed at all: a plain in-process test writes its sentinel within milliseconds, +# while a robot test needs a few hundred ms to spawn its subprocess and reach its first +# assertion. Without ordering support the robot test is merely *started* early, so by the time +# it actually looks, a fast writer has already written -- the case passes and proves nothing. +# Delaying the writer removes that race so the case fails without the fix, which is the only +# thing that makes it a guard. +# +# Choosing the value: sweeping the delay against the unfixed plugin put the cutoff between +# 150ms and 200ms on an M-series Mac (0/5 runs caught the bug at 150ms, 5/5 at 200ms). 1s +# leaves roughly 5x margin. +# +# The risk this carries: on a machine slow enough that a robot subprocess takes over a second +# to reach its assertion, these cases quietly stop catching regressions. They never fail +# wrongly -- with ordering support present they pass regardless of timing -- so the degradation +# is silent, which is the dangerous direction. If this suite starts running on much slower +# hardware, re-measure the cutoff rather than assuming this value still has margin. +ORDER_HANDOFF_DELAY = 1.0 + + +def _handoff_sleep(reader_is_robot: bool) -> str: + """Sleep line for a sentinel writer, or nothing when the reader is in-process.""" + return f" time.sleep({ORDER_HANDOFF_DELAY})\n" if reader_is_robot else "" + + +@pytest.mark.parametrize("marker_style", ["numeric", "after", "before"]) +@pytest.mark.parametrize( + "first_type, middle_type, last_type", + [ + ("ROBOT", "ROBOT", None), + ("ROBOT", "PLAIN", None), + ("PLAIN", "ROBOT", None), + ("PLAIN", "PLAIN", None), + ("ROBOT", "ROBOT", "PLAIN"), + ("ROBOT", "PLAIN", "ROBOT"), + ("PLAIN", "ROBOT", "ROBOT"), + ("ROBOT", "PLAIN", "PLAIN"), + ("PLAIN", "ROBOT", "PLAIN"), + ("PLAIN", "PLAIN", "ROBOT"), + ], +) +def test_order_marker_enforces_sequencing( + pytester, first_type, middle_type, last_type, marker_style +): + """ + Order markers enforce a first->middle->last chain across all mixed + robot/non-robot permutations. + + test_first writes sentinel_1; test_middle reads sentinel_1 and writes + sentinel_2; test_last reads sentinel_2. The file lists them in reverse + (last, middle, first) so collection order would fail -- passing proves the + full chain was enforced. + + ROBOT = robot fixture (isolated subprocess) PLAIN = plain test (in-process) + + numeric: first=order(1), middle=order(2), last=order(3) + after: middle=order(after="test_first"), last=order(after="test_middle") + before: first=order(before="test_middle"), middle=order(before="test_last") + + Not every permutation can detect a broken run loop, and that is expected: + + - PLAIN-PLAIN-None has no robot test at all, so there is nothing for the plugin to + mis-order -- the old loop deferred non-robot tests but preserved their order among + themselves. These three cases assert the chain still works; they are not regression + guards and cannot be made into any. + - Where the sentinel *reader* is a robot test, the writer sleeps ORDER_HANDOFF_DELAY so the + reader cannot win the spawn race and pass by luck. See that constant for the measurement + and the slow-machine caveat. + """ + _make_robot_module(pytester) + _configure_isolated_plugin(pytester, parallelism=4) + + # Only a robot reader needs the writer slowed down; a plain reader runs in-process and + # already observes the violation directly. + first_sleep = _handoff_sleep(middle_type == "ROBOT") + middle_sleep = _handoff_sleep(last_type == "ROBOT") + + def params(t): + return "(robot)" if t == "ROBOT" else "()" + + if marker_style == "numeric": + first_mark = "@pytest.mark.order(1)\n" + middle_mark = "@pytest.mark.order(2)\n" + last_mark = "@pytest.mark.order(3)\n" + elif marker_style == "before": + first_mark = '@pytest.mark.order(before="test_middle")\n' + middle_mark = '@pytest.mark.order(before="test_last")\n' + last_mark = "" + else: + first_mark = "" + middle_mark = '@pytest.mark.order(after="test_first")\n' + last_mark = '@pytest.mark.order(after="test_middle")\n' + + pytester.makepyfile( + test_order_sequence=( + """\ +import pathlib +import time + +import pytest + + +""" + + ( + f"""{last_mark}def test_last{params(last_type)}: + assert pathlib.Path("sentinel_2.txt").exists(), "test_middle must run before test_last" + + +""" + if last_type is not None + else "" + ) + + f"""{middle_mark}def test_middle{params(middle_type)}: + assert pathlib.Path("sentinel_1.txt").exists(), "test_first must run before test_middle" +{middle_sleep} pathlib.Path("sentinel_2.txt").write_text("done") + + +{first_mark}def test_first{params(first_type)}: +{first_sleep} pathlib.Path("sentinel_1.txt").write_text("done") +""" + ) + ) + + result = pytester.runpytest_subprocess("-vv") + count_of_tests = sum(x is not None for x in [first_type, middle_type, last_type]) + result.assert_outcomes(passed=count_of_tests) + + +@pytest.mark.parametrize( + "ini_body, first_mark, second_mark", + [ + ( + "addopts = --strict-markers", + "@pytest.mark.order(1)", + "@pytest.mark.order(2)", + ), + ( + "filterwarnings = error", + "@pytest.mark.order(1)", + "@pytest.mark.order(2)", + ), + ( + "filterwarnings = error", + "", + '@pytest.mark.order(after="test_first")', + ), + ], + ids=["strict_markers", "warnings_as_errors", "relative_marker_stays_quiet"], +) +def test_order_marker_is_registered_in_isolated_subprocess( + pytester, ini_body, first_mark, second_mark +): + """ + Ordered robot tests must still run when the project treats unknown markers + as a hard failure. + + The isolated subprocess runs with "-p no:order", which unloads pytest-order + along with its registration of the "order" marker -- but the decorators are + still on the tests the subprocess collects. Both settings below reach the + subprocess (it inherits the parent's command line and chdirs to the project + root before reading the ini), so without WorkerPlugin re-registering the + marker each case errors during collection *inside the worker*: + + --strict-markers -> Failed: 'order' not found in `markers` configuration option + filterwarnings=error -> pytest.PytestUnknownMarkWarning: Unknown pytest.mark.order + + Both are exercised because they fail through different mechanisms: the + first is pytest's own strict-marker check, the second is warning promotion. + + The third case guards the opposite edge. Re-registering the marker must not + drag pytest-order back into the worker: a relative marker there would emit + "cannot execute 'test_second' relative to others: 'test_first' - ignoring + the marker", because the worker only ever collects one test and cannot see + the other. Under filterwarnings=error that warning is an error, so this + case fails if the marker registration ever turns into a plugin reload. + + Note this guard only bites on the robot tests -- the parent process keeps + pytest-order loaded, so the marker is registered there either way. + """ + _make_robot_module(pytester) + _configure_isolated_plugin(pytester, parallelism=2) + pytester.makeini(f"[pytest]\n{ini_body}\n") + + pytester.makepyfile(test_strict_order=f"""\ +import pathlib + +import pytest + + +{second_mark} +def test_second(robot): + assert pathlib.Path("strict_sentinel.txt").exists() + + +{first_mark} +def test_first(robot): + pathlib.Path("strict_sentinel.txt").write_text("done") +""") + + result = pytester.runpytest_subprocess("-vv") + result.assert_outcomes(passed=2, errors=0) + + +@pytest.mark.parametrize( + "a_fixture, b_fixture", + [("robot", "robot"), ("robot", ""), ("", "robot")], + ids=["RR", "RN", "NR"], +) +def test_unordered_tests_still_run_in_parallel(pytester, a_fixture, b_fixture): + """ + Tests WITHOUT @pytest.mark.order must not be serialised by order-marker + support. With parallelism=2, two 1.5 s tests must overlap in wall-clock + time. + + NR is the case collection order alone cannot deliver: the plain test is + collected first, so following the given order would run it to completion + before the subprocess was even spawned. Within a group the run loop is free + to schedule for throughput, so it starts the isolated test before running any + in-process test and the two overlap regardless of which was collected first. + + NN is omitted: two in-process tests both run here, and this process runs one + test at a time -- serial by design, and no scheduling can change it. + """ + _make_robot_module(pytester) + _configure_isolated_plugin(pytester, parallelism=2) + + def params(f): + return f"({f})" if f else "()" + + pytester.makepyfile(test_parallel_execution=f"""\ +import pathlib +import time + + +def test_a{params(a_fixture)}: + pathlib.Path("a_start.txt").write_text(str(time.monotonic())) + time.sleep(1.5) + pathlib.Path("a_end.txt").write_text(str(time.monotonic())) + + +def test_b{params(b_fixture)}: + pathlib.Path("b_start.txt").write_text(str(time.monotonic())) + time.sleep(1.5) + pathlib.Path("b_end.txt").write_text(str(time.monotonic())) +""") + + result = pytester.runpytest_subprocess("-vv") + result.assert_outcomes(passed=2) + + root = pathlib.Path(pytester.path) + a_end = float((root / "a_end.txt").read_text()) + b_start = float((root / "b_start.txt").read_text()) + assert ( + b_start < a_end + ), f"Expected parallel: b_start={b_start:.3f} a_end={a_end:.3f}" + + +def _read_times(pytester, *names): + root = pathlib.Path(pytester.path) + return {n: float((root / f"{n}.txt").read_text()) for n in names} + + +_TIMED_ROBOT_TEST = """\ +def {name}(robot): + pathlib.Path("{name}_start.txt").write_text(str(time.monotonic())) + time.sleep(1.0) + pathlib.Path("{name}_end.txt").write_text(str(time.monotonic())) +""" + + +def test_module_level_order_marker_parallel_within_group(pytester): + """ + A module-level order marker positions the module as a whole; pytest-order + explicitly does not constrain the order of the tests *inside* it ("the tests + inside each module will be run in the same order as without any ordering"). + + So the two modules must be serialised against each other, but the robot tests + within a module must still overlap. test_group_a is collected first but marked + order(2), so passing also proves the modules were actually reordered. + """ + _make_robot_module(pytester) + _configure_isolated_plugin(pytester, parallelism=4) + + def module_src(order, names): + return ( + "import pathlib\nimport time\nimport pytest\n\n" + f"pytestmark = pytest.mark.order({order})\n\n\n" + + "\n\n".join(_TIMED_ROBOT_TEST.format(name=n) for n in names) + ) + + pytester.makepyfile( + test_group_a=module_src(2, ["test_a1", "test_a2"]), + test_group_b=module_src(1, ["test_b1", "test_b2"]), + ) + + result = pytester.runpytest_subprocess("-vv") + result.assert_outcomes(passed=4) + + t = _read_times( + pytester, + "test_a1_start", + "test_a1_end", + "test_a2_start", + "test_a2_end", + "test_b1_start", + "test_b1_end", + "test_b2_start", + "test_b2_end", + ) + + # the order(1) module must fully complete before the order(2) module starts + assert max(t["test_b1_end"], t["test_b2_end"]) <= min( + t["test_a1_start"], t["test_a2_start"] + ), f"module boundary not enforced: {t}" + + # ...but within each module the tests must have overlapped + assert t["test_b2_start"] < t["test_b1_end"], f"module b serialised: {t}" + assert t["test_a2_start"] < t["test_a1_end"], f"module a serialised: {t}" + + +def test_class_level_order_marker_parallel_within_group(pytester): + """ + Same as the module-level case, for a class-level marker: "the class as a whole + will be reordered without changing the test order inside the test class". + TestAlpha is collected first but marked order(2). + """ + _make_robot_module(pytester) + _configure_isolated_plugin(pytester, parallelism=4) + + def class_src(cls, order, names): + body = "\n".join( + " " + line if line else "" + for n in names + for line in _TIMED_ROBOT_TEST.format(name=n) + .replace("(robot)", "(self, robot)") + .split("\n") + ) + return f"@pytest.mark.order({order})\nclass {cls}:\n{body}\n" + + pytester.makepyfile( + test_classes="import pathlib\nimport time\nimport pytest\n\n\n" + + class_src("TestAlpha", 2, ["test_x1", "test_x2"]) + + "\n" + + class_src("TestBeta", 1, ["test_y1", "test_y2"]) + ) + + result = pytester.runpytest_subprocess("-vv") + result.assert_outcomes(passed=4) + + t = _read_times( + pytester, + "test_x1_start", + "test_x1_end", + "test_x2_start", + "test_x2_end", + "test_y1_start", + "test_y1_end", + "test_y2_start", + "test_y2_end", + ) + + assert max(t["test_y1_end"], t["test_y2_end"]) <= min( + t["test_x1_start"], t["test_x2_start"] + ), f"class boundary not enforced: {t}" + + assert t["test_y2_start"] < t["test_y1_end"], f"TestBeta serialised: {t}" + assert t["test_x2_start"] < t["test_x1_end"], f"TestAlpha serialised: {t}" + + +def test_separate_markers_with_equal_value_are_not_grouped(pytester): + """ + Grouping is by marker *identity*, not equality. Inheritance hands every test in + a class/module the same Mark object, but two independent @pytest.mark.order(5) + decorators are distinct objects that merely compare equal. Those stay + serialised -- the conservative choice, since only the inherited case has + documented "order inside is unconstrained" semantics. + """ + _make_robot_module(pytester) + _configure_isolated_plugin(pytester, parallelism=4) + + pytester.makepyfile( + test_equal_markers="import pathlib\nimport time\nimport pytest\n\n\n" + + "@pytest.mark.order(5)\n" + + _TIMED_ROBOT_TEST.format(name="test_p") + + "\n\n@pytest.mark.order(5)\n" + + _TIMED_ROBOT_TEST.format(name="test_q") + ) + + result = pytester.runpytest_subprocess("-vv") + result.assert_outcomes(passed=2) + + t = _read_times(pytester, "test_p_end", "test_q_start") + assert ( + t["test_p_end"] <= t["test_q_start"] + ), f"equal-valued markers were merged into one group: {t}" + + +def test_robot_tests_ordered_relative_to_each_other(pytester): + """ + Two robot-fixture tests carrying order markers must run one after the other. + + Both run in isolated subprocesses, so without order support they are started together + and overlap. They are written to the file in reverse, so collection order alone fails. + """ + _make_robot_module(pytester) + _configure_isolated_plugin(pytester, parallelism=4) + + pytester.makepyfile(test_robot_chain=f"""\ +import pathlib +import time + +import pytest + + +@pytest.mark.order(2) +def test_robot_second(robot): + assert pathlib.Path("robot_first.txt").exists(), "test_robot_first must run first" + + +@pytest.mark.order(1) +def test_robot_first(robot): + time.sleep({ORDER_HANDOFF_DELAY}) + pathlib.Path("robot_first.txt").write_text("done") +""") + + result = pytester.runpytest_subprocess("-vv") + result.assert_outcomes(passed=2) + + +def test_module_level_order_vs_robot_tests(pytester): + """ + A module-level order marker must sequence a robot test against another module's tests. + + test_group_a is collected first but marked order(2), so passing also proves the modules + were reordered. The robot test is the reader, since that is the direction the old run + loop broke: robot tests were hoisted ahead of every non-robot test. + """ + _make_robot_module(pytester) + _configure_isolated_plugin(pytester, parallelism=4) + + pytester.makepyfile( + test_group_a="""\ +import pathlib + +import pytest + +pytestmark = pytest.mark.order(2) + + +def test_robot_reads(robot): + assert pathlib.Path("early.txt").exists(), "the order(1) module must run first" +""", + test_group_b=f"""\ +import pathlib +import time + +import pytest + +pytestmark = pytest.mark.order(1) + + +def test_plain_writes(): + time.sleep({ORDER_HANDOFF_DELAY}) + pathlib.Path("early.txt").write_text("done") +""", + ) + + result = pytester.runpytest_subprocess("-vv") + result.assert_outcomes(passed=2) + + +def test_class_level_order_vs_robot_tests(pytester): + """ + A class-level order marker must sequence a robot test against another class's tests. + + TestLate is written first but marked order(2), so passing also proves the classes were + reordered. + """ + _make_robot_module(pytester) + _configure_isolated_plugin(pytester, parallelism=4) + + pytester.makepyfile(test_class_chain=f"""\ +import pathlib +import time + +import pytest + + +@pytest.mark.order(2) +class TestLate: + def test_robot_reads(self, robot): + assert pathlib.Path("early.txt").exists(), "TestEarly must run first" + + +@pytest.mark.order(1) +class TestEarly: + def test_plain_writes(self): + time.sleep({ORDER_HANDOFF_DELAY}) + pathlib.Path("early.txt").write_text("done") +""") + + result = pytester.runpytest_subprocess("-vv") + result.assert_outcomes(passed=2) + + +def test_class_level_relative_order_vs_robot_tests(pytester): + """ + A class-level RELATIVE marker (`after=`) must sequence a robot test against another class. + + pytest-order documents referencing a test class by name from `before=`/`after=`, which is a + different code path from the ordinal markers covered above. + """ + _make_robot_module(pytester) + _configure_isolated_plugin(pytester, parallelism=4) + + pytester.makepyfile(test_class_relative=f"""\ +import pathlib +import time + +import pytest + + +@pytest.mark.order(after="TestEarly") +class TestLate: + def test_robot_reads(self, robot): + assert pathlib.Path("early.txt").exists(), "TestEarly must run first" + + +class TestEarly: + def test_plain_writes(self): + time.sleep({ORDER_HANDOFF_DELAY}) + pathlib.Path("early.txt").write_text("done") +""") + + result = pytester.runpytest_subprocess("-vv") + result.assert_outcomes(passed=2) + + +def test_module_level_relative_order_vs_robot_tests(pytester): + """ + A module-level RELATIVE marker must sequence a robot test against another module's test. + + Note this form -- `pytestmark = pytest.mark.order(after="path::test")` -- is not documented + upstream; pytest-order shows `before=`/`after=` only at function and class scope. It works + because pytestmark is ordinary pytest marker inheritance, but it is de facto rather than + de jure, so this test also serves to detect it changing. + """ + _make_robot_module(pytester) + _configure_isolated_plugin(pytester, parallelism=4) + + pytester.makepyfile( + test_rel_a="""\ +import pathlib + +import pytest + +pytestmark = pytest.mark.order(after="test_rel_b.py::test_plain_writes") + + +def test_robot_reads(robot): + assert pathlib.Path("early.txt").exists(), "test_rel_b must run first" +""", + test_rel_b=f"""\ +import pathlib +import time + + +def test_plain_writes(): + time.sleep({ORDER_HANDOFF_DELAY}) + pathlib.Path("early.txt").write_text("done") +""", + ) + + result = pytester.runpytest_subprocess("-vv") + result.assert_outcomes(passed=2) + + +def test_function_marker_inside_marked_class(pytester): + """ + A function-level marker inside an already-marked class wins, and forms its own group. + + get_closest_marker returns the function's own Mark rather than the class's inherited one, + so the marked method is a group of one: it is serialised against its own siblings, not + merged with them. Here the method marked order(1) must overtake its class-mates even though + the class as a whole is marked order(2). + """ + _make_robot_module(pytester) + _configure_isolated_plugin(pytester, parallelism=4) + + pytester.makepyfile(test_nested_marker=f"""\ +import pathlib +import time + +import pytest + + +@pytest.mark.order(2) +class TestGroup: + def test_robot_reads(self, robot): + assert pathlib.Path("early.txt").exists(), "the order(1) method must run first" + + @pytest.mark.order(1) + def test_plain_writes_first(self): + time.sleep({ORDER_HANDOFF_DELAY}) + pathlib.Path("early.txt").write_text("done") +""") + + result = pytester.runpytest_subprocess("-vv") + result.assert_outcomes(passed=2) diff --git a/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py b/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py index 4114cc4d4..200d0a3cb 100644 --- a/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py +++ b/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py @@ -10,6 +10,8 @@ import time import typing as T +from collections import deque + import pytest import robotpy.main @@ -82,6 +84,24 @@ def __init__(self, channel: multiprocessing.connection.Connection): def sendevent(self, name: str, **kwargs: object): self.channel.send((name, kwargs)) + @pytest.hookimpl + def pytest_configure(self, config: pytest.Config): + # The worker runs with "-p", "no:order" (see _run_test), which unloads + # pytest-order entirely -- including its registration of the "order" + # marker. The @pytest.mark.order decorators are still on the tests we + # collect here, so without re-registering the marker they are "unknown" + # to this process: --strict-markers turns that into a collection error, + # and filterwarnings=error promotes the PytestUnknownMarkWarning into + # one. Both settings reach the worker, because it inherits the parent's + # command line and chdirs to the project root before reading the ini. + # + # Registering the marker restores those configurations without + # reactivating the reordering we deliberately turned off. + config.addinivalue_line( + "markers", + "order(*args, **kwargs): ordering is resolved by the parent process", + ) + @pytest.hookimpl(wrapper=True) def pytest_sessionstart(self, session: pytest.Session): self.config = session.config @@ -149,7 +169,27 @@ def _run_test( worker_plugin = WorkerPlugin(pipe) ec = pytest.main( - [item_nodeid, "--no-header", "-p", "no:terminalreporter", *config_args], + [ + item_nodeid, + "--no-header", + "-p", + "no:terminalreporter", + # "-p", "no:order" tells pytest in the isolated subprocess to not run pytest-order or look at + # the @pytest.mark.order decorators. This is fine because the isolated subprocess runs just one + # test at a time, so order does not matter at this level. The purpose of not running + # pytest-order is so that it doesn't give misleading warnings like: + # WARNING: cannot execute 'test_step2_reads_sentinel' relative to others: + # 'test_step1_writes_sentinel' - ignoring the marker. + # The warning would be accurate from the isolated subprocess point of view, + # it can't see the other test, but it is misleading because the main process + # successfully ordered the tests. + # + # Unloading the plugin also drops its registration of the "order" marker, so + # WorkerPlugin.pytest_configure re-registers it. + "-p", + "no:order", + *config_args, + ], plugins=[plugin, worker_plugin], ) @@ -184,6 +224,40 @@ def set_exit_code(self, ec: int): self.exit_code = ec +def _order_marker_groups( + items: list[pytest.Item], +) -> T.Iterator[list[pytest.Function]]: + """ + Split collected items into consecutive runs that share one order marker. + + An order marker applied to a class or module is inherited by every test underneath it, + and get_closest_marker returns the *same* Mark object for each of them. Grouping by + identity therefore treats those tests as one group: the group is serialized against + everything outside it, while its members -- whose relative order pytest-order explicitly + does not constrain -- can be scheduled freely. Identity is required rather than ==, + because two independent @pytest.mark.order(5) decorators compare equal but are separate + constraints that must not be merged into one group. + + Unmarked tests all return None, so a run of them forms a single group. + """ + group: list[pytest.Function] = [] + prev_marker = None + + for item in items: + assert isinstance(item, pytest.Function) + + marker = item.get_closest_marker("order") + if group and marker is not prev_marker: + yield group + group = [] + + group.append(item) + prev_marker = marker + + if group: + yield group + + class IsolatedTestsPlugin: """ This pytest plugin runs any test that uses the 'robot' fixture in an @@ -237,29 +311,59 @@ def pytest_runtestloop(self, session: pytest.Session) -> bool: return True running: list[IsolatedTestJob] = [] - deferred: list[pytest.Function] = [] try: - # Start any tests that use the robot fixture first. Tests that don't - # use the robot fixture will be ran later - for item in session.items: - assert isinstance(item, pytest.Function) - if "robot" not in item.fixturenames: - deferred.append(item) - continue - - while len(running) >= self._parallelism: + # Run tests one order marker group at a time, preserving the boundaries between + # groups while scheduling freely inside them. + # + # pytest-order has already sorted session.items during collection, so all this has + # to do is stop tests from overtaking each other across an ordering boundary. + # + # Within a group pytest-order has explicitly not constrained anything, so the order + # inside one carries no meaning and this loop is free to schedule for throughput. + # It does that by never leaving this process idle while it still has work: fill + # every subprocess slot first, then run in-process tests in the time those + # subprocesses take, and only block once there is nothing left to overlap with. + for group in _order_marker_groups(session.items): + # Crossing an ordering boundary: everything started so far has to finish before + # anything on the far side of the boundary begins. One drain covers both leaving + # a group -- so @pytest.mark.order(before="NAME") completes first -- and entering + # one, so @pytest.mark.order() and @pytest.mark.order(after="NAME") see + # the tests they were sorted behind already finished. + while running: self._wait_for_jobs(running, session) - running.append(self._start_isolated_test(item)) - self._maybe_raise(session) - - # Run the in-process tests now while the robot tests are finishing - for idx, item in enumerate(deferred): - nextitem = deferred[idx + 1] if idx + 1 < len(deferred) else None - session.config.hook.pytest_runtest_protocol( - item=item, nextitem=nextitem - ) - self._maybe_raise(session) + # Tests using the "robot" fixture go to isolated subprocesses, everything else + # runs here. Both are queues the loop below drains against each other. + isolated = deque(i for i in group if "robot" in i.fixturenames) + in_process = deque(i for i in group if "robot" not in i.fixturenames) + + while isolated or in_process: + # Reap whatever has already finished so its slot can be reused. This has to + # be a poll rather than a wait: slots only free up inside _wait_for_jobs, so + # without this a long run of in-process tests would hold the subprocess count + # at its high-water mark and refuse to start anything new. + self._wait_for_jobs(running, session, timeout=0) + + while isolated and len(running) < self._parallelism: + running.append(self._start_isolated_test(isolated.popleft())) + self._maybe_raise(session) + + if in_process: + item = in_process.popleft() + + # nextitem is pytest's teardown-optimization hint. These run back to + # back, so whatever is left at the front always qualifies; the last + # test hands over None because the group is drained after it. + nextitem = in_process[0] if in_process else None + + session.config.hook.pytest_runtest_protocol( + item=item, nextitem=nextitem + ) + self._maybe_raise(session) + elif running: + # Every slot is busy and there is no in-process work left to fill the + # wait with, so there is nothing to do but block. + self._wait_for_jobs(running, session) while running: self._wait_for_jobs(running, session) @@ -301,11 +405,26 @@ def _start_isolated_test(self, item: pytest.Function) -> IsolatedTestJob: start_time=time.time(), ) - def _wait_for_jobs(self, running: list[IsolatedTestJob], session: pytest.Session): + def _wait_for_jobs( + self, + running: list[IsolatedTestJob], + session: pytest.Session, + timeout: float | None = None, + ): + """ + Collect results from finished jobs, removing them from `running`. + + Blocks until at least one job has something to say. Pass timeout=0 to + poll instead, which reaps whatever has already finished and returns + immediately -- used to free subprocess slots without stalling this + process while it still has in-process tests to run. + """ if not running: return - ready = multiprocessing.connection.wait([job.conn for job in running]) + ready = multiprocessing.connection.wait( + [job.conn for job in running], timeout=timeout + ) for conn in ready: job = next(job for job in running if job.conn == conn)