Repository navigation
CI: move redundant scene tests to daily sweep - #1790
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds platform-aware manual test selection for standalone pytest tests and scene-test cases. It propagates ChangesManual test coverage
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant pytest
participant conftest
participant scene_test
participant CI workflow
pytest->>conftest: Read --manual
conftest->>scene_test: Match manual case to platform
scene_test-->>conftest: Return selection status
conftest->>pytest: Filter tests and resource jobs
CI workflow->>pytest: Pass manual_mode as --manual
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/_st-npu-a2a3.yml:
- Around line 27-31: Validate manual_mode via an environment variable and a
shell case statement accepting only exclude, include, or only before any command
uses it. In .github/workflows/_st-npu-a2a3.yml ranges 27-31 and 78-90,
.github/workflows/_st-npu-a5.yml ranges 22-26 and 65-69,
.github/workflows/_st-sim-a2a3.yml ranges 26-30 and 105-107, and
.github/workflows/_st-sim-a5.yml ranges 26-30 and 105-107, replace direct input
interpolation in every pytest and task-submit --run command with the validated
variable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 83e14adb-d084-4bbc-8faf-727c1af9a0d0
📒 Files selected for processing (32)
.claude/skills/testing/SKILL.md.github/workflows/_st-npu-a2a3.yml.github/workflows/_st-npu-a5.yml.github/workflows/_st-sim-a2a3.yml.github/workflows/_st-sim-a5.yml.github/workflows/daily.ymlconftest.pydocs/ci.mddocs/testing.mddocs/user/reference/cli.mdexamples/a2a3/tensormap_and_ringbuffer/README.mdexamples/a2a3/tensormap_and_ringbuffer/benchmark_bgemm/README.mdexamples/a2a3/tensormap_and_ringbuffer/benchmark_bgemm/test_benchmark_bgemm.pyexamples/a2a3/tensormap_and_ringbuffer/vector_example/test_vector_example.pyexamples/a5/tensormap_and_ringbuffer/vector_example/test_vector_example.pyexamples/workers/l2/hello_worker/test_hello_worker.pyexamples/workers/l2/vector_add/test_run_timing.pyexamples/workers/l2/vector_add/test_vector_add.pysimpler_setup/scene_test.pytests/st/a2a3/host_build_graph/graph_execution/test_graph_execution.pytests/st/a2a3/host_build_graph/matmul/test_matmul.pytests/st/a2a3/host_build_graph/vector_example/test_vector_example.pytests/st/a2a3/tensormap_and_ringbuffer/dummy_task/test_dummy_task.pytests/st/a2a3/tensormap_and_ringbuffer/fanin_lookup_perf/test_fanin_lookup_perf.pytests/st/a5/host_build_graph/vector_example/test_vector_example.pytests/st/a5/tensormap_and_ringbuffer/dummy_task/test_dummy_task.pytests/st/a5/tensormap_and_ringbuffer/fanin_lookup_perf/test_fanin_lookup_perf.pytests/st/task_timing/task_timing_slots/test_task_timing_e2e.pytests/st/worker/collectives/allgather/test_allgather.pytests/st/worker/collectives/allreduce/test_allreduce.pytests/st/worker/collectives/broadcast/test_broadcast.pytests/st/worker/collectives/reduce_scatter/test_reduce_scatter.py
1282e85 to
d2bc9ca
Compare
|
Overall LGTM — 机制干净,PR body 的移除清单与实际标记逐条对得上,与 ci-change-detection 规则(schedule 放独立 1. 新判定逻辑缺少单元测试 (Should-fix)PR 描述里引用的
目前没有任何直接的单测覆盖。而这几处恰恰有不少非平凡分支值得钉住:
建议补一个针对这三个 helper 的 UT(尤其是错误分支),这样后续改动能有回归护栏。 2.
|
- Add a scheduled full scene-test workflow for Sim, Onboard, and Pod lanes - Extend manual selection to standalone tests and platform-scoped cases - Validate reusable manual-mode inputs before shell execution - Move redundant single-card, timing, and P4 communication coverage while retaining representative Per-PR paths - Cover manual selector branches and document session-wide only semantics - Keep existing case parameters, kernels, golden checks, and timeout thresholds unchanged
d2bc9ca to
6335147
Compare
2 participants
Summary
timing/DFX-observation, P4 collective, and selected duplicate Sim cases
behind the existing manual selection.
--manual excludeas the reusable workflow default for PR CI; theseparate Daily workflow uses
--manual includeto retain the full corpus.markers, and pass the same mode to pre-compilation so only selected test
classes are compiled.
manual_modeasexclude,include, oronlybefore using it inshell commands.
and workflow timeout thresholds unchanged.
Per-PR coverage policy
Cases are moved out of Per-PR when their value is primarily shallow smoke,
rank/shape scale expansion, performance/marker observation, or a duplicate
execution path with a stronger representative still running. Per-PR retains
production-model paths, fault/recovery/stress coverage, large pressure cases,
and representative algorithm coverage on each supported platform.
Paged-attention shapes are unchanged, and A2/A3 Sim still retains the
500-task BGEMM pressure case.
Cases removed from Per-PR
Single-card and observability cases
test_hello_workertest_vector_addTestMatmulHostBuildGraph::defaultTestFaninLookupPerf64x64 casesTestDummyTask::LongDummyChainCase0remains on Sim; BGEMM 64 remains Onboardffn_tp_paralleldual_domain_overlapdomain_rank_mapremains on both Sim lanes; the full overlap computation remains A3 OnboardCollective cases
A5 Onboard is unchanged by the P4 migration because the P4 classes are not
defined for that platform. Daily continues to run every migrated P2 and P4
case on its supported platforms.
CI compatibility fixes
so it explicitly passes
--manual include; otherwise pytest deselects theonly target and exits non-zero.
manual_modethrough anenvironment variable before shell parsing, then reuse only the validated
value in pytest, pre-compile, and
task-submit --runcommands.macOS serial-build tail that previously exhausted the 20-minute job timeout.
Validation
additional P2 cases are manual only on the two Sim platforms.
python -m pytest tests/ut/py/test_scene_level_selection.py -q: 8 passed.repository policy files.
git diff --check upstream/main...HEAD: passed.