Skip to content

[ci] Adding determine_rocm_test_dependencies.py script - #4570

Merged
geomin12 merged 21 commits into
mainfrom
users/geomin12/test-dependency
May 6, 2026
Merged

geomin12 merged 21 commits into
mainfrom
users/geomin12/test-dependency

Conversation

@geomin12

Copy link
Copy Markdown
Contributor

For rocm-libraries, some components test are run even though upstreams changes to components do not affect downstream components. this results in wasted CI resources and wasted developer time.

Closes #3491

This is not used in TheRock but will be implemented in rocm-libraries and will continue to expand as we determine component specific updates.

Originally, we were going to use BUILD_TOPOLOGY, but it would test more than is needed (not granular enough for specific components). With these tests given from component teams, we can target which tests have best value

geomin12 and others added 19 commits March 5, 2026 13:49
Make the script simpler and more focused by:
- Removing verbose print methods and formatting logic
- Outputting only JSON array of packages to test
- Simplifying tests to 3 basic sanity checks
- Removing complex edge-case test scenarios
- Cleaning up module docstring

The script now has a clean, minimal API for both programmatic use
(create_analyzer) and command-line use (--changed flag).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a new --list-packages flag that outputs packages grouped by their
artifact/directory in pretty-printed JSON format.

Example output:
{
  "ml-libs": ["miopen", "hipblaslt", ...],
  "math-libs": ["rocblas", "hipblas", ...]
}

The --changed flag is now optional when using --list-packages.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ndencies

Replace hardcoded DIRECT_DEPENDENT_OVERRIDES with declarative test_deps field
in BUILD_TOPOLOGY.toml. This allows artifacts to specify which downstream
packages should be tested when they change, rather than testing the entire
dependency graph.

Changes:
- Add optional test_deps field to artifact schema in BUILD_TOPOLOGY.toml
- Set test_deps = ["solver"] for blas artifact to limit expensive test runs
- Update determine_rocm_test_dependencies.py to read test_deps from TOML
- Remove DIRECT_DEPENDENT_OVERRIDES Python dictionary
- Add tests for test_deps parsing, multiple changed packages, and real-world scenarios

When test_deps is specified, only those packages (plus the changed package
itself) are tested. When not specified, all direct downstream dependents
from the reverse artifact_deps graph are tested (existing behavior).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace BUILD_TOPOLOGY.toml artifact-level test dependencies with CMake-
generated subproject-level manifest for granular test control. This enables
specifying exactly which subprojects to test when a component changes
(e.g., rocBLAS → hipBLAS, rocSOLVER only, not all dependents).

CMake changes:
- Add TEST_SUBPROJECTS parameter to therock_cmake_subproject_declare()
- Add therock_emit_subproject_test_manifest() to generate JSON manifest
- Call manifest generation in top-level CMakeLists.txt finalization
- Store TEST_SUBPROJECTS in THEROCK_TEST_SUBPROJECTS target property

Subproject changes:
- Add TEST_SUBPROJECTS to rocBLAS declaration (hipBLAS, rocSOLVER)

Python script changes:
- Rewrite determine_rocm_test_dependencies.py to read CMake manifest
- Change from artifact-level (blas) to subproject-level (rocBLAS) granularity
- Read from build/subproject_test_manifest.json instead of BUILD_TOPOLOGY.toml
- Support --build-dir parameter for non-default build locations

Test changes:
- Update all tests to use JSON manifest format
- Change test data from TOML artifacts to JSON subprojects
- All 10 tests pass

Generated manifest structure:
{
  "subprojects": {
    "rocBLAS": {
      "runtime_deps": ["hip-clr", ...],
      "test_subprojects": ["hipBLAS", "rocSOLVER"]
    }
  }
}

When test_subprojects is specified, only those subprojects (plus the changed
one) are tested. Otherwise, all direct downstream dependents from the reverse
runtime_deps graph are tested.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a simpler programmatic API for getting test dependencies without
needing to create an analyzer instance explicitly.

Usage:
    from determine_rocm_test_dependencies import get_rocm_test_dependencies

    tests_to_run = get_rocm_test_dependencies(["rocBLAS"])
    # Returns: {'rocBLAS', 'hipBLAS', 'rocSOLVER'}

Changes:
- Add get_rocm_test_dependencies(changed_subprojects, therock_dir, build_dir)
- Update create_analyzer() docstring to recommend the new function
- Add test for the convenience function (11 tests total, all pass)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CMake changes:
- Add TEST_SUBPROJECTS to rocSPARSE, hipSPARSE, hipSPARSELt
- rocSPARSE update → test rocSPARSE and hipSPARSE
- hipSPARSE update → test hipSPARSE only
- hipSPARSELt update → test hipSPARSELt only

Script improvements:
- Minimize determine_rocm_test_dependencies.py (287→174 lines)
- Minimize test file (278→73 lines, 10→2 tests)
- Add --projects flag (alias for --changed)
- Add --format option (json/list) for easier parsing
- Simplify for integration with fetch_test_configurations.py

BUILD_TOPOLOGY.toml:
- Remove test_deps documentation (superseded by TEST_SUBPROJECTS)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace CMake-generated JSON manifest with direct CMake parsing.
The script now reads CMakeLists.txt files and extracts TEST_SUBPROJECTS
declarations using regex, eliminating the need for build-time manifest
generation.

Changes:
- Parse CMakeLists.txt files directly for TEST_SUBPROJECTS
- Remove therock_emit_subproject_test_manifest() CMake function
- Remove manifest generation from CMakeLists.txt
- Remove hardcoded config file (not needed)
- Simplify tests to parse real CMake files

Benefits:
- Single source of truth (CMakeLists.txt)
- No build directory required
- No JSON intermediate files
- Easier maintenance

Usage:
  python determine_rocm_test_dependencies.py --projects rocBLAS --format list

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jayhawk-commits

Copy link
Copy Markdown
Contributor

I think these new scripts are good candidates to be placed not in build_tools, but in the new directory we talked about: test_tools.

@geomin12

Copy link
Copy Markdown
Contributor Author

I think these new scripts are good candidates to be placed not in build_tools, but in the new directory we talked about: test_tools.

good call! done :)

Comment thread test_tools/determine_rocm_test_dependencies.py Outdated

@jayhawk-commits jayhawk-commits left a comment

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.

Issues I identified previously were addressed. LGTM

@geomin12
geomin12 merged commit e55ebbc into main May 6, 2026
96 of 103 checks passed
@geomin12
geomin12 deleted the users/geomin12/test-dependency branch May 6, 2026 22:49
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage May 6, 2026
@TorreZuk

TorreZuk commented May 6, 2026

Copy link
Copy Markdown
Contributor

Thanks @geomin12 for making progress on this.

kcossett-amd pushed a commit that referenced this pull request May 7, 2026
For `rocm-libraries`, some components test are run even though upstreams
changes to components do not affect downstream components. this results
in wasted CI resources and wasted developer time.

Closes #3491

This is not used in TheRock but will be implemented in `rocm-libraries`
and will continue to expand as we determine component specific updates.

Originally, we were going to use BUILD_TOPOLOGY, but it would test more
than is needed (not granular enough for specific components). With these
tests given from component teams, we can target which tests have best
value

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines +516 to +517
# Optional override for test dependencies (subproject names to test when this changes).
THEROCK_TEST_SUBPROJECTS "${ARG_TEST_SUBPROJECTS}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Guys, changes to the build system really need review from build system maintainers.

This approach will not work, see my comments on

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[TheRock CI] upstream component testing run and failures blocks even though unrelated

4 participants