Add: tool-smoke gate inside each DFX scene test#771
Merged
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a smoke testing utility in simpler_setup/tools/_smoke.py designed to verify that DFX capture pipeline tools can successfully parse schemas. These smoke tests have been integrated into several scene tests, including those for deps_to_graph, swimlane_converter, and dump_viewer. The reviewer suggested improving the robustness of the run_tool function by explicitly handling subprocess.TimeoutExpired to capture output during timeouts, and recommended removing Path from the __all__ export to maintain a cleaner public API.
Each DFX capture pipeline (dep_gen / l2_swimlane / tensor_dump) ships
with a consumer script under simpler_setup/tools/. The scene test for
that pipeline now invokes the consumer against the artifact it just
produced, asserting exit code 0. If a future schema change breaks the
tool, the failure attributes to the same CI step that captured the
artifact rather than surfacing later as a silent tooling rot.
Smoke is exit-code-only — HTML / PDF / diagram content is NOT validated.
The contract is "does the tool still parse this schema", not "is the
rendered output correct".
Wiring
- simpler_setup/tools/_smoke.py: run_tool + has_binary helpers shared
by all DFX tests.
- tests/.../dfx/dep_gen/test_dep_gen.py: deps_to_graph smoked in both
default and --show-tensor-info modes (guarded by has_binary("dot")
so dev machines without graphviz skip cleanly).
- tests/.../dfx/l2_swimlane/test_l2_swimlane.py: swimlane_converter
smoked against l2_perf_records.json.
- tests/.../dfx/tensor_dump/test_tensor_dump.py: dump_viewer smoked
against the captured tensor_dump/ directory.
- pmu has no consumer tool; no smoke added (raw csv is the artifact).
sched_overhead_analysis is intentionally NOT smoked — it requires a
real device log and would false-positive on sim. Reserve for a future
hardware DFX smoke.
CI: graphviz installed on the github-hosted sim runners (both Linux
and macOS) so deps_to_graph can render. The self-hosted onboard a2a3
runner warns if graphviz is missing instead of failing — the runner
admin should install it for full coverage, otherwise the has_binary
guard skips that one smoke.
Issue: follow-up to hw-native-sys#769
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Each DFX capture pipeline (
dep_gen,l2_swimlane,tensor_dump) ships with a consumer script undersimpler_setup/tools/. The corresponding scene test now invokes the consumer against the artifact it just produced and asserts exit code 0. A future schema change that breaks a tool fails the same CI step that captured the artifact — no silent tooling rot.pmu.csv.sched_overhead_analysisis intentionally skipped — it needs a real device log and would false-positive on sim. Reserve for a future hardware DFX smoke.Wiring
Each test calls the consumer via a plain
subprocess.run([sys.executable, "-m", "simpler_setup.tools.<name>", artifact, ...], check=True, timeout=60)— stdlib only, no helper module:tests/.../dfx/dep_gen/test_dep_gen.py— smokesdeps_to_graphin both default and--show-tensor-infomodes, guarded byshutil.which("dot")so dev machines without graphviz skip cleanly.tests/.../dfx/l2_swimlane/test_l2_swimlane.py— smokesswimlane_converteragainstl2_perf_records.json.tests/.../dfx/tensor_dump/test_tensor_dump.py— smokesdump_vieweragainst the capturedtensor_dump/directory..github/workflows/ci.yml— installs graphviz on the github-hosted sim runners (Linux apt + macOS brew); the self-hosted onboard a2a3 runner emits a::warning::ifdotis missing, then theshutil.whichguard skips that one smoke without failing CI.Test plan
pytest --platform a2a3simon macOS arm64 with corresponding--enable-*flags_smoke_deps.html,_smoke_deps_with_tensors.html,_smoke_swimlane.jsonproduced (presence verified)dump_viewersmoke runs (test passes with the call in place)Follow-up to #769.