chore: skip passed CI tests and split workflow into reusable modules#3551
Open
chore: skip passed CI tests and split workflow into reusable modules#3551
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces large inline CI jobs with reusable workflow calls, adds deterministic source-state hashing and cache-restore gating via two composite actions, introduces multiple reusable test/build workflows (backend, frontend, e2e, checks), removes two deprecated preview/prerelease workflows, and changes the “everything passed” check to a GH-API job-list verification. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Runner
participant Check as check-source-cache (action)
participant Cache as actions/cache (restore/save)
participant Workflow as Reusable Workflow Job
participant Artifacts as Artifacts (upload/download)
participant GH as GitHub API (gh)
Runner->>Check: compute source SHA256 -> produce cache-key
Check->>Cache: restore `.test-passed` using cache-key
Cache-->>Check: cache-hit true/false
alt cache-hit == true
Check->>Workflow: indicate cache-hit -> skip heavy steps
Workflow->>Artifacts: download artifacts if needed
else cache-hit != true
Runner->>Workflow: run setup, build/download artifacts, execute tests/checks
Workflow->>Artifacts: upload reports/artifacts
Workflow->>Cache: save `.test-passed` using cache-key
end
Note over Runner,GH: Aggregate final verification
Runner->>GH: gh api /repos/:owner/:repo/actions/runs/:run_id/jobs
GH-->>Runner: job conclusions
Runner->>Runner: fail if any relevant job conclusion != success/skipped
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Around line 73-74: The cache/pass marker key currently uses only
matrix.command and steps.hash (see the key: test-passed-${{ matrix.command
}}-${{ steps.hash.outputs.hash }}), which allows runs with different execution
modes (e.g., CI_RELEASE=true for push vs pull_request) to collide; update the
key to include the execution mode so release vs PR runs are distinct (for
example append ${{ env.CI_RELEASE }} or ${{ github.event_name }} or a dedicated
matrix value like ${ { matrix.mode }}), and apply the same change to all other
occurrences (the identical keys around lines noted in the comment) so
backend-test and e2e release-mode runs cannot be skipped by a green PR run.
- Around line 53-66: The "Compute backend source hash" step (name: Compute
backend source hash, id: hash) currently uses `git ls-files -- . | git
hash-object --stdin-paths` which only hashes blob IDs and misses
renames/mode-only changes; replace that pipeline to use `git ls-files -s -- .`
so the mode, blob ID and path are included before piping to `sha256sum`/cut, and
mirror the exact same change in the e2e hash step so both cache keys include
mode/path information.
- Around line 68-74: The cache restore step using actions/cache/restore@v4 (step
name "Check if already passed", id "cache") should be made best-effort by adding
continue-on-error: true to that step; apply the same change to the other restore
step used for e2e (the other actions/cache/restore@v4 step) and consider adding
continue-on-error: true to the corresponding cache save steps as well so cache
outages don't block the job.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dbaa3832-dd73-4158-9016-f4873c103afb
📒 Files selected for processing (1)
.github/workflows/test.yml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
53-75: Consider extracting the hash/cache probe into one reusable action.The compute-hash → restore lookup → skip notice pattern is now copied into five jobs. Centralizing it would make future key-shape or path-scope fixes much less likely to drift between jobs.
Also applies to: 180-200, 318-339, 424-444, 514-536
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 53 - 75, Extract the repeated compute-hash + cache-restore pattern (the steps named "Compute backend source hash" with id "hash" and "Check if already passed" with id "cache", using the key template test-passed-${{ matrix.command }}-${{ github.event_name }}-${{ steps.hash.outputs.hash }} and path .test-passed) into a single reusable action or composite step (e.g., .github/actions/compute-and-restore) that: computes the SHA256 from git ls-files, exposes the hash as an output, attempts a lookup-only restore of .test-passed with the same key logic, and returns a boolean/cache-hit output; replace the five duplicate blocks in each job with a call to that reusable action and update callers to read the new outputs (e.g., steps.<new>.outputs.hash and steps.<new>.outputs.cache_hit) so key shape and path scope remain consistent across jobs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Around line 318-330: The "Compute frontend source hash" step (id: hash)
currently blacklists only backend/, ee/, and e2e/ but misses backend-only
modules like server-app, data, security and ee-test, causing frontend cache keys
to invalidate on unrelated backend changes; update this step to either switch to
an allowlist of frontend inputs (e.g., include only webapp/, library/, and
shared CI config files) or expand the exclusion list to explicitly omit
server-app/, data/, security/, ee-test/ (and any other non-frontend modules) so
the computed hash only reflects true frontend sources used by the
frontend-code-check job.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 53-75: Extract the repeated compute-hash + cache-restore pattern
(the steps named "Compute backend source hash" with id "hash" and "Check if
already passed" with id "cache", using the key template test-passed-${{
matrix.command }}-${{ github.event_name }}-${{ steps.hash.outputs.hash }} and
path .test-passed) into a single reusable action or composite step (e.g.,
.github/actions/compute-and-restore) that: computes the SHA256 from git
ls-files, exposes the hash as an output, attempts a lookup-only restore of
.test-passed with the same key logic, and returns a boolean/cache-hit output;
replace the five duplicate blocks in each job with a call to that reusable
action and update callers to read the new outputs (e.g.,
steps.<new>.outputs.hash and steps.<new>.outputs.cache_hit) so key shape and
path scope remain consistent across jobs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b535599c-e2b5-4592-9378-de0b8db55692
📒 Files selected for processing (1)
.github/workflows/test.yml
JanCizmar
changed the title
JanCizmar
force-pushed
the
jancizmar/skip-passed-tests
branch
from
4274fbd to
540d4b9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.github/workflows/test--backend-tests.yml (1)
55-57: Minor: Missingsuccess()check for consistency.The "Remove ee" step's condition doesn't include
success() &&, unlike other conditional steps in this workflow. If the "Download backend build result" step fails, this step would still execute. While removing theeedirectory when download failed is harmless, it's inconsistent with the pattern used elsewhere.♻️ Suggested fix for consistency
- name: Remove ee - if: steps.source-cache.outputs.cache-hit != 'true' && matrix.command == 'server-app:runWithoutEeTests' + if: success() && steps.source-cache.outputs.cache-hit != 'true' && matrix.command == 'server-app:runWithoutEeTests' run: rm -rf ./ee🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test--backend-tests.yml around lines 55 - 57, The "Remove ee" step currently checks "steps.source-cache.outputs.cache-hit != 'true' && matrix.command == 'server-app:runWithoutEeTests'"; update its if condition to match other steps by prepending success() && so it only runs on overall job success (i.e., change the condition for the "Remove ee" step to include success() && before the existing checks) to keep behavior consistent with the rest of the workflow..github/workflows/test--frontend-and-static-checks.yml (1)
91-101: Consider adding source-cache pattern to ktlint and other static checks.The
backend-code-checks,data-cy-check,e2e-code-checks, andemail-code-checksjobs run unconditionally without source-state caching, which is inconsistent with the caching strategy applied tofrontend-code-check. If these checks are fast enough that the cache overhead isn't worthwhile, this is fine—but it may be worth adding the pattern for consistency and to benefit from skipping on unchanged sources.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test--frontend-and-static-checks.yml around lines 91 - 101, The static-check jobs (backend-code-checks, data-cy-check, e2e-code-checks, email-code-checks) currently run unconditionally; mirror the frontend-code-check caching pattern by adding the same source-cache step/inputs to these jobs (for example, add an actions/cache restore/save step or reuse the ./.github/actions/setup-env input/step that frontend-code-check uses) so the "Run ktlint" step in backend-code-checks and equivalent steps in the other jobs will skip when source files are unchanged; update the job definitions to include the same cache key/pattern and restore/save behavior used by frontend-code-check to ensure consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release--publish.yml:
- Around line 11-14: Add validation for the workflow input
"docker-publish-method" so only "gradle" or "manual" are accepted and the run
fails otherwise; implement this by adding an early job/step (e.g., a small
"validate-input" step) that checks the value of
github.event.inputs.docker-publish-method and exits non-zero (or uses
actions/github-script to set failure) if it's not exactly "gradle" or "manual",
ensuring the conditional branches that reference that input (the docker
build/publish branches around docker-publish-method and the downstream "npm run
release" step) cannot proceed when an unknown value is supplied.
- Around line 145-153: The billing dispatch curl step ("Trigger Billing repo
tests & testing deploy") currently won't fail the job on HTTP errors because
curl defaults to exit 0 on 4xx/5xx; update the step that posts to
https://api.github.com/repos/tolgee/billing/actions/workflows/test.yml/dispatches
(using steps.version.outputs.VERSION and inputs.trigger-billing) to detect and
fail on HTTP errors — e.g., add curl's --fail/-f flag or capture the HTTP status
with -w '%{http_code}' and explicitly exit non‑zero when the status is not 2xx,
so authentication/API errors cause the workflow step to fail.
In @.github/workflows/release.yml:
- Around line 13-19: The reusable release workflow caller
(.github/workflows/release.yml) must forward the tested branch and commit SHA to
the callee so the publish workflow checks out the exact tested commit; add
inputs named head_branch and head_sha to the with: block and set them to
github.event.workflow_run.head_branch and github.event.workflow_run.head_sha
respectively, and ensure the reusable workflow release--publish.yml accepts
these input names (e.g., inputs head_branch/head_sha) so its checkout step can
use them instead of defaulting to the repo default branch.
In @.github/workflows/test--frontend-and-static-checks.yml:
- Around line 61-73: The workflow condition uses env.TOLGEE_API_KEY which is
evaluated before step-level env is applied, causing the "Load newest
translations" and "Check missing translation keys" steps to be skipped; fix by
setting TOLGEE_API_KEY at the job level (so env.TOLGEE_API_KEY is available
during condition evaluation) and remove the redundant step-level env entries for
those steps; update the job-level env to include TOLGEE_API_KEY: ${{
secrets.TOLGEE_API_KEY }} so the if conditions on those step names work as
intended.
---
Nitpick comments:
In @.github/workflows/test--backend-tests.yml:
- Around line 55-57: The "Remove ee" step currently checks
"steps.source-cache.outputs.cache-hit != 'true' && matrix.command ==
'server-app:runWithoutEeTests'"; update its if condition to match other steps by
prepending success() && so it only runs on overall job success (i.e., change the
condition for the "Remove ee" step to include success() && before the existing
checks) to keep behavior consistent with the rest of the workflow.
In @.github/workflows/test--frontend-and-static-checks.yml:
- Around line 91-101: The static-check jobs (backend-code-checks, data-cy-check,
e2e-code-checks, email-code-checks) currently run unconditionally; mirror the
frontend-code-check caching pattern by adding the same source-cache step/inputs
to these jobs (for example, add an actions/cache restore/save step or reuse the
./.github/actions/setup-env input/step that frontend-code-check uses) so the
"Run ktlint" step in backend-code-checks and equivalent steps in the other jobs
will skip when source files are unchanged; update the job definitions to include
the same cache key/pattern and restore/save behavior used by frontend-code-check
to ensure consistency.
🪄 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: 7a7288a8-13f0-42e7-9eab-91cea628bbc8
📒 Files selected for processing (14)
.github/actions/check-source-cache/action.yaml.github/actions/save-source-cache/action.yaml.github/workflows/prerelease-alpha.yml.github/workflows/preview.yml.github/workflows/release--publish.yml.github/workflows/release.yml.github/workflows/test--backend-checks.yml.github/workflows/test--backend-tests.yml.github/workflows/test--build-backend.yml.github/workflows/test--build-frontend.yml.github/workflows/test--e2e-install-deps.yml.github/workflows/test--e2e-tests.yml.github/workflows/test--frontend-and-static-checks.yml.github/workflows/test.yml
💤 Files with no reviewable changes (2)
- .github/workflows/prerelease-alpha.yml
- .github/workflows/preview.yml
✅ Files skipped from review due to trivial changes (3)
- .github/workflows/test--build-backend.yml
- .github/workflows/test--e2e-install-deps.yml
- .github/actions/save-source-cache/action.yaml
dkrizan
reviewed
Apr 8, 2026
| total_jobs: [15] | ||
| job_index: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14] | ||
| steps: | ||
| - uses: actions/checkout@v4 |
Member
There was a problem hiding this comment.
Following steps are duplicated in multiple workflows, maybe it can be optimized.
actions/checkout@v4
Check source cache
Setup environment
Download backend build result
Member
Author
There was a problem hiding this comment.
Each job runs on a separate runner, so checkout, setup-env, and download-backend-build need to happen per job — there's no way around that. The check-source-cache and save-source-cache steps are already extracted into composite actions to minimize the duplication. The remaining per-job boilerplate is the minimum each runner needs.
Member
There was a problem hiding this comment.
I understand their need, but I was pointing out the code duplication.
Compute a hash of relevant source files for each backend-test and e2e matrix job. On success, store a marker in GitHub Actions cache keyed by job ID + hash. On subsequent runs with the same source state, the cache hit skips all expensive steps (env setup, artifact download, test execution) — the job completes in seconds instead of minutes. Backend tests ignore frontend/e2e/docs files in the hash; E2E tests hash everything except docs and config files. New files trigger tests by default (safe). Cache entries expire after 7 days (GitHub default). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use `git ls-files -s` instead of `git hash-object --stdin-paths` to include file modes and paths in the hash (detects renames/mode changes) - Add `continue-on-error: true` to cache restore/save steps so cache service outages don't fail the job - Include `github.event_name` in cache key so PR runs don't suppress push/release runs that use CI_RELEASE=true Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… cache hit Apply the same source-hash caching pattern to three more expensive jobs: - frontend-code-check: hashes frontend/library sources (ignores backend/e2e) - schema-check: hashes all sources (sensitive to both backend + frontend) - migration-check: hashes backend sources only (same as backend tests) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Create check-source-cache composite action (hash computation + cache lookup + skip notice) - Create save-source-cache composite action (marker creation + cache save) - Move backend test matrix to _backend-tests.yml reusable workflow - Move e2e test matrix to _e2e-tests.yml reusable workflow - Main test.yml now orchestrates builds, reusable workflows, and inline checks — down from 645 to 459 lines Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move frontend-code-check + ktlint + data-cy + e2e-static + email checks to _frontend-and-static-checks.yml - Move schema-check + migration-check to _backend-checks.yml - Replace verbose 12-block everything-passed with toJSON(needs) + jq - test.yml is now a 132-line orchestrator (down from 645) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of parsing the needs context with jq, query the GitHub API for all jobs in the current run attempt. This catches failures even if a new job is added but not listed in the needs array. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
test.yml is now a 73-line pure orchestrator. All job implementations live in .github/workflows/test/: - build-backend.yml, build-frontend.yml, e2e-install-deps.yml - backend-tests.yml, e2e-tests.yml - frontend-and-static-checks.yml, backend-checks.yml Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Delete preview.yml (no longer used) - Create release/publish.yml reusable workflow with the shared build, docker publish, and release steps - release.yml and prerelease-alpha.yml are now slim callers (19 and 13 lines) that pass configuration inputs - Update action versions from v3 to v4 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Allows callers to include additional data in the hash (e.g. another repo's commit SHA for multi-repo setups like billing). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GitHub Actions does not support reusable workflows in subdirectories of .github/workflows/. Move them to flat files with test-- and release-- prefixes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prevents caching the pass marker when tests actually failed. Without this, a failed test would save the marker and subsequent runs would incorrectly skip it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Needed for billing repo where tolgee-platform is checked out to a subdirectory, not the workspace root. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dkrizan
force-pushed
the
jancizmar/skip-passed-tests
branch
from
39c0a31 to
dc0de1b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
How source caching works
Backend tests ignore frontend/e2e/docs files in the hash; E2E tests hash everything except docs/config. New files always trigger tests (safe default). Cache entries auto-expire after 7 days.
Cross-repo reuse
The check-source-cache and save-source-cache composite actions are also used by the billing repo: https://github.com/tolgee/billing/pull/248
Test plan
Summary by CodeRabbit