Skip to content

Isolated robot-fixture tests execute in marker order - #299

Open
MikeStitt wants to merge 1 commit into
robotpy:mainfrom
MikeStitt:marker-order-on-main
Open

Isolated robot-fixture tests execute in marker order#299
MikeStitt wants to merge 1 commit into
robotpy:mainfrom
MikeStitt:marker-order-on-main

Conversation

@MikeStitt

Copy link
Copy Markdown
Collaborator

Forward-port of pyfrc#257 to the testing code's new
home in robotpy-wpilib, per comment there:

pyfrc is being archived and testing stuff is in robotpy-wpilib in mostrobotpy starting in 2027.
Will try to bring this along soon.

pyfrc#257 was itself a more robust replacement for pyfrc#256.

What this fixes

Under IsolatedTestsPlugin, tests carrying @pytest.mark.order now run in the order pytest-order
sorted them. Concretely, robot-fixture tests are correctly sequenced:

  • relative to each other, and
  • relative to pytest module-, class-, and function-ordered tests.

Unordered tests keep running isolated and in parallel exactly as before.

Problem

IsolatedTestsPlugin.pytest_runtestloop hoists every robot-fixture test to the front and defers
the rest:

for item in session.items:
    if "robot" not in item.fixturenames:
        deferred.append(item)      # <-- runs later
        continue
    running.append(self._start_isolated_test(item))

This discards collection order. So @pytest.mark.order has no effect under this plugin, even
though pytest-order sorted session.items correctly during collection. Ordering-dependent tests
silently 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.items in 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, so
pytest-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, but
it 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 order marker.
An order marker on a class or module is inherited by every test underneath it. pytest-order
documents that this does not constrain the interior:

all tests in this class will be handled as having the same ordinal marker, e.g. the class as a
whole will be reordered without changing the test order inside the test class

So draining around each such test would serialize an entire marked module for no correctness
benefit. get_closest_marker returns the same Mark object to every test that inherits it,
which means comparing by identity groups them naturally:

order_group_marker = item.get_closest_marker("order")
if order_group_marker is not prev_order_group_marker:
    while running:
        self._wait_for_jobs(running, session)
prev_order_group_marker = order_group_marker

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 stay
serialized. Only the inherited case is relaxed, which is the case documented as internally
unconstrained.

Changes

All under subprojects/robotpy-wpilib/:

File Change
wpilib/testing/pytest_isolated_tests_plugin.py order-aware run loop, -p no:order for the subprocess
pyproject.toml add pytest-order dependency
tests/requirements.txt add pytest-order
tests/test_pytest_plugins.py 41 new tests

Tests

test_pytest_plugins.py goes from 19 tests to 60. These six state the claim directly:

Test Asserts
test_robot_tests_ordered_relative_to_each_other two ordered robot tests run in sequence
test_module_level_order_vs_robot_tests robot test sequenced against a module-ordered test
test_class_level_order_vs_robot_tests robot test sequenced against a class-ordered test
test_module_level_relative_order_vs_robot_tests same, using after= rather than an ordinal
test_class_level_relative_order_vs_robot_tests same, using after="TestName"
test_function_marker_inside_marked_class a method's own marker overrides its class's, forming a group

Each 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_sequencing cases
run 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-clock
time, and that a marked group's interior is not needlessly serialized.

Full robotpy-wpilib suite via tests/run_tests.py, from a source build on macOS and Python
3.14:

============= 242 passed, 8 skipped, 1 xpassed in 66.66s (0:01:06) =============

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.0 now makes the writer sleep whenever its reader is a robot test. This
is applied selectively, because a plain reader observes the violation directly and needs no
delay. Here is the result across the 30 permutations:

before after
cases that pass without the fix 9 to 11, varying 3, deterministic
cases that are genuine guards 17 27

The 3 remaining ones are the PLAIN-PLAIN-None variants. They contain 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 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:

  • Cost. The delays take the suite from 38 s to 67 s.
  • Slow machines. If a robot subprocess ever takes more than a second to reach its assertion,
    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 TimedRobot
plus whatever robotpy add-tests generates. 3 runs per configuration, median wall-clock:

-j current main this PR
8 0.48s 0.50s
1 1.03s 0.99s

These are identical within noise, and the run-to-run spread was 0.01s. -j 8 is still about
twice as fast as -j 1, which shows the concurrency is intact rather than merely fast enough to
hide a regression.

This is what the implementation predicts. The default tests carry no order marker, so
get_closest_marker("order") returns None for every item. The boundary condition
order_group_marker is not prev_order_group_marker never fires, and no drain is ever inserted. A
suite with no ordering pays nothing.

To reproduce:

mkdir demo && cd demo
printf 'import wpilib\n\n\nclass MyRobot(wpilib.TimedRobot):\n    pass\n' > robot.py
python -m robotpy add-tests
time python -m robotpy test -j 8 -- -q
time python -m robotpy test -j 1 -- -q

Behaviour across isolation and job settings

The order-demo.zip
project has 19 tests. Here it is run under each mode against both
versions:

Mode current main this PR
--no-isolation 19 passed 19 passed
-j 1 4 failed, 15 pass 19 passed
-j default (15) 5 failed, 14 pass 19 passed

Three things this establishes.

--no-isolation is unaffected. That path uses RobotTestingPlugin and pytest's own run
loop, 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 robot
subprocess exists at a time. Turning down -j is not a workaround.

-j 1 does mask one case, and it is worth being precise about which one. The failure that
goes missing at -j 1 is the robot to robot chain,
test_ordered_robot.py::test_robot_second. In the current loop,
while len(running) >= self._parallelism with parallelism == 1 forces each robot subprocess to
finish 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 mixed
robot 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.zip

Attached is a complete minimal robot project, an empty TimedRobot plus a tests/ directory,
that demonstrates the bug and the fix through the real robotpy test entry point.

unzip order-demo.zip && cd order-demo
python -m robotpy test -- -q
python check_parallel.py

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:

...................                                                      [100%]
19 passed

distinct pids: 3 of 3 -> isolated
overlapping pairs: [('par_a','par_b'), ('par_a','par_c'), ('par_b','par_c')] -> PARALLEL

Without it, 5 failed and 14 passed, which is one failure per ordering style:

FAILED tests/test_ordered_robot.py::test_robot_second
FAILED tests/test_ordered_function.py::test_fn_second
FAILED tests/test_ordered_function.py::test_fn_third
FAILED tests/test_ordered_class.py::TestClassSecond::test_reads_sentinel
FAILED tests/test_ordered_module_b.py::test_module_b_robot

The demo's tests/robot_test.py is exactly what robotpy add-tests generates, and
check_parallel.py confirms 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.py also asserts that a sentinel does not already exist before writing it, which
turns 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.

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
auscompgeek self-requested a review August 2, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant