test: remove source guard tests - #87
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7f5a58932
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| void TestInt32PatternHasNoKnownCollisions() | ||
| { | ||
| using TileXRCollectivesTest::CanUseCollisionFreeInt32Pattern; |
There was a problem hiding this comment.
This function is an executable unit test, not a source guard: it verifies that the INT32 patterns used by the correctness and performance tools uniquely identify rank/index payloads. When ExpectedAllGatherValue, ExpectedAllToAllValue, or their bounds change, a collision can let a misplaced collective payload compare equal to the expected value; the remaining host suite has no equivalent check, while the consumers require NPU execution. Move this test into a dedicated unit target rather than deleting it with the source-inspection checks.
Useful? React with 👍 / 👎.
| assert "python3 \"${SCRIPT_DIR}/smoke_collectives.py\"" not in source | ||
|
|
||
|
|
||
| def test_smoke_launcher_accepts_preflight_sentinel_with_plugin_stdout(tmp_path) -> None: |
There was a problem hiding this comment.
This deleted test actually executes run_tilexr_collectives_smoke.sh; it is not a source guard. In environments where the selected Python interpreter or an auto-loaded plugin writes informational output before the preflight sentinel, it verifies that the launcher still accepts the interpreter and reaches the smoke command. The remaining test_vllm_collectives_patch.py tests adapter routing only, so deleting this file removes the sole regression test for that launcher behavior; move the test to a behavioral test module instead.
Useful? React with 👍 / 👎.
| @@ -308,9 +284,6 @@ foreach(_tilexr_collectives_link_target | |||
| TileXRAddAscendRpathLink(${_tilexr_collectives_link_target}) | |||
There was a problem hiding this comment.
After these source-oriented targets are removed, the recommended CTest command no longer checks collectives scripts, documentation, or general tool wiring, but README.md lines 314–320 and tests/collectives/README.md lines 18–19 still explicitly claim that it does. A contributor can therefore run the documented validation successfully and incorrectly report coverage that is no longer present; update those validation descriptions to list only the remaining header, library, and behavioral checks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6de9e3b77
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "${cmake_bin}" --build "${build_dir}" --target \ | ||
| test_tilexr_collectives_kernel_ownership \ | ||
| test_tilexr_collectives_tools_sources \ | ||
| tilexr_collective_perf -j"${build_jobs}" |
There was a problem hiding this comment.
When an operator follows docs/COLLECTIVES_MULTIHOST_PROFILING.md lines 88–94 and runs prepare, this changed target list builds only tilexr_collective_perf, while run_preflight now runs only the profile-report tests. The runbook still says that prepare performs guard checks on every peer, so a successful preparation can be reported as coverage that no longer occurred; update the runbook to describe the reduced preflight or restore equivalent checks.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation
scripts/ci/provision/verify.shgit diff --check, deleted-test reference scan, and source/guard filename scan passedNo NPU data-plane implementation behavior is changed.