Skip to content

Regenerate .test_durations, which was blind to a whole module - #140

Closed
amburger66 wants to merge 2 commits into
masterfrom
regenerate-test-durations
Closed

Regenerate .test_durations, which was blind to a whole module#140
amburger66 wants to merge 2 commits into
masterfrom
regenerate-test-durations

Conversation

@amburger66

Copy link
Copy Markdown

Regenerates .test_durations, which CI uses to balance its eight shards and which had drifted badly out of date.

Why

.github/workflows/predicators.yml:31 runs:

pytest -s tests/ --splits 8 --group ${{ matrix.group }} --splitting-algorithm least_duration

least_duration bin-packs tests into groups using the committed .test_durations. The file in master was generated on 6 August and holds 4707 entries — with zero for anything under code_sim_learning, a module that arrived with #135. pytest-split assigns unlisted tests a fallback duration, so a block of ~33 identically-estimated tests is exactly the input that makes bin-packing swing.

The visible consequence is that adding any test reshuffles which tests share a shard process, and that regrouping has been surfacing a pre-existing fragility in the NSRT/GNN learning path — dill raising TypeError: cannot pickle '_abc._abc_data' object inside nsrt_learning_approach._learn_nsrts.

Measured on a PR that adds two tests, comparing its shard 8 against master's:

  • master shard 8: 910 tests across 86 file-lines
  • that PR's shard 8: 913 tests across 89 file-lines
  • 95 of ~175 file-lines differ in test count

So test_gnn_action_policy_approach runs in shard 8 in both, but with a substantially different set of tests preceding it in the same process. Because the split is deterministic given (test set, .test_durations), the same regrouping recurs every run — which is why that shard failed twice identically rather than flapping.

Note the file-level view is misleading here: shard 8's file lists are byte-identical between the two. pytest-split splits at test granularity, so identical file lists prove nothing. Count tests.

What this does and does not fix

Does: makes the split reflect the actual test set, so a PR adding tests no longer perturbs grouping out of proportion to what it changed.

Does not: fix the dill fragility. This re-rolls the grouping; it does not make the NSRT/GNN path robust to what precedes it in a process. That is a real bug in master and is worth its own investigation — the reproducible handle is CI's shard 8.

Caveats worth reviewing

  • These durations come from a developer machine, not a CI runner. least_duration cares about relative weights and machine speed differences are broadly uniform, so the balance should transfer — but anything with a different CPU/GPU profile locally than on the runner may be mis-weighted.
  • This reshuffles groups once for everyone when it lands. That is the point, but it means the first CI run after merge may surface the fragility above in a new place before settling.
  • Generated from a full single-process green run at 1d809f3, so no duration here is the time a test took to fail.

Unrelated, and still unexplained

There is a separate local symptom that this does not explain and that should not be conflated with it: the full suite run in one process, at a fixed commit, with identical execution order and PYTHONHASHSEED=0 pinned, has produced 9 failures on one run and 0 on the next. Execution order, pytest-randomly, hash randomisation and git hooks are all ruled out as mechanisms. That one is open.

CI balances its eight shards with pytest-split's least_duration algorithm over
the committed .test_durations (predicators.yml:31). That file was generated on
6 August and carried 4707 entries with ZERO for anything under
code_sim_learning -- a module that arrived with #135. pytest-split gives an
unlisted test a fallback duration, so a block of ~33 identically-estimated
tests is exactly the input that makes bin-packing swing.

The consequence is that adding any test reshuffles which tests share a shard
PROCESS, and that regrouping has been surfacing a pre-existing fragility in the
NSRT/GNN path: dill raising "cannot pickle '_abc._abc_data'" inside
nsrt_learning_approach._learn_nsrts. Measured on a PR adding two tests, its
shard 8 held 913 tests against master's 910, with 95 of ~175 file-lines
differing in test count -- so test_gnn_action_policy_approach ran in shard 8 in
both, behind a substantially different set of tests. The split is deterministic
given (test set, .test_durations), which is why that shard failed twice
identically rather than flapping.

Worth knowing for anyone checking this: shard 8's FILE lists are byte-identical
between the two. pytest-split splits at test granularity, so identical file
lists prove nothing. Count tests.

Now 5641 entries, 153 of them under code_sim_learning.

Generated from a full single-process GREEN run at 1d809f3 (1574 passed), and
that mattered: the first attempt came back with the 9-failure cluster, and a
test that fails fast records the time it took to FAIL. test_oracle_samplers
logged 0.369 s on that run against 2.252 s on the green one, which would have
under-weighted it in every future split -- baking the problem in while claiming
to fix it.

Two things this does NOT do. It does not fix the dill fragility; it re-rolls
the grouping, and the arrangement it lands on may or may not be green. And the
durations come from a developer machine rather than a CI runner: least_duration
uses relative weights and machine differences are broadly uniform, so the
balance should transfer, but anything with a different CPU/GPU profile locally
than on the runner may be mis-weighted.
@amburger66 amburger66 self-assigned this Aug 18, 2026
@amburger66
amburger66 marked this pull request as ready for review August 18, 2026 16:02
@yichao-liang

Copy link
Copy Markdown
Collaborator

ok I'll trust you on this :D

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.

2 participants