-
Notifications
You must be signed in to change notification settings - Fork 14
Update global-ci workflows to also be able to test UI run-ui-tests #178
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.
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
base: main
Are you sure you want to change the base?
Conversation
Since the global-ci will run for any PR that changes the run-ui-tests action in the tackle2-ui repository, we want to be able to use the those changes in the global-ci run. This is done by checking out the tackle2-ui repository at a specific ref and running the run-ui-tests action "locally". See CI fails in konveyor/tackle2-ui#2787 for the motivation for this change. Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
WalkthroughThe PR refactors two CI/CD workflows to replace environment variable outputs (GITHUB_ENV) with explicit step outputs (GITHUB_OUTPUT) for UI URL and test reference values. It adds dedicated extraction steps and checks out the tackle2-ui repository locally to use its run-ui-tests action instead of remotely referencing it. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/global-ci-bundle.yml (1)
489-489: Minor: Refine grep pattern for consistency with API tests extraction.The UI tests pattern at line 489 uses
:(colon + single space), while the analogous API tests pattern at line 287 uses:\s*(colon + optional whitespace). Consider aligning to allow variable whitespace for robustness.- PULL_REQUEST_NUMBER=$(echo ${body} | grep -oP '[U|u][I|i] [T|t]ests [P|p][R|r]: \K\d+' || true) + PULL_REQUEST_NUMBER=$(echo ${body} | grep -oP '[U|u][I|i] [T|t]ests [P|p][R|r]:\s*\K\d+' || true)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/global-ci-bundle.yml(2 hunks).github/workflows/global-ci.yml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Verify api and ui tests run via
global-ci-bundle.ymlwith default input / e2e-api-integration-tests - GitHub Check: Verify api and ui tests run via
global-ci-bundle.ymlwith default input / e2e-ui-integration-tests - GitHub Check: Verify api and ui tests run via
global-ci.ymlwith default inputs / e2e-ui-integration-tests - GitHub Check: Verify api and ui tests run via
global-ci.ymlwith default inputs / e2e-api-integration-tests
🔇 Additional comments (2)
.github/workflows/global-ci-bundle.yml (2)
470-481: Approve GITHUB_OUTPUT migration for UI URL output.The refactoring from GITHUB_ENV to explicit GITHUB_OUTPUT is correct. The step id
ui-urlis set, and the outputui_urlis properly consumed downstream at line 510. This aligns with workflow best practices.
483-495: Approve GITHUB_OUTPUT migration for UI tests ref output.The extraction of the PR reference is now properly exposed via GITHUB_OUTPUT with step id
ui-tests-refand output nameref, which is correctly consumed downstream. The documented format ("UI tests PR: 140") is now aligned with the grep pattern.
| - name: "Checkout run-ui-tests action (ref: ${{ inputs.ui_tests_ref }})" | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: konveyor/tackle2-ui | ||
| path: tackle2-ui | ||
| ref: ${{ inputs.ui_tests_ref }} | ||
| sparse-checkout: .github/actions/run-ui-tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Checkout ref should use extracted PR reference, not default input.
The checkout step at line 502 uses ${{ inputs.ui_tests_ref }} (which defaults to "main"), but the extraction step immediately prior extracts a potentially different ref from the PR description (for testing modifications to the run-ui-tests action itself). This defeats the purpose of the extraction—the locally checked-out action will always be at the default ref, not the PR-specific changes.
Apply this diff:
- name: "Checkout run-ui-tests action (ref: ${{ inputs.ui_tests_ref }})"
uses: actions/checkout@v4
with:
repository: konveyor/tackle2-ui
path: tackle2-ui
- ref: ${{ inputs.ui_tests_ref }}
+ ref: ${{ steps.ui-tests-ref.outputs.ref }}
sparse-checkout: .github/actions/run-ui-tests📝 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.
| - name: "Checkout run-ui-tests action (ref: ${{ inputs.ui_tests_ref }})" | |
| uses: actions/checkout@v4 | |
| with: | |
| repository: konveyor/tackle2-ui | |
| path: tackle2-ui | |
| ref: ${{ inputs.ui_tests_ref }} | |
| sparse-checkout: .github/actions/run-ui-tests | |
| - name: "Checkout run-ui-tests action (ref: ${{ inputs.ui_tests_ref }})" | |
| uses: actions/checkout@v4 | |
| with: | |
| repository: konveyor/tackle2-ui | |
| path: tackle2-ui | |
| ref: ${{ steps.ui-tests-ref.outputs.ref }} | |
| sparse-checkout: .github/actions/run-ui-tests |
🤖 Prompt for AI Agents
.github/workflows/global-ci-bundle.yml lines 497-503: the checkout step
currently uses the static input `${{ inputs.ui_tests_ref }}` instead of the ref
extracted from the PR earlier; change the `ref` to use the output produced by
the extraction step (for example
`steps.<extract-step-id>.outputs.<output-name>`) so the locally checked-out
konveyor/tackle2-ui uses the PR-specific ref, and add a fallback to `${{
inputs.ui_tests_ref }}` if the extraction output is empty.
| - name: "Checkout run-ui-tests action (ref: ${{ inputs.ui_tests_ref }})" | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: konveyor/tackle2-ui | ||
| path: tackle2-ui | ||
| ref: ${{ inputs.ui_tests_ref }} | ||
| sparse-checkout: .github/actions/run-ui-tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "global-ci.yml" -o -name "*.yml" 2>/dev/null | head -20Repository: konveyor/ci
Length of output: 37
🏁 Script executed:
find . -path "*/.github/workflows/global-ci.yml" 2>/dev/nullRepository: konveyor/ci
Length of output: 88
🏁 Script executed:
ls -la .github/workflows/ 2>/dev/null | head -20Repository: konveyor/ci
Length of output: 660
🏁 Script executed:
cat .github/workflows/global-ci.yml | head -100Repository: konveyor/ci
Length of output: 4149
🏁 Script executed:
sed -n '400,450p' .github/workflows/global-ci.ymlRepository: konveyor/ci
Length of output: 1950
🏁 Script executed:
sed -n '1,100p' .github/workflows/global-ci.yml | grep -A 20 "ui_tests_ref"Repository: konveyor/ci
Length of output: 1079
🏁 Script executed:
# Let's verify what tackle2-ui repository contains to understand the context better
git log --oneline -1 2>/dev/null || echo "Not a git repo, checking workflow structure"Repository: konveyor/ci
Length of output: 129
🏁 Script executed:
sed -n '419,431p' .github/workflows/global-ci.ymlRepository: konveyor/ci
Length of output: 661
Use extracted UI tests ref for tackle2-ui checkout instead of ui_tests_ref input.
Line 438 uses ${{ inputs.ui_tests_ref }} for the tackle2-ui checkout ref, but the extraction step (lines 419-431) parses the PR description to extract an overridden ref and outputs it to steps.ui-tests-ref.outputs.ref. The checkout ignores this extracted output and uses the input directly, preventing the PR override mechanism from working. When a PR description specifies "UI tests PR: 123", the extraction correctly identifies it, but the tackle2-ui checkout still uses the default input value instead of the extracted ref.
Update line 438 to use the extracted ref:
- name: "Checkout run-ui-tests action (ref: ${{ inputs.ui_tests_ref }})"
uses: actions/checkout@v4
with:
repository: konveyor/tackle2-ui
path: tackle2-ui
- ref: ${{ inputs.ui_tests_ref }}
+ ref: ${{ steps.ui-tests-ref.outputs.ref }}
sparse-checkout: .github/actions/run-ui-testsAlso update the step name to remove the now-inaccurate hardcoded input reference:
- name: "Checkout run-ui-tests action (ref: ${{ inputs.ui_tests_ref }})"
+ name: "Checkout run-ui-tests action"📝 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.
| - name: "Checkout run-ui-tests action (ref: ${{ inputs.ui_tests_ref }})" | |
| uses: actions/checkout@v4 | |
| with: | |
| repository: konveyor/tackle2-ui | |
| path: tackle2-ui | |
| ref: ${{ inputs.ui_tests_ref }} | |
| sparse-checkout: .github/actions/run-ui-tests | |
| - name: "Checkout run-ui-tests action" | |
| uses: actions/checkout@v4 | |
| with: | |
| repository: konveyor/tackle2-ui | |
| path: tackle2-ui | |
| ref: ${{ steps.ui-tests-ref.outputs.ref }} | |
| sparse-checkout: .github/actions/run-ui-tests |
🤖 Prompt for AI Agents
.github/workflows/global-ci.yml around lines 433 to 439: the checkout step for
konveyor/tackle2-ui currently uses the hardcoded input `${{ inputs.ui_tests_ref
}}` and a name that references that input; replace the ref with the extracted
output `steps.ui-tests-ref.outputs.ref` so the PR override works (i.e. set ref
to `steps.ui-tests-ref.outputs.ref`), and update the step name to remove the
hardcoded "ref: ${{ inputs.ui_tests_ref }}" wording so it accurately reflects
that it’s using the extracted UI tests ref.
Since the global-ci will run for any PR that changes the run-ui-tests action in the tackle2-ui repository, we want to be able to use the those changes in the global-ci run. This is done by checking out the tackle2-ui repository at a specific ref and running the run-ui-tests action "locally".
See CI fails in konveyor/tackle2-ui#2787 for the motivation for this change.
Also changed a few GITHUB_ENV output to use GITHUB_OUTPUT so my workflow linter can stop warning about using undefined env values.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.