[tutorials] Remove dead test-dependency variables - #23019
Conversation
Test Results 23 files 23 suites 3d 15h 44m 4s ⏱️ Results for commit 2501c02. ♻️ This comment has been updated with latest results. |
Four `<tname>-depends` variables in tutorials/CMakeLists.txt are never
read by CMake, so the test ordering they describe is not applied.
The variables are consulted only as `${${tname}-depends}`, where `tname`
is the tutorial path with its extension stripped and `/` replaced by
`-`. A variable therefore has an effect only if a tutorial file resolves
to its name.
- `benchmarks-depends`: the file is `legacy/benchmarks.C`, so its tname
is `legacy-benchmarks`. It is also listed in `extra_veto`, so no test
is generated for it at all. Two of its entries are stale on top of
that: `tutorial-io-tree-tree120_ntuple.C` carries a stray `.C`, and
`tutorial-io-tree-spider` names a tutorial that no longer exists.
- `pyroot-benchmarks-depends`: `tutorials/pyroot/` was dissolved when
the pyroot tutorials were moved, and no `pyroot/benchmarks.py`
remains.
- `analysis-unfold-testUnfold7d-depends`: the unfold series stops at
`testUnfold7c.C`; there is no `testUnfold7d.C`. The parallel
`testUnfold5d` variable is kept, because that tutorial does exist.
- `machine_learning-TMVA_SOFIE_RDataFrame-py-depends`: `tname` never
ends in `-py`, because the `-py` suffix is appended to `tutorial_name`
and not to `tname`, and no tutorial file is named `*-py.py`. The
ordering it asks for is already in place: the Python tutorial reads
`machine_learning-TMVA_SOFIE_RDataFrame-depends`, which is set
unconditionally a few hundred lines earlier and already names the
producer with its `-py` suffix. Removing the unread variable does
not change the dependency the test ends up with.
No test loses a dependency it was actually getting.
dd9e477 to
2501c02
Compare
|
Thanks for approving the workflow. The run went red on 11 Linux jobs, but the failures are stale-build-tree errors, not something this diff can reach. Flagging the evidence rather than leaving it unexplained. The run was queued on 5 August and only executed on 9 August, so it restored a four-day-old incremental build tree. Two failure signatures, both from that tree:
Neither is reachable from this change. The diff removes four To get a run from a fresh artifact, I have rebased onto current AI disclosure: the log analysis above and the wording of this comment were AI-assisted (Claude Code). The job logs and the diff were read before posting, and I take responsibility for the claims. |
Four
<tname>-dependsvariables intutorials/CMakeLists.txtare never read, so the test ordering they describe is not applied. This removes them.How the variables are consumed
They are read only as
${${tname}-depends}, at four places (lines 758, 789, 1059, 1060).tnameis the tutorial path with its extension stripped and/replaced by-:So a
<x>-dependsvariable has an effect only if some tutorial file resolves to tname<x>.The four
benchmarks-depends(line 614) — the file islegacy/benchmarks.C, so its tname islegacy-benchmarks, notbenchmarks. It is also listed inextra_veto("These ones are disabled !!!"), so no test is generated for it under either name. Two of its entries are independently stale:tutorial-io-tree-tree120_ntuple.Ccarries a stray.Cthat no test name has, andtutorial-io-tree-spidernames a tutorial that is no longer in the tree. Introduced in c43f365 (2014), when the file wastutorials/benchmarks.C.pyroot-benchmarks-depends(line 887) —tutorials/pyroot/no longer exists; the directory was dissolved in #17208 and there is nopyroot/benchmarks.py.analysis-unfold-testUnfold7d-depends(line 598) — the unfold series stops attestUnfold7c.C; there is notestUnfold7d.C. Note the neighbouringanalysis-unfold-testUnfold5d-dependsis kept, becausetestUnfold5d.Cdoes exist — the asymmetry is real, not a typo.machine_learning-TMVA_SOFIE_RDataFrame-py-depends(line 859) — this is the only-py-dependsin the file.tnamenever ends in-py: the suffix is appended totutorial_name, not totname, and no tutorial file is named*-py.py. The dependency it asks for is nevertheless already in effect, because the Python tutorial readsmachine_learning-TMVA_SOFIE_RDataFrame-depends, set unconditionally at line 665 totutorial-machine_learning-TMVA_SOFIE_PyTorch_HiggsModel-py— already correctly suffixed.I checked that last one against CMake rather than trusting the reading, with a minimal project reproducing the idiom verbatim:
The ordering matters here —
TMVA_SOFIE_RDataFrame.pyraisesFileNotFoundErrorifHiggsModel.onnxis missing — and it is retained by line 665 after this change.One thing worth flagging separately, which I have not changed: that ordering survives only because
TMVA_SOFIE_RDataFrame.CandTMVA_SOFIE_RDataFrame.pycollapse to the same tname and therefore share one-dependsvariable. If the C++ tutorial is ever removed, line 665 would likely go with it and the Python test would lose its dependency silently. Happy to follow up on that if you think it is worth guarding.What I did not change
I resolved every
<tname>-dependsvariable in the file against the tutorials on disk. The only other candidate the sweep produced wastutorial-machine_learning-TMVA_SOFIE_GNN_Parserinsidemachine_learning-TMVA_SOFIE_GNN_Application-depends, which is correct: that test does not come from the glob but is added explicitly at line 712 with exactly that name. It is left alone.After the change the same sweep reports no
-dependsvariable whose owner is missing, and no list entry naming a nonexistent test other than the explicitly-added one above.Checks
if/endifcounts drop by exactly one each (the removedif (ROOT_TORCH_FOUND AND ROOT_ONNX_FOUND)block) and parentheses stay balanced;foreach/endforeachare untouched.+0/-21, one file.masterso it applies fast-forward.No test loses a dependency it was actually getting.
AI disclosure
AI-assisted (Claude Code). The tool was used to run the sweep described above, to build the minimal CMake reproduction, and to draft this description. The findings were checked before opening: the consumption sites and tname derivation were read in the file, each missing tutorial was confirmed absent from the tree, the blame commits were looked up, and the one false positive was identified and excluded. I have reviewed and understood the change and take responsibility for it.