USHIFT-6337: Align release CI tests with brew RPM source commit#7089
USHIFT-6337: Align release CI tests with brew RPM source commit#7089copejon wants to merge 1 commit into
Conversation
…I jobs Release CI periodic jobs test brew-sourced RPMs that may lag behind branch HEAD, but execute Robot Framework tests from HEAD. This causes false failures when new tests reference features not yet in the brew RPMs. Add brew_test_alignment.sh which extracts the source commit SHA from the brew RPM's NVR release field and checks out test/suites/ and test/resources/ at that commit before running release scenarios. The harness scripts (test/bin/) and scenario definitions stay at HEAD so infrastructure improvements always apply. Guard conditions ensure this only activates for release jobs, fails hard on unparseable NVR or excessive version skew, and emits a machine-readable brew-alignment.json artifact per scenario for audit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@copejon: This pull request references USHIFT-6337 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughAdds sourced Brew alignment helpers, release-job integration, restoration of test directories, and per-scenario ChangesBrew test alignment
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant BrewAlignment
participant Git
participant ScenarioArtifacts
CI->>BrewAlignment: Resolve Brew release commit
BrewAlignment->>Git: Fetch or resolve commit
CI->>BrewAlignment: Checkout aligned test directories
BrewAlignment->>Git: Checkout test/suites and test/resources
CI->>BrewAlignment: Restore tests on exit
BrewAlignment->>Git: Restore directories at HEAD
CI->>ScenarioArtifacts: Emit brew-alignment.json
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: copejon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
test/bin/brew_test_alignment.sh (1)
171-179: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCoverage delta only counts added files, missing modified ones.
--diff-filter=Aonly catches brand-new files betweenbrew_shaandHEAD; existing test files that were modified (e.g. new test cases added to an existing suite file) aren't reflected incoverage_delta, understating the audit's usefulness.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/bin/brew_test_alignment.sh` around lines 171 - 179, Update the added_files computation in the coverage_delta block to include modified test suite and resource files as well as newly added files, using the appropriate diff filter for added and modified paths. Preserve the existing brew_sha/head_sha guards and JSON conversion behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/bin/brew_test_alignment.sh`:
- Around line 1-19: Update the header of the brew test alignment script to use
the approved /usr/bin/bash interpreter and enable the required strict shell
options, including errexit, nounset, and pipefail. Preserve the existing
sourced-only guard and variable initialization in brew_test_alignment.sh.
- Line 107: Update the head_sha declaration in the relevant shell function to
declare the readonly local variable separately, then assign it from git
rev-parse in a distinct command so its failure status is preserved and handled
according to the existing shell error flow.
- Around line 97-158: Update checkout_brew_aligned_tests and restore_head_tests
to reconcile the scoped test directories rather than using overlay git checkout
alone: remove paths absent from the target commit and restore paths present
there, for both brew_sha alignment and HEAD restoration. Keep all operations
limited to test/suites/ and test/resources/, and preserve the existing success,
failure, and alignment-state behavior.
In `@test/bin/scenario.sh`:
- Around line 1854-1857: Update action_run around emit_brew_alignment_artifact
so SCENARIO_INFO_DIR is initialized for the current scenario before the artifact
is emitted. Set it from $1 before the call, or move the call until after
SCENARIO_SCRIPT/load_scenario_script establishes the directory, while preserving
the existing BREW_TEST_ALIGNED guard.
---
Nitpick comments:
In `@test/bin/brew_test_alignment.sh`:
- Around line 171-179: Update the added_files computation in the coverage_delta
block to include modified test suite and resource files as well as newly added
files, using the appropriate diff filter for added and modified paths. Preserve
the existing brew_sha/head_sha guards and JSON conversion behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b05d8832-d4ce-4b50-96fa-aa6df32a0008
📒 Files selected for processing (3)
test/bin/brew_test_alignment.shtest/bin/ci_phase_boot_and_test.shtest/bin/scenario.sh
| #!/bin/bash | ||
| # | ||
| # Aligns test code (test/suites/, test/resources/) with the brew RPM | ||
| # source commit so that release CI jobs do not run HEAD tests against | ||
| # older brew-built binaries. | ||
| # | ||
| # Must be sourced, not executed directly. | ||
|
|
||
| if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then | ||
| echo "This script must be sourced, not executed." | ||
| exit 1 | ||
| fi | ||
|
|
||
| MAX_BREW_SKEW="${MAX_BREW_SKEW:-100}" | ||
|
|
||
| BREW_SOURCE_COMMIT="${BREW_SOURCE_COMMIT:-}" | ||
| BREW_SOURCE_SHORTCOMMIT="${BREW_SOURCE_SHORTCOMMIT:-}" | ||
| BREW_TEST_ALIGNED="${BREW_TEST_ALIGNED:-false}" | ||
| BREW_COMMIT_DELTA="${BREW_COMMIT_DELTA:-0}" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/bash
set -euo pipefail
printf 'File present and first few lines:\n'
if [ -f test/bin/brew_test_alignment.sh ]; then
sed -n '1,35p' test/bin/brew_test_alignment.sh | cat -n
else
printf 'MISSING\n'
fi
printf '\nShellcheck availability and status for file:\n'
if command -v shellcheck >/dev/null 2>&1; then
shellcheck --severity=error test/bin/brew_test_alignment.sh || true
else
printf 'shellcheck not available\n'
fiRepository: openshift/microshift
Length of output: 1593
🏁 Script executed:
#!/usr/bin/bash
set -euo pipefail
if command -v shellcheck >/dev/ null 2>&1; then
shellcheck test/bin/brew_test_alignment.sh || true
else
printf 'shellcheck not available\n'
fiRepository: openshift/microshift
Length of output: 219
Use the approved shell scripting template.
test/bin/brew_test_alignment.sh starts with #!/bin/bash and does not enable set -euo pipefail; use #!/usr/bin/bash and the required strict shell options for shell script files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/bin/brew_test_alignment.sh` around lines 1 - 19, Update the header of
the brew test alignment script to use the approved /usr/bin/bash interpreter and
enable the required strict shell options, including errexit, nounset, and
pipefail. Preserve the existing sourced-only guard and variable initialization
in brew_test_alignment.sh.
Source: Coding guidelines
| checkout_brew_aligned_tests() { | ||
| BREW_TEST_ALIGNED="false" | ||
| BREW_COMMIT_DELTA=0 | ||
|
|
||
| local -r brew_sha="${BREW_SOURCE_COMMIT:-}" | ||
| if [[ -z "${brew_sha}" ]]; then | ||
| echo "BREW_SOURCE_COMMIT is not set, skipping test alignment" | ||
| return 0 | ||
| fi | ||
|
|
||
| local -r head_sha="$(git -C "${ROOTDIR}" rev-parse HEAD)" | ||
| if [[ "${brew_sha}" == "${head_sha}" ]]; then | ||
| echo "Brew source commit matches HEAD, no alignment needed" | ||
| BREW_TEST_ALIGNED="true" | ||
| BREW_COMMIT_DELTA=0 | ||
| export BREW_TEST_ALIGNED BREW_COMMIT_DELTA | ||
| return 0 | ||
| fi | ||
|
|
||
| if ! fetch_brew_commit_if_needed "${brew_sha}"; then | ||
| echo "ERROR: Cannot proceed with test alignment" | ||
| return 1 | ||
| fi | ||
|
|
||
| local delta | ||
| if delta=$(git -C "${ROOTDIR}" rev-list --count "${brew_sha}..${head_sha}" 2>/dev/null); then | ||
| BREW_COMMIT_DELTA="${delta}" | ||
| else | ||
| echo "WARNING: Cannot compute commit delta (commits not on same lineage)" | ||
| BREW_COMMIT_DELTA=-1 | ||
| fi | ||
| export BREW_COMMIT_DELTA | ||
|
|
||
| echo "Brew source commit: ${brew_sha}" | ||
| echo "HEAD commit: ${head_sha}" | ||
| echo "Commit delta: ${BREW_COMMIT_DELTA}" | ||
|
|
||
| if [[ "${BREW_COMMIT_DELTA}" -ne -1 ]] && [[ "${BREW_COMMIT_DELTA}" -gt "${MAX_BREW_SKEW}" ]]; then | ||
| echo "ERROR: Commit delta ${BREW_COMMIT_DELTA} exceeds MAX_BREW_SKEW=${MAX_BREW_SKEW}" | ||
| return 1 | ||
| fi | ||
|
|
||
| echo "Checking out test/suites/ and test/resources/ at brew commit ${brew_sha}" | ||
| if ! git -C "${ROOTDIR}" checkout "${brew_sha}" -- test/suites/ test/resources/ 2>&1; then | ||
| echo "ERROR: Failed to checkout test directories at ${brew_sha}" | ||
| return 1 | ||
| fi | ||
|
|
||
| BREW_TEST_ALIGNED="true" | ||
| export BREW_TEST_ALIGNED | ||
|
|
||
| echo "Test code aligned to brew source commit ${brew_sha}" | ||
| return 0 | ||
| } | ||
|
|
||
| # Restore test directories to HEAD after tests complete. | ||
| restore_head_tests() { | ||
| if [[ "${BREW_TEST_ALIGNED:-false}" == "true" ]]; then | ||
| echo "Restoring test/suites/ and test/resources/ to HEAD" | ||
| git -C "${ROOTDIR}" checkout HEAD -- test/suites/ test/resources/ 2>/dev/null || true | ||
| fi | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)brew_test_alignment\.sh$|(^|/)test/(suites|resources)/' | head -80
echo
echo "Target section:"
if [ -f test/bin/brew_test_alignment.sh ]; then
nl -ba test/bin/brew_test_alignment.sh | sed -n '1,190p'
fi
echo
echo "Search tests for BREW_TEST_ALIGNED / BREW_COMMIT_DELTA:"
rg -n "BREW_TEST_ALIGNED|BREW_COMMIT_DELTA|MAX_BREW_SKEW|checkout_brew_aligned_tests|restore_head_tests|BREW_SOURCE_COMMIT" .Repository: openshift/microshift
Length of output: 3369
🌐 Web query:
git checkout documentation overlay mode never removes files from index or working tree
💡 Result:
In Git, "overlay mode" is a behavior where the command being executed does not remove files from the index or the working tree if they are absent in the source (e.g., the commit or tree-ish being checked out) [1][2]. Specifically regarding git checkout: - When using git checkout to restore paths, it operates in a default overlay mode [1][3]. - In this overlay mode, git checkout will update existing files in the index and working tree to match the specified source, but it will never remove files that are already present in your index or working tree if those files do not exist in the source [1][4]. - While the git checkout documentation mentions --overlay and --no-overlay flags, these are primarily discussed in the context of their implementation or related commands; historically, git checkout has maintained this non-destructive "overlay" behavior as its default [1][2]. This behavior contrasts with the newer command, git restore, which was introduced to separate and clarify the complex responsibilities of git checkout [5]. By default, git restore operates in "no-overlay" mode, meaning it will remove tracked files from the working tree or index if they do not exist in the source [6][7]. Users can pass the --overlay flag to git restore to mimic the traditional, non-destructive behavior of git checkout [8][6].
Citations:
- 1: https://git-scm.com/docs/git-checkout
- 2: https://code.googlesource.com/git/+/HEAD/Documentation/git-checkout.adoc
- 3: https://git-scm.com/docs/git-checkout/2.54.0
- 4: https://git-scm.com/docs/git-checkout.html
- 5: https://stackoverflow.com/questions/61130412/what-is-the-difference-between-git-checkout-vs-git-restore-for-reverting-un
- 6: https://git-scm.com/docs/git-restore/2.23.0
- 7: https://git-scm.com/docs/git-restore/2.25.0
- 8: https://stackoverflow.com/questions/67775685/disambiguation-similar-behaving-git-commands-checkout-and-restore
🏁 Script executed:
#!/usr/bin/bash
set -euo pipefail
echo "Target section:"
if command -v nl >/dev/null; then
nl -ba test/bin/brew_test_alignment.sh | sed -n '1,190p'
else
cat -n test/bin/brew_test_alignment.sh | sed -n '1,190p'
fi
echo
echo "Search tests for BREW_TEST_ALIGNED / BREW_COMMIT_DELTA:"
rg -n "BREW_TEST_ALIGNED|BREW_COMMIT_DELTA|MAX_BREW_SKEW|checkout_brew_aligned_tests|restore_head_tests|BREW_SOURCE_COMMIT" . || true
echo
echo "Behavior probe for checkout-path semantics:"
cd /tmp
rm -rf repo-shape-probe
mkdir repo-shape-probe
cd repo-shape-probe
export GIT_AUTHOR_NAME="Test" GIT_AUTHOR_EMAIL="test@test.test" GIT_COMMITTER_NAME="Test" GIT_COMMITTER_EMAIL="test@test.test"
git init -q
echo "old" > test/suites/x.robot
git add test/suites/x.robot
git commit -q -m "base"
OLD="$(git rev-parse HEAD)"
mkdir -p newdir
echo "extra" > test/suites/newdir/y.robot
echo "old" > test/suites/x.robot
echo "old" > test/resources/z.res
git add test/suites/newdir/y.robot test/suites/x.robot test/resources/z.res
git rm -q test/suites/x.robot
git commit -q -m "late"
HEAD="$(git rev-parse HEAD)"
echo "Before OLD checkout:"
find test -type f | sort
git -C "$PWD" checkout "$OLD" -- test/suites/ test/resources/ 2>&1 || true
echo "After OLD checkout:"
find test -type f | sort
git -C "$PWD" status --short
echo
git -C "$PWD" checkout "$HEAD" -- test/suites/ test/resources/ 2>&1 || true
echo "After restore HEAD checkout:"
find test -type f | sort
git -C "$PWD" status --shortRepository: openshift/microshift
Length of output: 11234
🏁 Script executed:
#!/usr/bin/bash
set -euo pipefail
echo "Behavior probe in temp repo:"
cd /tmp
rm -rf repo-shape-probe
mkdir -p repo-shape-probe
pushd repo-shape-probe >/dev/null
export GIT_AUTHOR_NAME="Test" GIT_AUTHOR_EMAIL="test@test.test" \
GIT_COMMITTER_NAME="Test" GIT_COMMITTER_EMAIL="test@test.test"
git init -q
mkdir -p test/suites test/resources
printf 'old\n' > test/suites/x.robot
git add test/suites/x.robot
git commit -q -m 'base'
OLD="$(git rev-parse HEAD)"
mkdir -p test/suites/newdir
printf 'extra\n' > test/suites/newdir/y.robot
printf 'old\n' > test/suites/x.robot
printf 'old\n' > test/resources/z.res
git add test/suites/newdir/y.robot test/suites/x.robot test/resources/z.res
git rm -q test/suites/x.robot
git commit -q -m 'late'
HEAD="$(git rev-parse HEAD)"
echo '--- before checkout ---'
find test -type f | sort
printf '--- checkout OLD ---\n'
git checkout "$OLD" -- test/suites/ test/resources/
echo '--- after checkout ---'
find test -type f | sort
git status --short
printf '--- checkout HEAD ---\n'
git checkout "$HEAD" -- test/suites/ test/resources/
echo '--- after restore ---'
find test -type f | sort
git status --short
popd >/dev/nullRepository: openshift/microshift
Length of output: 351
🌐 Web query:
git checkout pathspec diff-filter=A does not filter deleted files documentation
💡 Result:
The git checkout command does not support the --diff-filter option [1][2][3]. The --diff-filter option is a feature of git diff and other similar commands used for comparing revisions, but it is not available in git checkout [4][5]. To achieve the desired result—such as checking out files that have been deleted or modified in another branch—you must first use git diff to identify the files and then pass those file paths to git checkout [6][7]. A common and recommended approach is to use xargs to pipe the output of git diff into git checkout: git diff --diff-filter=A --name-only -z | xargs -0 git checkout -- In this command: 1. git diff --diff-filter=A --name-only -z identifies files that have been added (or otherwise match the filter) in the specified commit or branch and lists them as null-terminated strings (-z) [4][5][7]. 2. xargs -0 processes these null-terminated file names and passes them as arguments to the git checkout command [7]. 3. git checkout -- restores the specified files from the source branch to your current working directory [1][6]. Using the -z and -0 flags is best practice to correctly handle file names that contain spaces or special characters [7].
Citations:
- 1: https://git-scm.com/docs/git-checkout.html
- 2: https://git.github.io/htmldocs/git-checkout.html
- 3: https://git-scm.com/docs/git-checkout
- 4: https://git-scm.com/docs/git-diff
- 5: https://cdn.kernel.org/pub/software/scm/git/docs/git-diff.html
- 6: https://stackoverflow.com/questions/31682939/git-how-to-checkout-files-from-another-branch-based-on-diff-filter
- 7: https://stackoverflow.com/questions/35362525/pipe-git-diff-to-git-checkout
🏁 Script executed:
#!/usr/bin/bash
set -euo pipefail
echo "Behavior probe in temp repo:"
cd /tmp
rm -rf repo-shape-probe
mkdir -p repo-shape-probe
pushd repo-shape-probe >/dev/null
export GIT_AUTHOR_NAME="Test" GIT_AUTHOR_EMAIL="test@test.test" \
GIT_COMMITTER_NAME="Test" GIT_COMMITTER_EMAIL="test@test.test"
git init -q
mkdir -p test/suites test/resources
printf '%s\n' old > test/suites/x.robot
git add test/suites/x.robot
git commit -q -m 'base'
OLD="$(git rev-parse HEAD)"
mkdir -p test/suites/newdir
printf '%s\n' extra > test/suites/newdir/y.robot
printf '%s\n' old > test/suites/x.robot
printf '%s\n' old > test/resources/z.res
git add test/suites/newdir/y.robot test/suites/x.robot test/resources/z.res
git rm -q test/suites/x.robot
git commit -q -m 'late'
HEAD="$(git rev-parse HEAD)"
printf '%s\n' '--- before checkout ---'
find test -type f | sort
printf '%s\n' '--- checkout OLD ---'
git checkout "$OLD" -- test/suites/ test/resources/
printf '%s\n' '--- after checkout ---'
find test -type f | sort
git status --short
printf '%s\n' '--- checkout HEAD ---'
git checkout "$HEAD" -- test/suites/ test/resources/
printf '%s\n' '--- after restore ---'
find test -type f | sort
git status --short
printf '%s\n' '--- diff added/deleted between ---'
git diff --name-status --diff-filter=AD "$OLD"..."$HEAD" -- test/suites/ test/resources/
popd >/dev/nullRepository: openshift/microshift
Length of output: 353
🏁 Script executed:
#!/usr/bin/bash
set -euo pipefail
echo "Behavior probe in temp repo:"
cd /tmp
rm -rf repo-shape-probe
mkdir -p repo-shape-probe
pushd repo-shape-probe >/dev/null
export GIT_AUTHOR_NAME="Test" GIT_AUTHOR_EMAIL="test@test.test" \
GIT_COMMITTER_NAME="Test" GIT_COMMITTER_EMAIL="test@test.test"
git init -q
mkdir -p test/suites test/resources
printf '%s\n' old > test/suites/x.robot
printf '%s\n' base > test/resources/a.res
git add test
git commit -q -m 'base'
OLD="$(git rev-parse HEAD)"
mkdir -p test/suites/newdir
printf '%s\n' extra > test/suites/newdir/y.robot
printf '%s\n' old > test/suites/x.robot
printf '%s\n' removed > test/resources/removed.res
git add test/suites/newdir/y.robot test/suites/x.robot test/resources/removed.res
git rm -q test/suites/x.robot test/resources/removed.res
git commit -q -m 'late'
HEAD="$(git rev-parse HEAD)"
printf '%s\n' '--- before checkout ---'
find test -type f | sort
printf '%s\n' '--- checkout OLD ---'
git checkout "$OLD" -- test/suites test/resources
printf '%s\n' '--- after checkout ---'
find test -type f | sort
git status --short
printf '%s\n' '--- checkout HEAD ---'
git checkout "$HEAD" -- test/suites test/resources
printf '%s\n' '--- after restore ---'
find test -type f | sort
git status --short
printf '%s\n' '--- diff added/deleted between ---'
git diff --name-status --diff-filter=AD "$OLD"..."$HEAD" -- test/suites test/resources
popd >/dev/nullRepository: openshift/microshift
Length of output: 333
Align and restore by reconciling added and deleted paths.
git checkout <sha> -- test/suites test/resources uses overlay mode, so it won’t remove files added after brew_sha and won’t delete files removed after brew_sha; the restore checkout can leave those leftovers in the working tree. Use a diff-based cleanup or scoped tree export/delete logic for both alignment and HEAD restore.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/bin/brew_test_alignment.sh` around lines 97 - 158, Update
checkout_brew_aligned_tests and restore_head_tests to reconcile the scoped test
directories rather than using overlay git checkout alone: remove paths absent
from the target commit and restore paths present there, for both brew_sha
alignment and HEAD restoration. Keep all operations limited to test/suites/ and
test/resources/, and preserve the existing success, failure, and alignment-state
behavior.
| return 0 | ||
| fi | ||
|
|
||
| local -r head_sha="$(git -C "${ROOTDIR}" rev-parse HEAD)" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Declare/assign separately to avoid masking git rev-parse failures.
local -r head_sha="$(git -C "${ROOTDIR}" rev-parse HEAD)" masks the command substitution's exit status behind local's own (shellcheck SC2155). Without set -e, a failing rev-parse here silently yields an empty head_sha and the function proceeds with bogus comparisons/delta math.
As per coding guidelines, "Shell scripts must pass shellcheck."
🩹 Proposed fix
- local -r head_sha="$(git -C "${ROOTDIR}" rev-parse HEAD)"
+ local head_sha
+ if ! head_sha="$(git -C "${ROOTDIR}" rev-parse HEAD)"; then
+ echo "ERROR: Cannot resolve HEAD commit"
+ return 1
+ fi
+ readonly head_sha📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local -r head_sha="$(git -C "${ROOTDIR}" rev-parse HEAD)" | |
| local head_sha | |
| if ! head_sha="$(git -C "${ROOTDIR}" rev-parse HEAD)"; then | |
| echo "ERROR: Cannot resolve HEAD commit" | |
| return 1 | |
| fi | |
| readonly head_sha |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/bin/brew_test_alignment.sh` at line 107, Update the head_sha declaration
in the relevant shell function to declare the readonly local variable
separately, then assign it from git rev-parse in a distinct command so its
failure status is preserved and handled according to the existing shell error
flow.
Source: Coding guidelines
| if [[ "${BREW_TEST_ALIGNED:-false}" == "true" ]]; then | ||
| emit_brew_alignment_artifact | ||
| fi | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "SCENARIO_INFO_DIR=|^SCENARIO=" test/bin/scenario.sh
rg -n "load_global_settings\(\)|load_scenario_script\(\)" -A5 test/bin/scenario.shRepository: openshift/microshift
Length of output: 580
🏁 Script executed:
#!/usr/bin/bash
set -euo pipefail
printf '--- scenario.sh action context ---\n'
sed -n '1820,1870p' test/bin/scenario.sh
printf '\n--- brew_test_alignment usages/definitions ---\n'
fd -a 'brew_test_alignment.sh|scenario.sh' . | sed 's#^\./##' | xargs -r -n1 sh -c '
echo "== $0 =="
rg -n "emit_brew_alignment_artifact|SCENARIO_INFO_DIR|SCENARIO_SCRIPT=|SCENARIO=" "$0"
'
printf '\n--- load/load_global/load_scenario sections ---\n'
sed -n '1700,1785p' test/bin/scenario.shRepository: openshift/microshift
Length of output: 249
🏁 Script executed:
#!/usr/bin/bash
set -euo pipefail
printf '%s\n' '--- scenario.sh action context ---'
sed -n '1820,1870p' test/bin/scenario.sh
printf '%s\n' ''
printf '%s\n' '--- brew_test_alignment usages/definitions ---'
file_re='^test/bin/(scenario\.sh|brew_test_alignment\.sh)$'
if [ -f test/bin/scenario.sh ]; then
rg -n "emit_brew_alignment_artifact|SCENARIO_INFO_DIR|SCENARIO_SCRIPT=|SCENARIO=" test/bin/scenario.sh
fi
if [ -f test/bin/scenario.sh.bash ]; then
rg -n "emit_brew_alignment_artifact|SCENARIO_INFO_DIR|SCENARIO_SCRIPT=|SCENARIO=" test/bin/scenario.sh.bash
fi
if [ -f test/bin/brew_test_alignment.sh ]; then
rg -n "emit_brew_alignment_artifact|SCENARIO_INFO_DIR|SCENARIO_SCRIPT=|SCENARIO=" test/bin/brew_test_alignment.sh
fi
printf '%s\n' ''
printf '%s\n' '--- load sections ---'
sed -n '1700,1785p' test/bin/scenario.shRepository: openshift/microshift
Length of output: 5470
Initialize SCENARIO_INFO_DIR before emitting the Brew artifact.
emit_brew_alignment_artifact writes to ${SCENARIO_INFO_DIR}/${SCENARIO}, but in action_run the artifact is emitted before that path is established via ${SCENARIO_SCRIPT}/load_scenario_script. Set the required scenario directory from $1 before this call, or move the call after load_scenario_script.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/bin/scenario.sh` around lines 1854 - 1857, Update action_run around
emit_brew_alignment_artifact so SCENARIO_INFO_DIR is initialized for the current
scenario before the artifact is emitted. Set it from $1 before the call, or move
the call until after SCENARIO_SCRIPT/load_scenario_script establishes the
directory, while preserving the existing BREW_TEST_ALIGNED guard.
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
@copejon: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Summary
test/bin/brew_test_alignment.sh— extracts the source commit SHA from the brew RPM's NVR release field and checks outtest/suites/andtest/resources/at that commit before running release scenariosci_phase_boot_and_test.shwith a release-only guard (.*releases.*pattern), trap-based cleanup, and configurable skew threshold (MAX_BREW_SKEW, default 100 commits)brew-alignment.jsonsidecar artifact containing brew commit, HEAD commit, commit delta, and coverage delta (list of test files at HEAD but absent at the brew SHA)Fixes the false-failure problem where new Robot Framework tests reference features not yet present in brew-built RPMs, while keeping harness scripts and scenario definitions at HEAD so infrastructure improvements always apply.
Test plan
Brew source commit:,Commit delta:)brew-alignment.jsonartifact is written per scenarioMAX_BREW_SKEW=0causes hard failure🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes