rocr-debug-agent: Add unit test execution to CI wrapper script by amd-bfilipov · Pull Request #13375 · ROCm/rocm-systems

amd-bfilipov · 2026-10-13T16:31:47Z

Update test_rocr-debug-agent.py (the CI wrapper) to run the CPU unit tests as well as the GPU integration tests.

Changes

get_default_paths() discovers the unit test binary at its flat install path, tests/rocm-debug-agent/rocm-debug-agent-unit-tests (no unit/ subdirectory, matching the flat installation from CMakeLists.txt). Only the binaries needed by the selected tiers are required.
Added run_unit_tests(). Unit tests are deterministic, so they run once, without retries. The integration tests keep their retry-with-backoff behavior.
Added --gpu-tests and --cpu-tests flags to select which tiers run.
A command that cannot be launched at all (e.g. a non-executable binary) is reported as Failed to launch ... and exits 1, instead of a Python traceback. Launch failures are not retried.
TESTING.md documents the two test tiers, the flags and required binaries, and the environment the wrapper sets up.

Flag behavior

Flags	Runs	Required binaries
none	integration + unit	integration; unit is optional (skipped with a warning if missing)
`--gpu-tests`	integration only	integration
`--cpu-tests`	unit only (no GPU)	unit
`--gpu-tests --cpu-tests`	both	integration and unit

Required binaries are checked up front, before any test runs. The first failing tier stops the run.

Compatibility

With no flags the wrapper still requires the integration binary, so GPU-only runners keep working. The unit tests are optional in that mode "for now", for compatibility: they run if rocm-debug-agent-unit-tests was built (ENABLE_UNIT_TESTS=ON) and are skipped with a warning otherwise. Note that a no-flag job on a build that ships the unit test binary now also runs the unit tests; use --gpu-tests to run only the integration tests.

JIRA ID : AIROCGDB-635

🤖 Generated with Claude Code

therock-pr-bot · 2026-10-13T16:32:35Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	PR does not contain code files — Unit Test auto-passed
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

lumachad · 2026-10-14T09:26:12Z

I'll go through this one. Thanks!

lumachad

Drop retry logic for unit tests. run_unit_tests reuses the max_retries/retry_delay wrapper from run_tests. Unit tests are deterministic CPU-only tests, not GPU tests — retries mask real failures and waste CI time. Run once via subprocess.run(cmd, cwd=..., env=..., check=True), exit non-zero on failure, no max_retries/retry_delay params.
--skip-unit is redundant. --gpu-tests alone already skips unit tests — same code path. Drop --skip-unit.
PR description doesn't match the code. Description says --gou-only/--cpu-only; actual flags are --gpu-tests/--cpu-tests. Fix the description.
TESTING.md is stale. Still says "There is no CPU-only or unit-test tier" / lists unit tests under "Tiers that do not exist yet." Update it since this PR makes unit tests runnable via the CI wrapper.
Minor: args.gpu_tests or (not args.gpu_tests and not args.cpu_tests) simplifies to args.gpu_tests or not args.cpu_tests.

🤖 This review was generated with assistance from Claude Code.

lumachad · 2026-10-28T10:25:01Z

One more thing: TheRock's CI calls this script with no flags, assuming it only runs GPU integration tests. With this PR, the no-flags default now also runs unit tests. If the unit test binary isn't present/buildable in that CI path yet, or if adding ~unit test time to every GPU run isn't desired there, this changes TheRock's existing behavior without it opting in.

Is there a fallback planned for that caller (e.g. keep default as GPU-only until TheRock passes --cpu-tests/--gpu-tests explicitly), or has TheRock's invocation already been updated in lockstep with this change?

🤖 This review was generated with assistance from Claude Code.

lumachad · 2026-11-02T12:29:40Z

@amd-bfilipov Any updates on this one?

amd-bfilipov · 2026-11-02T14:25:36Z

@amd-bfilipov Any updates on this one?

@lumachad Yes:

The default (no flags) is back to GPU integration tests only, so TheRock's existing no-flag call is unchanged. Unit tests now run only with --cpu-tests; --gpu-tests --cpu-tests runs both. --cpu-tests exits 1 if the unit test binary is missing, so an opted-in job can't pass silently without it. The TheRock CPU/GPU jobs will pass the flags explicitly once the rocm-systems pin includes this PR.
Also addressed the other comments:

