Repository navigation
fix(tools): reject dangling test graph targets - #11668
Conversation
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
THEMANnt
left a comment
THEMANnt
left a comment
There was a problem hiding this comment.
Reviewed dcb1013c482aaeddc84ec8c366e35d987c807ee2. Approve: graph loading rejects a non-null test target with no closure, while null targets, explicit empty closures and empty graphs remain valid. The graph producer creates a closure for every selected executable target.
Windows/Python 3.11.9: all 38 selector tests passed, including temporary CMake/Ninja builds. The submitted graph-control test against disposable production from author merge base c831db72 fails because the old CLI exits 0 with an empty selection for a dangling target; the candidate reports the invalid graph instead. An initial control omitted an imported helper; after adding the unchanged baseline helper, it produced that behavioral result.
Hosted Windows, Linux and both macOS relinker jobs ran all 12 split selector registrations on merges containing this head; product jobs were skipped. This covers test selection and registration rather than a full product build.
|
Checked |
What
A test graph could refer to a non-null build target absent from its closure map. Selection then silently treated that missing target as an empty closure and could omit relevant tests. Reject the dangling target while loading the graph, identifying the test entry and target. Null targets, explicit empty closures and an empty graph remain valid.
Basis: the graph producer/consumer contract in
tools/select_tests.py: every non-null target consumed by selection needs a closure entry. Regressions exercise production loading and real--explainCLI selection, rather than duplicating the validator.The existing aggregate select_tests suite approached the 30-second limit under competing work. Preserve all 17 original SelectTests method bodies and distribute them across four classes, then register all 12 test classes separately without increasing a timeout. The original literal
add_test(NAME select_tests ...)registration remains; only that CTest entry setsANYPS5_SELECT_TEST_CLASS=SelectTests. Running the file normally without that environment variable still runs the whole suite. Other CTest entries pass explicit class names; CMake tracks the test source as a configure dependency.Integration: no prerequisite PR. The original registration anchors are retained to accommodate #11062. Recorded-head tool projections with #10619, #11486 and #11062 were tested below. Each preserves both sets of production behavior/tests; no merge ordering is required. Legacy shared-file comparisons are described below rather than asserted to be whole-PR merges.
Limits: this validates target-to-closure references, not every possible graph semantic error. Windows and Linux executed the registered groups; macOS and an older Python runtime remain untested.
Tested
Base:
c831db72df8d3236a910661c3fc37507fc646a33. Tested PR head:dcb1013c482aaeddc84ec8c366e35d987c807ee2.Windows: Python 3.14.3, MinGW GCC 15.2.0, CMake 4.3.1, Ninja. Ubuntu 26.04 under WSL: Python 3.14.4, GCC 15.2.0, CMake 4.2.3, Ninja; 8 logical CPUs.
Windows skips the existing non-ASCII ANSI-path case; Linux skips the Windows-only chunk-size case.
Configured with
cmake -S <source> -B <build> -G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=ON -DANYPS5_RELINKER_ONLY=ON. Ranctest --test-dir <build> -R '^select_tests($|_)' --parallel 4 -V --output-junit <report>on Windows,--parallel 8on WSL; XML skip-policy checks passed.CTest JSON listing proves one registration per test class, including the legacy entry environment. AST comparison proves all 17 original method bodies are unchanged. Test-registration audit: 196 scripts each registered or run by a workflow.
Negative control: replaying the final regression tests with the unmodified production tools from
c831db72df8d3236a910661c3fc37507fc646a33reproduces the target assertion failures. Missing imports/modules were not used as regression evidence. Current-main conventions,git diff --checkand Python 3.9 grammar parsing passed. An older Python runtime was not run.Seven-project local integration on the earlier recorded base
27ad7244d22153ebfd576e04e3c4d63e0b92264b: combined commite3fd69f3840131bc4300f109dfcd99dd157578f1; 19 registered groups passed on each platform (251 unittest cases reported; 2 Windows inner skips and 1 Linux inner skip). Linux validates fresh Git archive snapshots of the recorded source; its local snapshot commit IDs differ from the PR commits.Related tool-only integration contexts, each applied independently on that earlier seven-project combined tree and tested on Windows and WSL:
a5c5d9908a0c547fd553889dc4d840f0018d9e60, local projection94bf4017e8a5c2f4df36c0da1e76f7c0c46f9433; focused suites passed on both platforms.8608bd21713b5a007a324c5148dd30a571657295, local projection0e817d51db938ed2a407774b89c1ca9c83b8ad35; focused suites passed on both platforms.2d04e57a5753c1dbf94aa1369a0465541fcf58b3, local projectionc0586d77d87648be6131795e12e047f52c80af9a; focused suites passed on both platforms.These projections cover the listed tool/CMake/test files, not whole external PR builds, GPU execution or hosted CI. Exact file blobs and Linux snapshot hashes are retained in the local verification record.
The subsequent rebase to the tested base changed none of the seven production/test blobs or the relevant main tool, root-CMake and repository-rule inputs. Independent Windows/WSL focused suites and negative controls were rerun at the final heads; earlier combined/projection runs are reported with their actual historical commits above. Upstream continued merging unrelated native-code changes during publication: the push gate fetches main, verifies unchanged tools/root CMake/cmake/rules/template inputs and checks the actual merge tree preserves every changed blob. Local test and native-build claims refer to the explicit base/head above, not to later native-code merges.
Publication overlap check: the repository
pr_overlap.py --branchCLI was run at this exact branch head. It reported incomplete CMake comparisons for #674, #1835, #3786, #5021, #7201, #8588, #9508 and #11366 because those PRs conflict with base. Manual exact-revision per-file three-way merges are clean for six; #8588 and #11366 already have conflicts with current main and their conflict chunks are byte-identical after adding this branch. None implements this dangling-target fix. This is not a claim that those whole PRs are mergeable.Not run: hosted CI (pending after publication), macOS runtime, older Python runtime, full system-library build, game/PS5 execution or GPU validation. Host-tool changes do not require an inferred PS5 ABI/layout or graphics/performance evidence; none is claimed.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)