chore: Run CI on changed files; add retries & gating#1614
chore: Run CI on changed files; add retries & gating#1614edwinjosechittilappilly wants to merge 1 commit into
Conversation
WalkthroughThis PR unifies four GitHub Actions workflows ( ChangesCI/CD Workflow Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.github/workflows/lint-backend.yml:
- Around line 119-131: The Verify lint result step currently only checks ${{
needs.lint.result }} so a skipped lint due to a failing changes job can be
overlooked; update that step to also inspect ${{ needs.changes.result }} (from
the 'changes' job) by capturing both into variables (e.g. result_lint and
result_changes) and failing the step (exit 1) if either equals "failure" or
"cancelled", otherwise print a passing message; keep the step name "Verify lint
result" and update the conditional logic accordingly.
In @.github/workflows/lint-frontend.yml:
- Around line 55-67: The guard in the required job only checks
needs.biome.result and misses failures from the changes job; update the run
script in the required job (the steps under the job named "required" that
currently reads needs.biome.result) to also inspect needs.changes.result (or the
appropriate needs.<job>.result for your changes job) and treat "failure" or
"cancelled" from either job as a failing condition—i.e., fetch both results, if
either equals "failure" or "cancelled" echo a descriptive message and exit 1,
otherwise continue and print a passing message.
In @.github/workflows/test-e2e.yml:
- Around line 196-208: The verification step in the "required" job only checks
needs.e2e.result and ignores the outcome of the needs.changes job, allowing a
failed changes job to still show green; update the Verify e2e result step (the
script in the "required" job) to also read and validate needs.changes.result (in
addition to needs.e2e.result) and fail if either result is "failure" or
"cancelled" — ensure the script references both needs.e2e.result and
needs.changes.result and returns non‑zero when any of those indicate
failure/cancellation.
In @.github/workflows/test-integration.yml:
- Around line 150-162: The 'required' job only inspects needs.tests.result so it
can pass if 'changes' failed and caused 'tests' to be skipped; update the job
named "required" to depend on the changes job and/or explicitly check its
result: add changes to the job's needs (e.g., needs: [tests, changes]) and
modify the shell check to consider both needs.tests.result and
needs.changes.result (treat failure or cancelled for either as a failing
condition) so the job correctly fails when changes fails.
- Around line 34-35: The globs 'src/**.py' and 'tests/**.py' do not match nested
files; update the workflow to use recursive globs by replacing those patterns
with 'src/**/*.py' and 'tests/**/*.py' so changes in nested directories (e.g.
src/a/b.py) trigger the integration tests; modify the patterns wherever
'src/**.py' and 'tests/**.py' appear in the test-integration.yml workflow.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ecedfa54-67fa-45e4-a19b-c5c65f46139b
📒 Files selected for processing (4)
.github/workflows/lint-backend.yml.github/workflows/lint-frontend.yml.github/workflows/test-e2e.yml.github/workflows/test-integration.yml
| required: | ||
| needs: lint | ||
| if: always() | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Verify lint result | ||
| run: | | ||
| result="${{ needs.lint.result }}" | ||
| if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then | ||
| echo "lint job did not succeed: $result" | ||
| exit 1 | ||
| fi | ||
| echo "lint result: $result (success or skipped — passing)" |
There was a problem hiding this comment.
Include changes result in the required gate.
required only validates needs.lint.result. If changes fails, lint is skipped and this job still passes, which can hide a real CI failure.
Proposed fix
required:
- needs: lint
+ needs: [changes, lint]
if: always()
runs-on: ubuntu-latest
steps:
- name: Verify lint result
run: |
- result="${{ needs.lint.result }}"
- if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then
- echo "lint job did not succeed: $result"
+ changes_result="${{ needs.changes.result }}"
+ lint_result="${{ needs.lint.result }}"
+ if [ "$changes_result" = "failure" ] || [ "$changes_result" = "cancelled" ]; then
+ echo "changes job did not succeed: $changes_result"
+ exit 1
+ fi
+ if [ "$lint_result" = "failure" ] || [ "$lint_result" = "cancelled" ]; then
+ echo "lint job did not succeed: $lint_result"
exit 1
fi
- echo "lint result: $result (success or skipped — passing)"
+ echo "changes=$changes_result, lint=$lint_result (success or skipped — passing)"📝 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.
| required: | |
| needs: lint | |
| if: always() | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Verify lint result | |
| run: | | |
| result="${{ needs.lint.result }}" | |
| if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then | |
| echo "lint job did not succeed: $result" | |
| exit 1 | |
| fi | |
| echo "lint result: $result (success or skipped — passing)" | |
| required: | |
| needs: [changes, lint] | |
| if: always() | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Verify lint result | |
| run: | | |
| changes_result="${{ needs.changes.result }}" | |
| lint_result="${{ needs.lint.result }}" | |
| if [ "$changes_result" = "failure" ] || [ "$changes_result" = "cancelled" ]; then | |
| echo "changes job did not succeed: $changes_result" | |
| exit 1 | |
| fi | |
| if [ "$lint_result" = "failure" ] || [ "$lint_result" = "cancelled" ]; then | |
| echo "lint job did not succeed: $lint_result" | |
| exit 1 | |
| fi | |
| echo "changes=$changes_result, lint=$lint_result (success or skipped — passing)" |
🤖 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 @.github/workflows/lint-backend.yml around lines 119 - 131, The Verify lint
result step currently only checks ${{ needs.lint.result }} so a skipped lint due
to a failing changes job can be overlooked; update that step to also inspect ${{
needs.changes.result }} (from the 'changes' job) by capturing both into
variables (e.g. result_lint and result_changes) and failing the step (exit 1) if
either equals "failure" or "cancelled", otherwise print a passing message; keep
the step name "Verify lint result" and update the conditional logic accordingly.
| required: | ||
| needs: biome | ||
| if: always() | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Verify biome result | ||
| run: | | ||
| result="${{ needs.biome.result }}" | ||
| if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then | ||
| echo "biome job did not succeed: $result" | ||
| exit 1 | ||
| fi | ||
| echo "biome result: $result (success or skipped — passing)" |
There was a problem hiding this comment.
Fail the required job when changes fails.
Current guard only inspects needs.biome.result. A failed changes job can still yield a green required result via skipped biome.
Proposed fix
required:
- needs: biome
+ needs: [changes, biome]
if: always()
runs-on: ubuntu-latest
steps:
- name: Verify biome result
run: |
- result="${{ needs.biome.result }}"
- if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then
- echo "biome job did not succeed: $result"
+ changes_result="${{ needs.changes.result }}"
+ biome_result="${{ needs.biome.result }}"
+ if [ "$changes_result" = "failure" ] || [ "$changes_result" = "cancelled" ]; then
+ echo "changes job did not succeed: $changes_result"
+ exit 1
+ fi
+ if [ "$biome_result" = "failure" ] || [ "$biome_result" = "cancelled" ]; then
+ echo "biome job did not succeed: $biome_result"
exit 1
fi
- echo "biome result: $result (success or skipped — passing)"
+ echo "changes=$changes_result, biome=$biome_result (success or skipped — passing)"📝 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.
| required: | |
| needs: biome | |
| if: always() | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Verify biome result | |
| run: | | |
| result="${{ needs.biome.result }}" | |
| if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then | |
| echo "biome job did not succeed: $result" | |
| exit 1 | |
| fi | |
| echo "biome result: $result (success or skipped — passing)" | |
| required: | |
| needs: [changes, biome] | |
| if: always() | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Verify biome result | |
| run: | | |
| changes_result="${{ needs.changes.result }}" | |
| biome_result="${{ needs.biome.result }}" | |
| if [ "$changes_result" = "failure" ] || [ "$changes_result" = "cancelled" ]; then | |
| echo "changes job did not succeed: $changes_result" | |
| exit 1 | |
| fi | |
| if [ "$biome_result" = "failure" ] || [ "$biome_result" = "cancelled" ]; then | |
| echo "biome job did not succeed: $biome_result" | |
| exit 1 | |
| fi | |
| echo "changes=$changes_result, biome=$biome_result (success or skipped — passing)" |
🤖 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 @.github/workflows/lint-frontend.yml around lines 55 - 67, The guard in the
required job only checks needs.biome.result and misses failures from the changes
job; update the run script in the required job (the steps under the job named
"required" that currently reads needs.biome.result) to also inspect
needs.changes.result (or the appropriate needs.<job>.result for your changes
job) and treat "failure" or "cancelled" from either job as a failing
condition—i.e., fetch both results, if either equals "failure" or "cancelled"
echo a descriptive message and exit 1, otherwise continue and print a passing
message.
| required: | ||
| needs: e2e | ||
| if: always() | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Verify e2e result | ||
| run: | | ||
| result="${{ needs.e2e.result }}" | ||
| if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then | ||
| echo "e2e job did not succeed: $result" | ||
| exit 1 | ||
| fi | ||
| echo "e2e result: $result (success or skipped — passing)" |
There was a problem hiding this comment.
required should validate both changes and e2e outcomes.
Right now a failed changes job can still surface as green because only needs.e2e.result is evaluated.
Proposed fix
required:
- needs: e2e
+ needs: [changes, e2e]
if: always()
runs-on: ubuntu-latest
steps:
- name: Verify e2e result
run: |
- result="${{ needs.e2e.result }}"
- if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then
- echo "e2e job did not succeed: $result"
+ changes_result="${{ needs.changes.result }}"
+ e2e_result="${{ needs.e2e.result }}"
+ if [ "$changes_result" = "failure" ] || [ "$changes_result" = "cancelled" ]; then
+ echo "changes job did not succeed: $changes_result"
+ exit 1
+ fi
+ if [ "$e2e_result" = "failure" ] || [ "$e2e_result" = "cancelled" ]; then
+ echo "e2e job did not succeed: $e2e_result"
exit 1
fi
- echo "e2e result: $result (success or skipped — passing)"
+ echo "changes=$changes_result, e2e=$e2e_result (success or skipped — passing)"🤖 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 @.github/workflows/test-e2e.yml around lines 196 - 208, The verification step
in the "required" job only checks needs.e2e.result and ignores the outcome of
the needs.changes job, allowing a failed changes job to still show green; update
the Verify e2e result step (the script in the "required" job) to also read and
validate needs.changes.result (in addition to needs.e2e.result) and fail if
either result is "failure" or "cancelled" — ensure the script references both
needs.e2e.result and needs.changes.result and returns non‑zero when any of those
indicate failure/cancellation.
| - 'src/**.py' | ||
| - 'tests/**.py' |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In picomatch (used by dorny/paths-filter), does src/.pymatch nested paths likesrc/a/b.py, or is src//*.py required for recursive matching?
💡 Result:
In picomatch, src/.py does NOT match nested paths like src/a/b.py; you need src//*.py for recursive matching. Reason (per picomatch glob semantics): * matches any characters excluding path separators, while is the special “globstar” that can match across path separators. The picomatch docs explicitly note that `*` won’t match `a/b.js` (it returns false for `a/b.js`) and that is what matches path separators when used appropriately (nested directories require ``). [1][2] So, src/.py is effectively “`src/` + any non-separator chars + `.py`”, not “`src/` + any number of nested directories + `.py`”. [1] For dorny/paths-filter: since it evaluates path expressions using picomatch, the same matching rules apply there. [3]
Citations:
- 1: https://www.npmjs.com/package/picomatch
- 2: https://github.com/ultramatch/picomatch
- 3: https://stackoverflow.com/questions/63422281/use-glob-to-match-file-extensions-based-on-recursive-parameter
Use recursive Python globs that match nested files.
src/**.py and tests/**.py do not match nested files like src/a/b.py with picomatch semantics (used by dorny/paths-filter). This causes integration tests to be skipped when changes occur in nested directories. Use src/**/*.py and tests/**/*.py for proper recursive matching.
Proposed fix
filters: |
relevant:
- - 'src/**.py'
- - 'tests/**.py'
+ - 'src/**/*.py'
+ - 'tests/**/*.py'
- 'pyproject.toml'
- 'uv.lock'
- 'sdks/**'📝 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.
| - 'src/**.py' | |
| - 'tests/**.py' | |
| - 'src/**/*.py' | |
| - 'tests/**/*.py' |
🤖 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 @.github/workflows/test-integration.yml around lines 34 - 35, The globs
'src/**.py' and 'tests/**.py' do not match nested files; update the workflow to
use recursive globs by replacing those patterns with 'src/**/*.py' and
'tests/**/*.py' so changes in nested directories (e.g. src/a/b.py) trigger the
integration tests; modify the patterns wherever 'src/**.py' and 'tests/**.py'
appear in the test-integration.yml workflow.
| required: | ||
| needs: tests | ||
| if: always() | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Verify tests result | ||
| run: | | ||
| # For PRs, always build locally since we're testing new code | ||
| # For workflow_dispatch, use the input (defaults to true) | ||
| USE_LOCAL="${{ inputs.use_local_images }}" | ||
| if [ "${{ github.event_name }}" == "pull_request" ] || [ "$USE_LOCAL" != "false" ]; then | ||
| echo "Running tests with locally built images..." | ||
| make test-ci-local | ||
| else | ||
| echo "Running tests with DockerHub images..." | ||
| make test-ci | ||
| result="${{ needs.tests.result }}" | ||
| if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then | ||
| echo "tests job did not succeed: $result" | ||
| exit 1 | ||
| fi | ||
| echo "Keys directory after tests:" | ||
| ls -la keys/ || echo "No keys directory" | ||
| echo "tests result: $result (success or skipped — passing)" |
There was a problem hiding this comment.
Prevent required from passing when changes fails.
required only checks needs.tests.result; if changes fails, tests can be skipped and this job still passes.
Proposed fix
required:
- needs: tests
+ needs: [changes, tests]
if: always()
runs-on: ubuntu-latest
steps:
- name: Verify tests result
run: |
- result="${{ needs.tests.result }}"
- if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then
- echo "tests job did not succeed: $result"
+ changes_result="${{ needs.changes.result }}"
+ tests_result="${{ needs.tests.result }}"
+ if [ "$changes_result" = "failure" ] || [ "$changes_result" = "cancelled" ]; then
+ echo "changes job did not succeed: $changes_result"
+ exit 1
+ fi
+ if [ "$tests_result" = "failure" ] || [ "$tests_result" = "cancelled" ]; then
+ echo "tests job did not succeed: $tests_result"
exit 1
fi
- echo "tests result: $result (success or skipped — passing)"
+ echo "changes=$changes_result, tests=$tests_result (success or skipped — passing)"🤖 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 @.github/workflows/test-integration.yml around lines 150 - 162, The
'required' job only inspects needs.tests.result so it can pass if 'changes'
failed and caused 'tests' to be skipped; update the job named "required" to
depend on the changes job and/or explicitly check its result: add changes to the
job's needs (e.g., needs: [tests, changes]) and modify the shell check to
consider both needs.tests.result and needs.changes.result (treat failure or
cancelled for either as a failing condition) so the job correctly fails when
changes fails.
Update GitHub Actions workflows (lint-backend, lint-frontend, test-e2e, test-integration) to only run on relevant changed files by adding a preliminary `changes` job that uses dorny/paths-filter and exposes a run flag. Introduce merge_group support and add lightweight `required` verifier jobs so skipped jobs report as passing to the merge queue. Serialize heavy tests (E2E and Integration) with a shared concurrency group and wrap Playwright and integration test steps with nick-fields/retry to add timeouts, retries, and cleanup/on-retry teardown logic. Misc: gate jobs on the changes output, small whitespace/checkout fixes, and improved logging for decision paths.
edwinjosechittilappilly
force-pushed
the
fix-strict-pipeline
branch
from
4c4116b to
ff48340
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/lint-frontend.yml (1)
55-74:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winApply the same conditional execution pattern to the typecheck job.
The
typecheckjob is not gated on thechangesoutput, so it will always run even when no frontend files have changed. This is inconsistent with the conditional execution pattern applied to thebiomejob and defeats the core purpose of this PR.🔧 Proposed fix
typecheck: + needs: changes + if: needs.changes.outputs.run == 'true' name: TypeScript type check runs-on: ubuntu-latest🤖 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 @.github/workflows/lint-frontend.yml around lines 55 - 74, The typecheck job currently always runs; add the same conditional gating used by the biome job so typecheck only runs when frontend changes were detected. Update the "typecheck" job (name/typecheck) to include the same if: expression used by the biome job (e.g. if: ${{ steps.files.outputs.changes == 'true' }} or the equivalent outputs variable your workflow uses) so the job is skipped when no frontend files changed.
♻️ Duplicate comments (1)
.github/workflows/lint-frontend.yml (1)
75-87:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winValidate all upstream jobs in the required gate.
The
requiredjob has two critical gaps:
Missing
changesjob validation (flagged in past review): If thechangesjob fails,biomeis skipped, andrequiredsees "skipped" and passes—allowing a failed workflow to report success.Missing
typecheckjob validation: Thetypecheckjob is not validated at all. If type checking fails, the workflow will still pass because onlybiomeis checked.Both gaps violate the PR's goal of preventing silent failures.
🛡️ Proposed fix to validate all jobs
required: - needs: biome + needs: [changes, biome, typecheck] if: always() runs-on: ubuntu-latest steps: - - name: Verify biome result + - name: Verify all job results run: | - result="${{ needs.biome.result }}" - if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then - echo "biome job did not succeed: $result" + changes_result="${{ needs.changes.result }}" + biome_result="${{ needs.biome.result }}" + typecheck_result="${{ needs.typecheck.result }}" + + if [ "$changes_result" = "failure" ] || [ "$changes_result" = "cancelled" ]; then + echo "changes job did not succeed: $changes_result" + exit 1 + fi + if [ "$biome_result" = "failure" ] || [ "$biome_result" = "cancelled" ]; then + echo "biome job did not succeed: $biome_result" + exit 1 + fi + if [ "$typecheck_result" = "failure" ] || [ "$typecheck_result" = "cancelled" ]; then + echo "typecheck job did not succeed: $typecheck_result" exit 1 fi - echo "biome result: $result (success or skipped — passing)" + echo "changes=$changes_result, biome=$biome_result, typecheck=$typecheck_result (all success or skipped — passing)"🤖 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 @.github/workflows/lint-frontend.yml around lines 75 - 87, Update the required job's verification step to validate all upstream jobs instead of only `biome`: check `needs.biome.result`, `needs.changes.result`, and `needs.typecheck.result`, and fail the job if any of those results is not "success" (treat "failure", "cancelled", or "skipped" as a non-success state). Replace the single `result="${{ needs.biome.result }}"` check with a loop or explicit checks that reads each `needs.<job>.result` and exits with an error and a clear message if any is not "success", ensuring `required` cannot pass when `changes` or `typecheck` (or `biome`) did not succeed.
🤖 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.
Outside diff comments:
In @.github/workflows/lint-frontend.yml:
- Around line 55-74: The typecheck job currently always runs; add the same
conditional gating used by the biome job so typecheck only runs when frontend
changes were detected. Update the "typecheck" job (name/typecheck) to include
the same if: expression used by the biome job (e.g. if: ${{
steps.files.outputs.changes == 'true' }} or the equivalent outputs variable your
workflow uses) so the job is skipped when no frontend files changed.
---
Duplicate comments:
In @.github/workflows/lint-frontend.yml:
- Around line 75-87: Update the required job's verification step to validate all
upstream jobs instead of only `biome`: check `needs.biome.result`,
`needs.changes.result`, and `needs.typecheck.result`, and fail the job if any of
those results is not "success" (treat "failure", "cancelled", or "skipped" as a
non-success state). Replace the single `result="${{ needs.biome.result }}"`
check with a loop or explicit checks that reads each `needs.<job>.result` and
exits with an error and a clear message if any is not "success", ensuring
`required` cannot pass when `changes` or `typecheck` (or `biome`) did not
succeed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 95e7e81d-7a9c-4ff7-9407-ba957ffe5eff
📒 Files selected for processing (4)
.github/workflows/lint-backend.yml.github/workflows/lint-frontend.yml.github/workflows/test-e2e.yml.github/workflows/test-integration.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/lint-backend.yml
- .github/workflows/test-e2e.yml
- .github/workflows/test-integration.yml
|
@coderabbitai create a note on why this PR is relevant and draft a proposal also. |
|
Tip For best results, initiate chat on the files or code changes.
📋 Note: Why PR
|
| Workflow | Changes |
|---|---|
lint-backend.yml |
Add changes + required jobs, remove paths trigger |
lint-frontend.yml |
Add merge_group trigger, changes + required jobs |
test-e2e.yml |
Add changes + required jobs, concurrency group, retry wrapper |
test-integration.yml |
Add changes + required jobs, concurrency group, retry wrapper |
Benefits
| Benefit | Before | After |
|---|---|---|
| Unnecessary CI runs | All jobs run on every push | Only runs when relevant files change |
| Flaky test failures | Single attempt, hard fail | Up to 2 retries with clean state |
| Merge queue compatibility | Skipped jobs block merge queue | required jobs always report status |
| Heavy test parallelism | May run concurrently | Serialized per branch ref |
Risks & Mitigations
| Risk | Mitigation |
|---|---|
paths-filter missing a relevant path pattern |
Patterns should be reviewed carefully; merge_group always forces run=true as a safety net |
| Retry masking real failures | Max 2 attempts only; teardown between retries ensures clean state |
| Concurrency group causing long queue waits | cancel-in-progress: false ensures no run is silently dropped |
Recommendation
✅ Approve and merge this PR. It establishes a solid foundation for scalable and developer-friendly CI with minimal risk, and directly unblocks reliable merge queue usage.
Note to reviewers: Pay particular attention to the
paths-filterpatterns in eachchangesjob — these determine which file changes trigger which jobs and should be validated against the actual project layout.
Update GitHub Actions workflows (lint-backend, lint-frontend, test-e2e, test-integration) to only run on relevant changed files by adding a preliminary
changesjob that uses dorny/paths-filter and exposes a run flag. Introduce merge_group support and add lightweightrequiredverifier jobs so skipped jobs report as passing to the merge queue. Serialize heavy tests (E2E and Integration) with a shared concurrency group and wrap Playwright and integration test steps with nick-fields/retry to add timeouts, retries, and cleanup/on-retry teardown logic. Misc: gate jobs on the changes output, small whitespace/checkout fixes, and improved logging for decision paths.Summary by CodeRabbit