Unit tests no longer use the retry wrapper. run_unit_tests runs once via subprocess.run(..., check=True) and exits non-zero on failure.
Dropped --skip-unit.
Simplified the condition to args.gpu_tests or not args.cpu_tests.
Updated TESTING.md for the unit tier, the flags and the no-retry behavior.
Fixed the PR description to say --gpu-tests/--cpu-tests.

lumachad · 2026-11-02T17:20:56Z

@amd-bfilipov Any updates on this one?

@lumachad Yes:

The default (no flags) is back to GPU integration tests only, so TheRock's existing no-flag call is unchanged. Unit tests now run only with --cpu-tests; --gpu-tests --cpu-tests runs both. --cpu-tests exits 1 if the unit test binary is missing, so an opted-in job can't pass silently without it. The TheRock CPU/GPU jobs will pass the flags explicitly once the rocm-systems pin includes this PR. Also addressed the other comments:

Unit tests no longer use the retry wrapper. run_unit_tests runs once via subprocess.run(..., check=True) and exits non-zero on failure.

Dropped --skip-unit.

Simplified the condition to args.gpu_tests or not args.cpu_tests.

Updated TESTING.md for the unit tier, the flags and the no-retry behavior.

Fixed the PR description to say --gpu-tests/--cpu-tests.

Thanks. I've added a few bits and pieces on top. See if that makes sense please.

Update test_rocr-debug-agent.py to run both integration and unit tests: - Modified get_default_paths() to discover unit test binary (flat install path) - Added run_unit_tests() function, which runs the unit tests once - Updated main() to execute unit tests after integration tests - Unit tests are optional (skipped if the binary is not found) Unit tests are deterministic CPU-only tests, so unlike the GPU integration tests they are not retried: a failure is a real failure. Unit tests are installed flat at tests/rocm-debug-agent/rocm-debug-agent-unit-tests (no unit/ subdirectory, matching the flat installation from CMakeLists.txt). Ticket: AIROCGDB-635

Add explicit flags to control which test suite runs: - --gpu-tests: Run GPU-based integration tests - --cpu-tests: Run CPU-based unit tests - Default (no flags): Run integration tests only - Both flags: Run both test suites The default is unchanged from before unit tests were added, so existing callers that pass no flags (e.g. TheRock CI) keep running only the GPU integration tests and opt in to unit tests explicitly. Passing --cpu-tests when the unit test binary is missing is an error. This enables separate CI job configurations for CPU-only workers (unit tests) and GPU workers (integration tests), allowing unit tests to run on cheaper infrastructure while GPU tests remain on specialized hardware. Update TESTING.md, which still stated that there is no CPU-only or unit-test tier, to describe both tiers and the wrapper flags. Ticket: AIROCGDB-635

Make the CI wrapper's binary requirements follow its flags: - no flags: integration required, unit tests optional (run if present, for compatibility) - --gpu-tests: integration required, no unit tests - --cpu-tests: unit test binary required, no GPU artifacts - --gpu-tests --cpu-tests: both required Previously --cpu-tests still exited if the GPU integration binary or run-test.py was missing, and a missing unit test binary was only detected after the integration tests had already run. Required paths are now validated up front, based on the selected tiers. Also: - Report test launch failures (e.g. a non-executable binary) with a "Failed to launch" error instead of a Python traceback. Launch failures are not retried. - Move the command logging, timing and launch handling shared by run_tests and run_unit_tests into a _run_once helper. The retry policy stays in run_tests and unit tests still run once. - Update TESTING.md with a table of the flags and required binaries, and describe the wrapper environment accurately: PATH and LD_LIBRARY_PATH are prepended, ROCM_PATH is dropped unless --try-rocm-path is given, and HSA_TOOLS_LIB is set by run-test.py for the integration tests only. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

amd-bfilipov · 2026-11-03T14:59:15Z

@amd-bfilipov Any updates on this one?

@lumachad Yes:
...
Thanks. I've added a few bits and pieces on top. See if that makes sense please.

Yes, thank you.

One nit:
I see in codeowners file:
Unknown owner on line 29: make sure @solaiys exists and has write access to the repository

