Repository navigation
Docs: select the SDMA tests by marker in the testing skill's recipe - #1620
Conversation
The skill tells the reader, three lines above the command block, that
quarantines "are markers on the tests, so mirror the sweep with `-m "not sdma"`
rather than copying a path list". The block then copied a path list:
pytest examples/a2a3/tensormap_and_ringbuffer/sdma_async_completion_demo \
examples/a2a3/tensormap_and_ringbuffer/prefetch_async_demo
That is the fragility the marker replaced. A demo directory that moves leaves
this command silently running less than it claims — `pytest` on a path that
does not exist exits 4, but on a path that still exists while a *third* SDMA
test appears elsewhere it simply misses it, and CI's `-m sdma` would not. The
recipe is now `pytest examples tests/st -m sdma`, the same corpus and the same
selector the workflow uses, so the two cannot diverge.
The device claim is also corrected in three places. "Their own devices" is true
only on aarch64, where the SDMA step takes `task-submit --device auto
--device-num 2`; the x86_64 branch has no `task-submit` and both steps use the
same `${DEVICE_RANGE}`. What holds on both is **ordering** — the SDMA step runs
after the sweep, so no fault-injection case can meet a device that has already
provisioned SDMA. The prose, the quarantine table's "runs instead in" cell, and
the command comment now say that instead of asserting a device disjointness
that half the runners do not provide.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe testing skill updates SDMA quarantine guidance. It documents marker-based exclusion from the general sweep and a dedicated post-sweep ChangesSDMA testing guidance
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Follow-up to #1609 and #1617 — the same two defects, in the file whose stated purpose is reproducing CI.
1. The recipe contradicted the instruction three lines above it. The skill says quarantines "are markers on the tests, so mirror the sweep with
-m "not sdma"rather than copying a path list" — and then the command block copied a path list:pytest examples/a2a3/tensormap_and_ringbuffer/sdma_async_completion_demo \ examples/a2a3/tensormap_and_ringbuffer/prefetch_async_demoThat is exactly the fragility #1609 removed from
ci.yml. It is nowpytest examples tests/st -m sdma— the same corpus and the same selector as the workflow'sSDMA_TESTS, so the recipe cannot drift from the job it mirrors.2. "Their own devices" is aarch64-only, in three places (prose, the quarantine table's "runs instead in" cell, and the command comment). Only the aarch64 branch gives the SDMA step
task-submit --device auto --device-num 2; x86_64 has notask-submitand both steps use${DEVICE_RANGE}. What holds on both is ordering — SDMA runs after the sweep, so no fault-injection case meets a device that has already provisioned SDMA. Same correction #1617 made todocs/ci.md.Testing
ci.ymlverbatim in corpus and selector: sweepexamples tests/st -m "not sdma", SDMA stepexamples tests/st -m sdma(ci.yml:627).claude/is inNON_CODE, so every gated job should reportskipping