From 12f6eac80c69511a3028fda9e85d14c95cb65c63 Mon Sep 17 00:00:00 2001 From: Mike Stitt Date: Wed, 29 Jul 2026 22:55:22 -0400 Subject: [PATCH 1/4] Run isolated 'robotpy test' tests in pytest marker order IsolatedTestsPlugin pulled every isolated robot-fixture test ahead of the rest, which discarded the order pytest-order established during collection. Tests using @pytest.mark.order silently ran in the wrong sequence. The run loop now walks session.items in order and drains in-flight subprocesses at ordering boundaries. Unordered tests are untouched and still run isolated and in parallel. - Group tests by order-marker identity, so a class or module marker serialises the group against everything outside it without serialising its interior - Pass -p no:order to the isolated subprocess, which otherwise warns about markers referencing tests it cannot see - Add pytest-order to dependencies and tests/requirements.txt --- subprojects/robotpy-wpilib/pyproject.toml | 1 + .../robotpy-wpilib/tests/requirements.txt | 1 + .../tests/test_pytest_plugins.py | 533 ++++++++++++++++++ .../testing/pytest_isolated_tests_plugin.py | 84 ++- 4 files changed, 600 insertions(+), 19 deletions(-) diff --git a/subprojects/robotpy-wpilib/pyproject.toml b/subprojects/robotpy-wpilib/pyproject.toml index 2677094e5..d232124c6 100644 --- a/subprojects/robotpy-wpilib/pyproject.toml +++ b/subprojects/robotpy-wpilib/pyproject.toml @@ -31,6 +31,7 @@ dependencies = [ # For running robot tests "pytest>=3.9", + "pytest-order", "pytest-reraise", ] 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..d0f50512d 100644 --- a/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py +++ b/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py @@ -432,3 +432,536 @@ 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( + "a_fixture, b_fixture", + [("robot", "robot"), ("robot", "")], + ids=["RR", "RN"], +) +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 when the first test uses the robot fixture (starts async subprocess, + allowing the second test to run concurrently). + + NR is omitted: a non-robot test runs in-process synchronously, so it + completes before the subsequent robot subprocess starts -- serial by design. + """ + _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..e03852159 100644 --- a/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py +++ b/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py @@ -149,7 +149,24 @@ 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. + "-p", + "no:order", + *config_args, + ], plugins=[plugin, worker_plugin], ) @@ -237,28 +254,57 @@ 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: + # Run tests in the order that they are given to us, running robot fixture tests in a + # subprocess, while preserving order marker boundaries. + # + # 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. + # + # 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. Comparing + # order group markers 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 -- still run in parallel. 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. + prev_order_group_marker = None + for idx, item in enumerate(session.items): assert isinstance(item, pytest.Function) - if "robot" not in item.fixturenames: - deferred.append(item) - continue - while len(running) >= self._parallelism: - self._wait_for_jobs(running, session) + order_group_marker = item.get_closest_marker("order") + + if order_group_marker is not prev_order_group_marker: + # 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. Two unmarked tests in a row are + # both None, so they do not trigger a drain. + while running: + self._wait_for_jobs(running, session) + prev_order_group_marker = order_group_marker + + if "robot" in item.fixturenames: + # This test uses the "robot" fixture, run it in an available, isolated subprocess. + while len(running) >= self._parallelism: + self._wait_for_jobs(running, session) + + running.append(self._start_isolated_test(item)) + else: + # This test runs in this process. Only pass nextitem as a teardown-optimization + # hint when the next test also runs in this process -- otherwise the next item is + # handled by a subprocess and this one must tear down fully. + nextitem = ( + session.items[idx + 1] if idx + 1 < len(session.items) else None + ) + if nextitem is not None and "robot" in nextitem.fixturenames: + nextitem = None + + session.config.hook.pytest_runtest_protocol( + item=item, nextitem=nextitem + ) - 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) while running: From dbbd00ba1e6a88dd161be63e5779af4e990e250c Mon Sep 17 00:00:00 2001 From: Mike Stitt Date: Mon, 3 Aug 2026 12:22:02 -0400 Subject: [PATCH 2/4] Register the order marker in isolated test subprocesses Passing "-p no:order" to the worker unloads pytest-order entirely, including its registration of the "order" marker. The @pytest.mark.order decorators are still on the tests the worker collects, so the marker became unknown there. Projects that escalate unknown markers therefore could not run ordered robot tests at all: --strict-markers made it a collection error, and filterwarnings=error promoted the PytestUnknownMarkWarning into one. Both settings reach the worker, which inherits the parent's command line and chdirs to the project root before reading the ini. The worker died during its own collection, so the parent reported "no tests ran" with no indication of why. WorkerPlugin now re-registers the marker in pytest_configure. This restores only the registration that --strict-markers and the unknown-mark check consult; it does not reload pytest-order, so the reordering stays off in the worker where a single collected test cannot satisfy a relative marker anyway. - Register "order" from WorkerPlugin.pytest_configure, which runs before collection in the isolated subprocess - Note at the "-p no:order" site that the two halves belong together - Add a regression test covering --strict-markers and filterwarnings=error, plus a relative-marker case that fails if the registration ever turns back into a plugin reload --- .../tests/test_pytest_plugins.py | 75 +++++++++++++++++++ .../testing/pytest_isolated_tests_plugin.py | 21 ++++++ 2 files changed, 96 insertions(+) diff --git a/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py b/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py index d0f50512d..936fed873 100644 --- a/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py +++ b/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py @@ -564,6 +564,81 @@ def params(t): 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", "")], 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 e03852159..b8c070780 100644 --- a/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py +++ b/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py @@ -82,6 +82,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 @@ -163,6 +181,9 @@ def _run_test( # 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, From 39cdec31916864b57f62994a38e17c7cbca69429 Mon Sep 17 00:00:00 2001 From: Mike Stitt Date: Mon, 3 Aug 2026 15:38:41 -0400 Subject: [PATCH 3/4] Overlap in-process tests with in-flight isolated subprocesses Walking session.items in collection order preserved ordering but gave up the overlap the previous run loop got for free. That loop deferred every non-robot test to the end, so isolated subprocesses were always started first and in-process tests ran while they were in flight. Following collection order means an in-process test collected before any robot test runs with nothing in flight, and its time is added to the run rather than hidden inside it. pytest-order does not constrain the order within a group, so the sequence inside one carries no meaning and the loop is free to schedule for throughput. It now treats each group as two queues: fill every subprocess slot, run in-process tests in the time those subprocesses take, and block only once there is nothing left to overlap with. Boundaries between groups are untouched and still drain. Reaping is a poll between in-process tests, or a wait when none are left. Slots are only released inside _wait_for_jobs, so a loop that only polled would spin once the in-process queue emptied, and one that only waited would hold the subprocess count at its high-water mark through a long run of in-process tests and refuse to start anything new. Measured with in-process tests collected ahead of four 1.5s robot tests at -j 4, each in-process test sleeping 1ms: tests before after 50 2.01s 1.95s 250 2.37s 1.95s 500 2.86s 1.94s 1000 3.74s 2.06s The old loop grows with the volume of in-process work. The new one is flat, because that work now fits inside the robot tests. Suites with no in-process tests, and suites with more robot tests than -j, measured unchanged. - Schedule each order marker group as an isolated queue and an in-process queue drained against each other - Add a timeout argument to _wait_for_jobs so finished jobs can be reaped without stalling this process - Extract _order_marker_groups, carrying the Mark-identity rationale with it - Add the NR permutation to test_unordered_tests_still_run_in_parallel; the case it documented as serial by design now overlaps, and it fails without this change --- .../tests/test_pytest_plugins.py | 17 +- .../testing/pytest_isolated_tests_plugin.py | 152 ++++++++++++------ 2 files changed, 113 insertions(+), 56 deletions(-) diff --git a/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py b/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py index 936fed873..04b153ce4 100644 --- a/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py +++ b/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py @@ -641,18 +641,23 @@ def test_first(robot): @pytest.mark.parametrize( "a_fixture, b_fixture", - [("robot", "robot"), ("robot", "")], - ids=["RR", "RN"], + [("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 when the first test uses the robot fixture (starts async subprocess, - allowing the second test to run concurrently). + time. - NR is omitted: a non-robot test runs in-process synchronously, so it - completes before the subsequent robot subprocess starts -- serial by design. + 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) 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 b8c070780..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 @@ -222,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 @@ -276,58 +312,59 @@ def pytest_runtestloop(self, session: pytest.Session) -> bool: running: list[IsolatedTestJob] = [] try: - # Run tests in the order that they are given to us, running robot fixture tests in a - # subprocess, while preserving order marker boundaries. + # 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. + # 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. # - # 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. Comparing - # order group markers 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 -- still run in parallel. 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. - prev_order_group_marker = None - for idx, item in enumerate(session.items): - assert isinstance(item, pytest.Function) - - order_group_marker = item.get_closest_marker("order") - - if order_group_marker is not prev_order_group_marker: - # 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. Two unmarked tests in a row are - # both None, so they do not trigger a drain. - while running: - self._wait_for_jobs(running, session) - prev_order_group_marker = order_group_marker - - if "robot" in item.fixturenames: - # This test uses the "robot" fixture, run it in an available, isolated subprocess. - while len(running) >= self._parallelism: + # 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) + + # 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) - running.append(self._start_isolated_test(item)) - else: - # This test runs in this process. Only pass nextitem as a teardown-optimization - # hint when the next test also runs in this process -- otherwise the next item is - # handled by a subprocess and this one must tear down fully. - nextitem = ( - session.items[idx + 1] if idx + 1 < len(session.items) else None - ) - if nextitem is not None and "robot" in nextitem.fixturenames: - nextitem = None - - session.config.hook.pytest_runtest_protocol( - item=item, nextitem=nextitem - ) - - self._maybe_raise(session) - while running: self._wait_for_jobs(running, session) finally: @@ -368,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) From f087bd7cdd55a502bd05bcfdf98fc559de873fc9 Mon Sep 17 00:00:00 2001 From: Mike Stitt Date: Mon, 3 Aug 2026 16:19:02 -0400 Subject: [PATCH 4/4] The robotpy-wpilib subproject does not require pytest-order The isolated test plugin reads order markers through get_closest_marker, which returns None for every test when the plugin is absent, and passes "-p no:order" to its subprocesses, which pytest ignores for a plugin name it does not know about. --- subprojects/robotpy-wpilib/pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/subprojects/robotpy-wpilib/pyproject.toml b/subprojects/robotpy-wpilib/pyproject.toml index d232124c6..2677094e5 100644 --- a/subprojects/robotpy-wpilib/pyproject.toml +++ b/subprojects/robotpy-wpilib/pyproject.toml @@ -31,7 +31,6 @@ dependencies = [ # For running robot tests "pytest>=3.9", - "pytest-order", "pytest-reraise", ]