solaiys · 2026-11-03T18:11:31Z

@amd-bfilipov Any updates on this one?

@lumachad Yes:
...
Thanks. I've added a few bits and pieces on top. See if that makes sense please.

Yes, thank you.

One nit: I see in codeowners file: Unknown owner on line 29: make sure @solaiys exists and has write access to the repository

Hi @amd-bfilipov ,
I am (@solaiys ) the initial author for this file : projects/rocm-core/rdhc/rdhc.py.
I still exist and i guess i should have the write access as well.

lumachad

Thanks. This is OK.

systems-assistant · 2026-11-05T15:44:20Z

TheRock Submodule Bump Activity

Newest first

2026-11-05 — Included in TheRock via ROCm/TheRock#9739.

amd-bfilipov requested a review from a team as a code owner October 13, 2026 16:31

github-actions Bot added the project: rocr-debug-agent label Oct 13, 2026

systems-assistant Bot added the organization: ROCm label Oct 13, 2026

amd-bfilipov force-pushed the users/bfilipov/add-unit-to-test-script branch from 73defc3 to 1313ecf Compare October 19, 2026 13:56

lumachad requested changes Oct 28, 2026

View reviewed changes

lumachad assigned amd-bfilipov Oct 28, 2026

amd-bfilipov force-pushed the users/bfilipov/add-unit-to-test-script branch from 1313ecf to 9287cf2 Compare November 2, 2026 14:22

lumachad self-requested a review November 2, 2026 17:19

Bratislav Filipovic and others added 3 commits November 2, 2026 17:22

lumachad force-pushed the users/bfilipov/add-unit-to-test-script branch from cd08c15 to 36b2b88 Compare November 2, 2026 17:22

lumachad mentioned this pull request Nov 4, 2026

debug-tools: Split rocr-debug-agent tests into CPU and GPU jobs ROCm/TheRock#9357

Open

lumachad approved these changes Nov 4, 2026

View reviewed changes

amd-bfilipov merged commit 5787e71 into develop Nov 4, 2026
61 of 62 checks passed

amd-bfilipov deleted the users/bfilipov/add-unit-to-test-script branch November 4, 2026 14:54

Navigation Menu

Repository navigation

rocr-debug-agent: Add unit test execution to CI wrapper script - #13375

rocr-debug-agent: Add unit test execution to CI wrapper script#13375
amd-bfilipov merged 3 commits into
developfrom
users/bfilipov/add-unit-to-test-script

amd-bfilipov commented Oct 13, 2026 •

edited by lumachad

Loading

Uh oh!

therock-pr-bot Bot commented Oct 13, 2026

Uh oh!

lumachad commented Oct 14, 2026

Uh oh!

lumachad left a comment •

edited

Loading

Uh oh!

lumachad commented Oct 28, 2026

Uh oh!

lumachad commented Nov 2, 2026

Uh oh!

amd-bfilipov commented Nov 2, 2026

Uh oh!

lumachad commented Nov 2, 2026

Uh oh!

amd-bfilipov commented Nov 3, 2026 •

edited

Loading

Uh oh!

solaiys commented Nov 3, 2026 •

edited

Loading

Uh oh!

lumachad left a comment

Uh oh!

Uh oh!

systems-assistant Bot commented Nov 5, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

amd-bfilipov commented Oct 13, 2026 • edited by lumachad Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Flag behavior

Compatibility

Uh oh!

therock-pr-bot Bot commented Oct 13, 2026

✅ All Policy Checks Passed

Uh oh!

lumachad commented Oct 14, 2026

Uh oh!

lumachad left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lumachad commented Oct 28, 2026

Uh oh!

lumachad commented Nov 2, 2026

Uh oh!

amd-bfilipov commented Nov 2, 2026

Uh oh!

lumachad commented Nov 2, 2026

Uh oh!

amd-bfilipov commented Nov 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

solaiys commented Nov 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lumachad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

systems-assistant Bot commented Nov 5, 2026

TheRock Submodule Bump Activity

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

amd-bfilipov commented Oct 13, 2026 •

edited by lumachad

Loading

lumachad left a comment •

edited

Loading

amd-bfilipov commented Nov 3, 2026 •

edited

Loading

solaiys commented Nov 3, 2026 •

edited

Loading