Fix e2e tests trigger #4850
Fix e2e tests trigger #4850
Conversation
Code Coverage - Integration Tests
|
Code Coverage - Frontend unit tests
Test suite run success5048 tests passing in 663 suites. Report generated by 🧪jest coverage report action from 12f2a87 |
Code Coverage - Backend unit tests
Test suite run success2950 tests passing in 286 suites. Report generated by 🧪jest coverage report action from 12f2a87 |
| builds-complete: | ||
| needs: [build-docker, build-appimage] |
There was a problem hiding this comment.
intermediate job to wait for both builds to complete (~5mins) to avoid running tests when the other build has failed and the whole workflow will be marked as failed in the end anyway
KrumTy
requested review from
dantovska,
ArtemHoruzhenko and
valkirilov
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR fixes E2E test triggering by replacing the previous label-based approach with approval-based triggering and deduplication. The solution addresses the issue where E2E tests weren't triggering automatically when PRs were approved, despite the e2e-approved label being added.
Key changes:
- Replace label-based triggering (
pull_request.labeled) with approval-based triggering (pull_request_review.submitted) - Implement deduplication logic to prevent multiple E2E runs for the same commit SHA
- Add a reusable workflow for approval and deduplication logic that other workflows can leverage
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/workflows/tests-e2e.yml |
Updated to use approval-based triggering and the new approval-dedupe workflow, restructured job dependencies |
.github/workflows/tests-e2e-approve.yml |
Removed entirely as its functionality is now handled by the main E2E workflow |
.github/workflows/approval-dedupe.yml |
New reusable workflow implementing approval gating and deduplication logic by PR head SHA |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| description: The workflow filename to inspect for prior runs (e.g., tests-e2e.yml) | ||
| required: true | ||
| type: string | ||
| default: tests-e2e.yml |
There was a problem hiding this comment.
The default value 'tests-e2e.yml' should not be hardcoded since this is meant to be a reusable workflow. Remove the default or make it empty to force callers to explicitly specify their workflow filename.
| default: tests-e2e.yml |
Copilot uses AI. Check for mistakes.
| let data = await poll() | ||
| const started = Date.now() | ||
| while (data.status !== 'completed') { | ||
| if (Date.now() - started > 60 * 60 * 1000) { |
There was a problem hiding this comment.
The timeout value of 60 * 60 * 1000 (1 hour) is a magic number. Consider extracting this to a workflow input parameter or at least add a comment explaining the timeout duration.
Copilot uses AI. Check for mistakes.
| core.setFailed('Timeout waiting for prior run') | ||
| return | ||
| } | ||
| await new Promise(r => setTimeout(r, 15000)) |
There was a problem hiding this comment.
The polling interval of 15000ms (15 seconds) is a magic number. Consider extracting this to a workflow input parameter or at least add a comment explaining the polling frequency.
Copilot uses AI. Check for mistakes.
Description
Currently, when a PR is approved for the first time, it is labeled as e2e-approved. And when a e2e-approved label is added, it should trigger e2e tests.
In reality what happens is when a PR is approved, the e2e-approved label is added, but the workflow for tests does not trigger. However if the PR is labeled manually via Github ui as e2e-approved, the tests do trigger.
Potential solution could be adding github secret to the label action to mimic authorized labelingSolution summary
This PR reworks how E2E tests are triggered:
E2E tests run only after a PR is approved (or when manually dispatched), per on commit sha