ROCm / ROCm/rocm-cli

Workflow contract helpers match commented-out lines in run: blocks

Open Beginner friendly
#388 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

github_actions
Dominant language
Rust
Stars
40
Forks
9
Avg merge
4d 20h
Merged PRs (30d)
59

Description

multiline_run_blocks in xtask/src/workflow_contract.rs keeps shell comment lines when it flattens a run: | block. Every contains assertion built on it can therefore be satisfied by a line that is commented out and has no effect on the runner.

Why it matters

This is not only theoretical. Commenting out the two exports in the Windows lifecycle step:

# $env:ROCM_CLI_BINARY = "$targetDir\release\rocm.exe"
# $env:ROCM_CLI_ROCMD_BINARY = "$targetDir\release\rocmd.exe"

leaves ci_windows_lifecycle_lane_reuses_the_binaries_it_built green, even though it pins the full assignment strings — the commented line still contains them. Commenting two lines out is exactly what someone bisecting a CI failure does, so the guard is weakest in the situation it exists for.

The same class defeats the fail-fast Test-Path/throw assertion added in #342.

Four tests share the helper: self_hosted_prebuilt_e2e_lanes_export_rocmd, nightly_prebuilt_e2e_lanes_export_rocmd, and the two ..._enable_test_hooks tests.

Also in scope

assert_prebuilt_e2e_lanes_export_rocmd pins the full assignment for rocmd but only the bare variable name for rocm, so $env:ROCM_CLI_BINARY = "$targetDir\debug\rocm.exe" passes for e2e-selfhosted.yml and nightly.yml. #342 closed that asymmetry in its own new test and deliberately left the shared helper alone; both reviewers agreed the helper fix belongs in its own change. Doing both together is the tidier move.

Proposed fix

Drop comment-only lines when building the block, then pin the rocm assignment in the shared helper.

One thing to check before landing: assert_prebuilt_e2e_lanes_enable_test_hooks asserts a block contains e2e-test-hooks. Confirm no lane satisfies that only via a comment, or stripping comments will turn a passing test red for the right reason but at an inconvenient moment.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read xtask/src/workflow_contract.rs, starting with multiline_run_blocks and assert_prebuilt_e2e_lanes_export_rocmd. Run the four named workflow-contract tests, including the enable-test-hooks checks, and inspect the referenced Windows and nightly workflow blocks. Done means commented-out lines cannot satisfy the assertions and the intended rocm assignment is checked without breaking valid lanes.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, powershell, rust
Domain
ci-cd, testing-qa, tooling
Issue type
Bug
Difficulty
2/5
Estimated time
Half a day
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
86/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.