Repository navigation
Conversation
gin.sbatch already builds rccl-UnitTestsGinAnvilPlugin under GIN_ANVIL_UNIT_TESTS=ON; execute it next to the SDMA factory gtest so logical-context bind coverage is exercised in CI (AICOMRCCL-2339). Co-authored-by: Cursor <cursoragent@cursor.com>
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
![amd-argus[bot]](https://avatars.githubusercontent.com/in/4740020?s=60&v=4){kind=small}
There was a problem hiding this comment.
ARGUS
Bottom line: Worth settling before approval: gin-tests.json:57-63 already runs rccl-UnitTestsGinAnvilPlugin via run-gin-ci.sh in step 7, so this block re-runs the same binary with no timeout and no failure accounting.
Comment @argus review to re-run, or @argus re-review after pushing.
| [[ -d "${_sp}" ]] && _sysdeps_lib="${_sp}" | ||
| done | ||
| export LD_LIBRARY_PATH="${RCCL_INSTALL_PREFIX}/lib:${ROCSHMEM_INSTALL_DIR}/lib:${ROCM_PATH}/lib${_sysdeps_lib:+:${_sysdeps_lib}}:${LD_LIBRARY_PATH:-}" | ||
| "${anvil_plugin_test}" --gtest_output="xml:${logs_dir}/anvil-plugin-test.xml" \ |
There was a problem hiding this comment.
Medium
This invocation has no wall-clock cap, unlike every other test run in this job: run-gin-ci.sh:255 wraps each fixtures run in timeout with BENCH_TIMEOUT (600s by default). Because this runs before step 7, a wedged run consumes the 4h --time budget at line 7 and slurm kills the job, so lines 361-367 never write tests-failed and the hang reads as an infra failure instead of a test timeout. Could we wrap it the same way?
| printf 'export RCCL_TESTS_BIN_DIR=%q\n' "${rccl_tests_build}" | ||
| printf 'export RCCL_TESTS_DIR=%q\n' "${rccl_tests_src}" | ||
| printf 'export RCCL_FIXTURES_BIN_DIR=%q\n' "$(dirname "${rccl_fixtures_bin}")" | ||
| printf 'export RCCL_ANVIL_PLUGIN_TEST=%q\n' "${rccl_conn_plugin_bin}" |
There was a problem hiding this comment.
Low
RCCL_ANVIL_PLUGIN_TEST has no consumer and no effect: line 122 makes it ${rccl_build}/test/rccl-UnitTestsGinAnvilPlugin, which is exactly what the :- fallback at line 339 expands to through RCCL_FIXTURES_BIN_DIR. It cannot serve as an operator override either, since line 316 re-sources rccl.env over any inherited value before line 339 reads it. Could we drop it and locate the binary from RCCL_FIXTURES_BIN_DIR the way line 323 does?
| "${anvil_plugin_test}" --gtest_output="xml:${logs_dir}/anvil-plugin-test.xml" \ | ||
| || echo "WARNING: Anvil plugin unit tests failed (non-gating)" >&2 | ||
| else | ||
| echo " SKIP: ${anvil_plugin_test} not found (GIN_ANVIL_UNIT_TESTS may be off)" |
There was a problem hiding this comment.
Low
This SKIP names a cause the script forecloses: line 227 hard-fails the build when rccl_conn_plugin_bin is not executable and line 303 exits before reaching here, so the branch is unreachable. GIN_ANVIL_UNIT_TESTS is not what gates the target either; test/CMakeLists.txt:466 keys it on ENABLE_ROCSHMEM_GIN and ROCM_VERSION >= 60400.
| fi | ||
|
|
||
| # --- unit test: Anvil host plugin gtest (already built in step 4). --- | ||
| # Covers bind-signals / logical-context stripe coverage that install.sh -t skips. |
There was a problem hiding this comment.
Nit
| # Covers bind-signals / logical-context stripe coverage that install.sh -t skips. | |
| # Covers the bind-signals and conn-check cases in the Anvil host plugin gtest. |
Give the early GIN sbatch run the same timeout and tests-failed accounting as the fixtures suite, and drop the unused path export and unreachable skip. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Closing this. Step 6b only runs the same binary again. The SDMA factory early run is different: |
Motivation
Follow-up to #11569 / AICOMRCCL-2339.
gin.sbatchalready buildsrccl-UnitTestsGinAnvilPluginunderGIN_ANVIL_UNIT_TESTS=ON, but only the SDMA factory gtest is executed. Logical-context bind coverage therefore has no automated GIN-CI run.Technical Details
RCCL_ANVIL_PLUGIN_TESTfrom the build stageJIRA ID: AICOMRCCL-2339
Test Plan
rccl-UnitTestsGinAnvilPluginTest Result
Submission Checklist
Made with Cursor