Skip to content

ci(rccl): run Anvil plugin unit tests in GIN sbatch #12441

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dlamd1dai wants to merge 1 commit into from
Closed

ci(rccl): run Anvil plugin unit tests in GIN sbatch #12441

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions projects/rccl/tools/ci/lib/gin.sbatch
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ nm -CD "${librccl}" 2>/dev/null | grep -E " U .*${sym}" || echo " not undefined
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}"

@amd-argus amd-argus Bot Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low

RCCL_ANVIL_PLUGIN_TEST duplicates the fallback that consumes it: RCCL_FIXTURES_BIN_DIR is the dirname of line 121, the same ${rccl_build}/test directory line 122 builds this path from, so both branches of line 339 resolve to the same file. It is not an override hook either, since line 316 re-sources rccl.env last and overwrites any inherited value. Could this use the plain form line 323 uses?

} > "${workdir}/.ci-out/rccl.env"
) > "${build_out}" 2> "${build_err}" || build_rc=$?
echo "==> build phase logs: ${build_out} / ${build_err}"
Expand Down Expand Up @@ -332,6 +333,22 @@ else
echo " SKIP: ${sdma_factory_test} not found (BUILD_TESTS may be off)"
fi

# --- unit test: Anvil host plugin gtest (already built in step 4). ---
# Covers bind-signals / logical-context stripe coverage that install.sh -t skips.
echo "==> [6b/7] run Anvil plugin unit tests"

@amd-argus amd-argus Bot Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium

Step 7 already runs this binary, so the stated gap does not hold: projects/rccl/tools/ci/lib/gin-tests.json:58 registers rccl-UnitTestsGinAnvilPlugin as rccl-gin-conn-check-plugin-ut, and projects/rccl/tools/ci/run-gin-ci.sh:202 resolves that to the same ${rccl_build}/test path this line uses. That run is also timeout wrapped and feeds the tests-failed sentinel, while this one only echoes to stderr. Could this step be dropped?

anvil_plugin_test="${RCCL_ANVIL_PLUGIN_TEST:-${RCCL_FIXTURES_BIN_DIR}/rccl-UnitTestsGinAnvilPlugin}"
if [[ -x "${anvil_plugin_test}" ]]; then
_sysdeps_lib=""
for _sp in "${ROCM_PATH}/lib/rocm_sysdeps/lib" "${ROCM_PATH}/core/lib/rocm_sysdeps/lib"; do
[[ -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:-}"

@amd-argus amd-argus Bot Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low

Lines 341-345 are byte identical to 325-329 in the SDMA factory block, and the rest of this block mirrors 320-334 with only the names swapped. A helper taking binary, xml name and label would collapse both sites and stop them drifting; it needs to sit at top level, since the build subshell that holds the other helpers ends at line 299.

"${anvil_plugin_test}" --gtest_output="xml:${logs_dir}/anvil-plugin-test.xml" \

@amd-argus amd-argus Bot Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low

This gtest runs with no wall clock cap, unlike every fixtures entry in projects/rccl/tools/ci/run-gin-ci.sh:255, which wraps the same binary in timeout --kill-after. A stall here would burn the job's --time=04:00:00 budget before step 7 ever starts. Could it be wrapped the same way?

|| 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)"

@amd-argus amd-argus Bot Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low

This SKIP branch cannot be reached: line 227 already exits 1 when the plugin binary is not executable, and line 303 skips the whole test phase whenever the build failed. Its hint is wrong too, since projects/rccl/test/CMakeLists.txt:466 gates this target on ENABLE_ROCSHMEM_GIN and ROCM_VERSION >= 60400, not on GIN_ANVIL_UNIT_TESTS.

fi

# --- test phase -> test-%j.{out,err}. Don't fail the sbatch on test failures
# (that would look like a build/infra failure); drop a `tests-failed` sentinel
# instead, which the workflow turns into a red (non-gating for gin) result. ---
Expand Down
Loading