Delete runners/test_dsv41flash_b300.py - #3273
Conversation
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟡
runners/test_dsv41flash_b300.py— This PR deletes runners/test_dsv41flash_b300.py outright, leaving benchmarks/single_node/agentic/dsv41flash_fp4_b300_vllm_mtp.sh's capture-tier branching (TP/CONC thresholds for --max-cudagraph-capture-size, --max-num-batched-tokens, --compilation-config, --gpu-memory-utilization) with zero test coverage. No other test in runners/ or elsewhere references this script. AGENTS.md's Test quality section only allows deleting a test without replacement if it fails the four kept-test questions (structure-only, config-pinning, tautological, duplicate) — this test ran the real script via subprocess with stubbed binaries and asserted on computed vllm args, which is exactly the pattern the guide requires ('Shell scripts are tested by running them with stubbed binaries on PATH and checking what they…Extended reasoning...
…produced'). Fix: restore equivalent behavior coverage for the three capture/concurrency tiers in dsv41flash_fp4_b300_vllm_mtp.sh, or justify why the branch logic no longer needs a regression test.
The deleted test parametrized (tp, concurrency) over (4,1), (4,8), (2,128) and asserted the script's if/elif/else in dsv41flash_fp4_b300_vllm_mtp.sh (lines ~33-40) picks the right GRAPH_SIZES tier, MAX_BATCHED_TOKENS, and GPU_MEMORY_UTILIZATION. After this deletion, a future edit that inverts the CONC<=4 or CONC>=128&&TP==2 condition, or breaks CAPTURE_SIZE derivation from GRAPH_SIZES, or drops --gpu-memory-utilization forwarding, passes CI silently. Nothing else in the repo runs this script or checks its emitted vllm CLI args (grep for dsv41flash_fp4_b300_vllm_mtp and for test_dsv41flash_b300 across the repo returns no other hits). The consequence is a benchmark launcher regression reaching production runs undetected, since docs/testing.md's CI auto-discovery only catches removed tests, not silently-broken shell logic.
Verification: nit. The deleted runners/test_dsv41flash_b300.py was a legitimate behavior test, exactly the kind AGENTS.md line 90 says to keep ("Shell scripts are tested by running them with stubbed binaries on PATH and checking what they produced, not by reading their text"). It sourced the real benchmarks/single_node/agentic/dsv41flash_fp4_b300_vllm_mtp.sh with stubbed source/hf/nvidia-smi/vllm/etc.,…
No description provided.