Isolated robot-fixture tests execute in marker order - #299
Open
MikeStitt wants to merge 1 commit into
Open
Conversation
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
auscompgeek
self-requested a review
August 2, 2026 14:39
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Forward-port of pyfrc#257 to the testing code's new
home in
robotpy-wpilib, per comment there:pyfrc#257 was itself a more robust replacement for pyfrc#256.
What this fixes
Under
IsolatedTestsPlugin, tests carrying@pytest.mark.ordernow run in the order pytest-ordersorted them. Concretely, robot-fixture tests are correctly sequenced:
pytestmodule-, class-, and function-ordered tests.Unordered tests keep running isolated and in parallel exactly as before.
Problem
IsolatedTestsPlugin.pytest_runtestloophoists every robot-fixture test to the front and defersthe rest:
This discards collection order. So
@pytest.mark.orderhas no effect under this plugin, eventhough pytest-order sorted
session.itemscorrectly during collection. Ordering-dependent testssilently run in the wrong sequence.
The failure is specifically between robot-fixture tests and non-robot-fixture tests. A chain of
only non-robot-fixture tests still passes today, because the deferred list preserves their
relative order among themselves, and are not spawned into subprocesses.
This is why tests below mixes the two.
Fix
Walk
session.itemsin the pre-sorted order given, and drain in-flight subprocesses at ordering boundaries.Unordered robot-fixture tests still run isolated and still run in parallel. Only the boundaries serialize.
The subprocess also gets
-p no:order. It runs exactly one test and cannot see the others, sopytest-order would otherwise emit misleading warnings like
cannot execute 'test_b' relative to others: 'test_a' - ignoring the marker. That warning is accurate from inside the subprocess, butit is wrong about the outcome, because the parent did order them.
Design note: grouping by marker identity
This is a refinement over pyfrc#257, which drained around every item carrying an
ordermarker.An
ordermarker on a class or module is inherited by every test underneath it. pytest-orderdocuments that this does not constrain the interior:
So draining around each such test would serialize an entire marked module for no correctness
benefit.
get_closest_markerreturns the sameMarkobject to every test that inherits it,which means comparing by identity groups them naturally:
The group is serialized against everything outside it, and its members stay parallel. This also
folds pyfrc#257's separate pre-drain and post-drain into one. Leaving a group covers
before=,and entering one covers ordinals and
after=.The fix uses identity rather than
==to compare markers. This is deliberate and conservative.Two independent
@pytest.mark.order(5)decorators compare equal but are distinct constraints, so they stayserialized. Only the inherited case is relaxed, which is the case documented as internally
unconstrained.
Changes
All under
subprojects/robotpy-wpilib/:wpilib/testing/pytest_isolated_tests_plugin.py-p no:orderfor the subprocesspyproject.tomlpytest-orderdependencytests/requirements.txtpytest-ordertests/test_pytest_plugins.pyTests
test_pytest_plugins.pygoes from 19 tests to 60. These six state the claim directly:test_robot_tests_ordered_relative_to_each_othertest_module_level_order_vs_robot_teststest_class_level_order_vs_robot_teststest_module_level_relative_order_vs_robot_testsafter=rather than an ordinaltest_class_level_relative_order_vs_robot_testsafter="TestName"test_function_marker_inside_marked_classEach one is written to its file in reverse order, so passing also proves reordering occurred.
Each one fails against the current loop.
There is supporting coverage too. 30 parametrized
test_order_marker_enforces_sequencingcasesrun a first, middle, last chain across all 10 robot/plain permutations, times ordinal,
before=and
after=. There are also regression guards that unordered tests still overlap in wall-clocktime, and that a marked group's interior is not needlessly serialized.
Full
robotpy-wpilibsuite viatests/run_tests.py, from a source build on macOS and Python3.14:
Every ordering test was checked against the unfixed loop
A test that passes with and without the fix guards nothing, so each one was run against current
main. That exposed a real problem, which is now fixed.A sentinel written by a plain in-process test lands within milliseconds. A robot test needs a few
hundred milliseconds to spawn its subprocess and reach its first assertion. So whenever the
sentinel reader was a robot test, an unfixed run still found the sentinel already written, and it
passed without proving anything. Sweeping the delay against the unfixed plugin put the cutoff
between 150 ms and 200 ms on an M-series Mac. At 150 ms it caught the bug in 0 runs out of 5, and
at 200 ms it caught it in 5 out of 5.
ORDER_HANDOFF_DELAY = 1.0now makes the writer sleep whenever its reader is a robot test. Thisis applied selectively, because a plain reader observes the violation directly and needs no
delay. Here is the result across the 30 permutations:
The 3 remaining ones are the
PLAIN-PLAIN-Nonevariants. They contain no robot test at all, sothere is nothing for the plugin to mis-order. The old loop deferred non-robot tests but preserved
their order among themselves. These cases assert that the chain still works. They confirm two
ordered plain tests run in the right order. What they cannot do is catch this particular bug,
because the bug is robot tests jumping ahead of non-robot tests, and these cases have no robot test.
They pass the same way before and after the fix. Adding a delay to guard against race conditions
between spawned and not spawned tests is not needed because these tests are not spawned.
The test docstring says this, so nobody later mistakes them for broken guards.
Two caveats are recorded alongside the
ORDER_HANDOFF_DELAY:these cases quietly stop catching regressions. They never fail wrongly, because with ordering
support present they pass regardless of timing. That makes the degradation silent, which is the
dangerous direction. The measured cutoff is in the comment, so a future maintainer can
re-measure rather than guess.
pyfrc#257 used the same logic, minus the identity grouping, and was green across all 24 CI jobs
there.
The default robot tests keep their parallelism
This was measured on a project containing only the default tests, which is an empty
TimedRobotplus whatever
robotpy add-testsgenerates. 3 runs per configuration, median wall-clock:-jmainThese are identical within noise, and the run-to-run spread was 0.01s.
-j 8is still abouttwice as fast as
-j 1, which shows the concurrency is intact rather than merely fast enough tohide a regression.
This is what the implementation predicts. The default tests carry no
ordermarker, soget_closest_marker("order")returnsNonefor every item. The boundary conditionorder_group_marker is not prev_order_group_markernever fires, and no drain is ever inserted. Asuite with no ordering pays nothing.
To reproduce:
Behaviour across isolation and job settings
The order-demo.zip
project has 19 tests. Here it is run under each mode against both
versions:
main--no-isolation-j 1-jdefault (15)Three things this establishes.
--no-isolationis unaffected. That path usesRobotTestingPluginand pytest's own runloop, so it never reaches the code this PR changes. Ordering already works there, before and
after. The defect is specific to isolated mode.
The defect is not a concurrency race. It reproduces at
-j 1, where at most one robotsubprocess exists at a time. Turning down
-jis not a workaround.-j 1does mask one case, and it is worth being precise about which one. The failure thatgoes missing at
-j 1is the robot to robot chain,test_ordered_robot.py::test_robot_second. In the current loop,while len(running) >= self._parallelismwithparallelism == 1forces each robot subprocess tofinish before the next one starts. The hoisted robot tests are already in pytest-order's sorted
sequence, so serialising them accidentally yields the right order. Robot against robot ordering
therefore appears to work at
-j 1, and it breaks as soon as concurrency is allowed. The mixedrobot and non-robot chains fail at every
-j, because those break from the hoisting itself,which puts robot tests ahead of all non-robot tests regardless of the job limit.
Reproducing it yourself with
order-demo.zipAttached is a complete minimal robot project, an empty
TimedRobotplus atests/directory,that demonstrates the bug and the fix through the real
robotpy testentry point.The demo clears its own state at session start, so it can be run repeatedly, and in either order
against either version, without manual cleanup.
With this PR, 19 passed:
Without it, 5 failed and 14 passed, which is one failure per ordering style:
The demo's
tests/robot_test.pyis exactly whatrobotpy add-testsgenerates, andcheck_parallel.pyconfirms the default tests still run isolated and concurrently.Both figures above were reproduced three times each, in both orders, with no cleanup between
runs. A stale sentinel would make an unfixed run report fewer failures than it should. So
tests/_sentinel.pyalso asserts that a sentinel does not already exist before writing it, whichturns that into a loud error rather than a reassuring wrong answer.
There is one caveat baked into the demo. The first test of each chain sleeps 1.0s before writing
its sentinel. Without that delay the chains are races rather than deterministic failures, because
a robot subprocess takes a few hundred milliseconds to spawn and a fast in-process test often
wins. The chain then passes by luck. An earlier draft showed only 2 failures for exactly that
reason.