-
Notifications
You must be signed in to change notification settings - Fork 51
🧪 Update workflow to run cypress tests for PRs/pushes #2770
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
WalkthroughAdds a repo-level GitHub Actions workflow that extracts test tags from PR bodies and invokes the reusable UI e2e workflow, deletes a prior tier0-specific workflow, and makes two display-name edits in the reusable e2e-run-ui-tests workflow. (46 words) Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant GH as GitHub Actions (repo workflow)
participant Extract as extract-test-tags job
participant Reusable as e2e-run-ui-tests (reusable workflow)
PR->>GH: push / PR affecting cypress/** triggers `e2e-repo-main`
GH->>Extract: run `extract-test-tags`
Extract->>Extract: parse PR body for "ci test tags: `@tag1`,`@tag2`,..." via regex
alt tags found
Extract->>GH: output test_tags (parsed)
else no tags
Extract->>GH: output test_tags = "@ci,`@tier0`"
end
GH->>Reusable: invoke reusable `e2e-run-ui-tests` with inputs (test_tags, feature_auth_required, bundle_image, install_konveyor_version, test_ref)
Reusable->>Reusable: execute UI e2e tests
Reusable-->>GH: return workflow result
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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: 0
🧹 Nitpick comments (1)
.github/workflows/e2e-repo-main.yaml (1)
34-35: Make the regex pattern more robust to whitespace variations.The pattern
@.+(,@.+)*requires tags to end exactly at the line boundary with no trailing whitespace. Consider trimming or handling trailing spaces to make the extraction more forgiving of common formatting variations (e.g., accidental trailing spaces after tags).After extracting the pattern, trim whitespace:
if [[ -n "$EXTRACTED" ]]; then echo "Found test tags in PR body: $EXTRACTED" - echo "test_tags=$EXTRACTED" >> "$GITHUB_OUTPUT" + EXTRACTED=$(echo "$EXTRACTED" | xargs) # Trim leading/trailing whitespace + echo "test_tags=$EXTRACTED" >> "$GITHUB_OUTPUT"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/e2e-repo-main.yaml(1 hunks).github/workflows/e2e-tier0-repo-main.yaml(0 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/e2e-tier0-repo-main.yaml
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: sjd78
Repo: konveyor/tackle2-ui PR: 2694
File: .github/workflows/e2e-tier0-push-main.yaml:7-8
Timestamp: 2025-11-13T23:21:33.345Z
Learning: In the tackle2-ui repository, the `.github/workflows/e2e-tier0-push-main.yaml` workflow is specifically designed to trigger only on changes to test code under `cypress/**`. Other separate workflows are responsible for running cypress tests when application code outside of `cypress/` changes. This separation of concerns is intentional.
Learnt from: sjd78
Repo: konveyor/tackle2-ui PR: 0
File: :0-0
Timestamp: 2025-12-04T17:50:41.607Z
Learning: When maintainers import tests into konveyor/tackle2-ui from tackle-ui-tests and ask to defer fixes, create a single severity-ordered tracking issue with PR and comment backlinks, assign to sjd78, and offer to split into categorized issues and add labels/milestones on request.
Learnt from: sjd78
Repo: konveyor/tackle2-ui PR: 0
File: :0-0
Timestamp: 2025-12-04T17:50:41.607Z
Learning: When a PR bulk-imports tests from an external repo in konveyor/tackle2-ui, maintainers prefer not to fix test issues in that PR. Instead, create a follow-up issue that catalogs potential issues in severity order with PR and comment backlinks, and assign to sjd78.
📚 Learning: 2025-11-13T23:21:33.345Z
Learnt from: sjd78
Repo: konveyor/tackle2-ui PR: 2694
File: .github/workflows/e2e-tier0-push-main.yaml:7-8
Timestamp: 2025-11-13T23:21:33.345Z
Learning: In the tackle2-ui repository, the `.github/workflows/e2e-tier0-push-main.yaml` workflow is specifically designed to trigger only on changes to test code under `cypress/**`. Other separate workflows are responsible for running cypress tests when application code outside of `cypress/` changes. This separation of concerns is intentional.
Applied to files:
.github/workflows/e2e-repo-main.yaml
📚 Learning: 2025-10-17T16:41:30.776Z
Learnt from: sjd78
Repo: konveyor/tackle2-ui PR: 2656
File: cypress/package.json:8-8
Timestamp: 2025-10-17T16:41:30.776Z
Learning: In the `cypress/package.json` file of the tackle2-ui repository, the npm scripts run with the current working directory set to `cypress/`, so relative paths in those scripts are resolved from the cypress folder, not the repository root.
Applied to files:
.github/workflows/e2e-repo-main.yaml
⏰ 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). (2)
- GitHub Check: unit-test
- GitHub Check: build-and-upload-for-global-ci
🔇 Additional comments (3)
.github/workflows/e2e-repo-main.yaml (3)
47-55: Verify the reusable workflow interface and parameter compatibility.The workflow invokes
.github/workflows/e2e-run-ui-tests.yamlwithtest_tagsas a new input. Confirm that the reusable workflow:
- Accepts the
test_tagsparameter- Correctly handles the extracted tag format (
@tag1,@tag2,...)- Maintains backward compatibility with existing hardcoded parameters (
feature_auth_required,bundle_image,install_konveyor_version,test_ref)
16-18: Update concurrency group name to reflect new workflow purpose.The concurrency group is named
e2e-tier0-repo-main, but this workflow now handles multiple test tags (not just tier0). This naming is confusing and suggests the workflow only runs tier0 tests. Since this replaces the dedicated tier0 workflow, update the group name to reflect its broader purpose.Apply this diff:
concurrency: - group: e2e-tier0-repo-main-${{ github.ref }} + group: e2e-repo-main-${{ github.ref }} cancel-in-progress: true⛔ Skipped due to learnings
Learnt from: sjd78 Repo: konveyor/tackle2-ui PR: 2694 File: .github/workflows/e2e-tier0-push-main.yaml:7-8 Timestamp: 2025-11-13T23:21:33.345Z Learning: In the tackle2-ui repository, the `.github/workflows/e2e-tier0-push-main.yaml` workflow is specifically designed to trigger only on changes to test code under `cypress/**`. Other separate workflows are responsible for running cypress tests when application code outside of `cypress/` changes. This separation of concerns is intentional.
26-45: The grep PCRE pattern at line 34 should function correctly for multi-line PR bodies as written. In PCRE regex (which grep -P uses),^and$are line anchors that match at line start and end positions by default when processing multi-line input—they are not limited to absolute string boundaries. The pattern will correctly match "ci test tags: ..." on any line of the PR body without requiring the(?m)flag.Likely an incorrect or invalid review comment.
This workflow allows PRs to specify test tags to use for the E2E tests in the PR body. By adding a line like this to the PR body: ``` ci test tags: @ci,@tier0,@Tier1 ``` the workflow will run the cypress tests for the ci, tier0 and tier1 tagged tests for this PR. When the workflow is run outside of a PR, or the PR body does not have the string, the workflow will use the default tags of `@ci,@tier0`. Part of: konveyor#2670 Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
3e037a7 to
a3a053f
Compare
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: 0
🧹 Nitpick comments (1)
.github/workflows/e2e-repo-main.yaml (1)
32-46: Consider refining the pattern to reject inputs with unexpected separators or spacing.The current pattern
@.+(,@.+)*is greedy and will match unexpected formats. For example, inputs like"ci test tags: @ci, @tier0"(with space after comma) or"ci test tags: @ ci"would match but represent invalid tag syntax. A more restrictive pattern like@[a-zA-Z0-9_-]+(,@[a-zA-Z0-9_-]+)*would validate that tags contain only alphanumeric characters, underscores, and hyphens, rejecting spaces and unexpected separators.- EXTRACTED=$(echo "$PR_BODY" | grep -oP '^ci test tags: \K@.+(,@.+)*$' || true) + EXTRACTED=$(echo "$PR_BODY" | grep -oP '^ci test tags: \K@[a-zA-Z0-9_-]+(,@[a-zA-Z0-9_-]+)*$' || true)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/e2e-repo-main.yaml(1 hunks).github/workflows/e2e-run-ui-tests.yaml(2 hunks).github/workflows/e2e-tier0-repo-main.yaml(0 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/e2e-tier0-repo-main.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/e2e-run-ui-tests.yaml
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: sjd78
Repo: konveyor/tackle2-ui PR: 2694
File: .github/workflows/e2e-tier0-push-main.yaml:7-8
Timestamp: 2025-11-13T23:21:33.345Z
Learning: In the tackle2-ui repository, the `.github/workflows/e2e-tier0-push-main.yaml` workflow is specifically designed to trigger only on changes to test code under `cypress/**`. Other separate workflows are responsible for running cypress tests when application code outside of `cypress/` changes. This separation of concerns is intentional.
Learnt from: sjd78
Repo: konveyor/tackle2-ui PR: 0
File: :0-0
Timestamp: 2025-12-04T17:50:41.607Z
Learning: When maintainers import tests into konveyor/tackle2-ui from tackle-ui-tests and ask to defer fixes, create a single severity-ordered tracking issue with PR and comment backlinks, assign to sjd78, and offer to split into categorized issues and add labels/milestones on request.
Learnt from: sjd78
Repo: konveyor/tackle2-ui PR: 0
File: :0-0
Timestamp: 2025-12-04T17:50:41.607Z
Learning: When a PR bulk-imports tests from an external repo in konveyor/tackle2-ui, maintainers prefer not to fix test issues in that PR. Instead, create a follow-up issue that catalogs potential issues in severity order with PR and comment backlinks, and assign to sjd78.
📚 Learning: 2025-11-13T23:21:33.345Z
Learnt from: sjd78
Repo: konveyor/tackle2-ui PR: 2694
File: .github/workflows/e2e-tier0-push-main.yaml:7-8
Timestamp: 2025-11-13T23:21:33.345Z
Learning: In the tackle2-ui repository, the `.github/workflows/e2e-tier0-push-main.yaml` workflow is specifically designed to trigger only on changes to test code under `cypress/**`. Other separate workflows are responsible for running cypress tests when application code outside of `cypress/` changes. This separation of concerns is intentional.
Applied to files:
.github/workflows/e2e-repo-main.yaml
📚 Learning: 2025-10-17T16:41:30.776Z
Learnt from: sjd78
Repo: konveyor/tackle2-ui PR: 2656
File: cypress/package.json:8-8
Timestamp: 2025-10-17T16:41:30.776Z
Learning: In the `cypress/package.json` file of the tackle2-ui repository, the npm scripts run with the current working directory set to `cypress/`, so relative paths in those scripts are resolved from the cypress folder, not the repository root.
Applied to files:
.github/workflows/e2e-repo-main.yaml
⏰ 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). (3)
🔇 Additional comments (2)
.github/workflows/e2e-repo-main.yaml (2)
48-56: Job orchestration and reusable workflow invocation look correct.The
e2e-testsjob properly depends onextract-test-tags, correctly extracts the output, and passes all necessary inputs to the reusable workflow. Thetest_reflogic correctly uses the PR head SHA for pull requests and the current commit SHA otherwise.Confirm that the
e2e-run-ui-tests.yamlworkflow accepts all the inputs being passed (particularlytest_tags), to ensure the downstream workflow properly processes the dynamically extracted tags.
27-46: No actionable concerns with the grep pattern implementation.The workflow correctly uses
grep -oPto extract test tags. While portability to arbitrary systems is a concern, GitHub'subuntu-latestrunner (22.04 LTS) reliably includes GNU grep with PCRE2 support. The regex pattern is sound and the|| truefallback is appropriate defensive coding. No changes needed.
| feature_auth_required: true | ||
| bundle_image: quay.io/konveyor/tackle2-operator-bundle:latest | ||
| install_konveyor_version: main | ||
| test_ref: ${{ github.event.pull_request.head.sha || github.sha }} |
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.
Looks like GitHub Actions expression language does not support || . This line will silently resolve incorrectly.
This might help :
test_ref: ${{ github.event.pull_request.head.sha != '' && github.event.pull_request.head.sha || github.sha }}
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.
Weird, the test_ref did get resolved correctly for this PR. For a PR, it should resolve to the sha of the HEAD commit.
I haven't run the workflow directly against a branch yet. Not sure I can do that while it is still only in a PR.
| run: | | ||
| if [[ -n "$PR_BODY" ]]; then | ||
| # Look for pattern "ci test tags: @tag1,@tag2,..." in PR body | ||
| EXTRACTED=$(echo "$PR_BODY" | grep -oP '^ci test tags: \K@.+(,@.+)*$' || true) |
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.
|
We can also extend this PR to support (PR #2744 ):
Example : If PR Body contains |
|
Modified workflow file , Please review. |
This workflow allows PRs to specify test tags to use for the E2E tests in the PR body. By adding a line like this to the PR body:
the workflow will run the cypress tests for the ci, tier0 and tier1 tagged tests for this PR.
When the workflow is run outside of a PR, or the PR body does not have the string, the workflow will use the default tags of
@ci,@tier0.Part of: #2670
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.