Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
develop, master and release branches
Builds ready [3783ebf]
Page Load Metrics (250 ± 245 ms)
Bundle size diffs
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #25605 +/- ##
========================================
Coverage 69.60% 69.60%
========================================
Files 1364 1364
Lines 48172 48172
Branches 13291 13291
========================================
Hits 33526 33526
Misses 14646 14646 ☔ View full report in Codecov by Sentry. |
| } catch (error) { | ||
| console.error('Error reading from file:', error); | ||
| return ''; | ||
| return []; |
There was a problem hiding this comment.
returning an empty array, in order to handle properly the subsequent filter, when there is no file (in develop, master and RC branch)
| - /^Version-v(\d+)[.](\d+)[.](\d+)/ | ||
| requires: | ||
| - prep-build | ||
| - get-changed-files-with-git-diff |
There was a problem hiding this comment.
this is not needed here, since the vault decrypt is a special test, that only runs on develop and RC branches, and only runs this single test (doesn't use the run-all script)
| - /^Version-v(\d+)[.](\d+)[.](\d+)/ | ||
| requires: | ||
| - prep-build | ||
| - get-changed-files-with-git-diff |
There was a problem hiding this comment.
We can also remove get-changed-files-with-git-diff as a requirement for the test-e2e-firefox-flask and test-e2e-firefox-confirmation-redesign jobs, which also only run on develop/master/rc
There was a problem hiding this comment.
that's a great point! Removed too 👍
danjm
left a comment
There was a problem hiding this comment.
One more suggested change from me
| at: . | ||
| - run: | ||
| name: Get changed files with git diff | ||
| command: npx tsx .circleci/scripts/git-diff-develop.ts |
There was a problem hiding this comment.
For simpler and easier to maintain code, I think that instead of this code change to the command, we can do the following change above:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index d9f0754335..9fe95a7f6b 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -106,6 +106,12 @@ workflows:
- check-pr-tag
- prep-deps
- get-changed-files-with-git-diff:
+ filters:
+ branches:
+ ignore:
+ - develop
+ - master
+ - /^Version-v(\d+)[.](\d+)[.](\d+)/
requires:
- prep-deps
- test-deps-audit:
And the other jobs that depend on get-changed-files-with-git-diff should still run, see the docs on that points: "Note: When jobs in the current workflow that are listed as dependencies are not executed (due to a filter function for example), their requirement as a dependency for other jobs will be ignored by the requires option. However, if all dependencies of a job are filtered, then that job will not be executed either." https://circleci.com/docs/configuration-reference/#requires
There was a problem hiding this comment.
wooa that's what I was looking for! I fully agree with this approach. I initially had it like this, but couldn't find anywhere if the jobs requiring the skipped job would succeed. Pushing the changes now. Thank you!!
|
Builds ready [7d28ce5]
Page Load Metrics (166 ± 205 ms)
Bundle size diffs
|



Description
This PR excludes running the git-diff script for the e2e quality gate added here, for
developas well as formasterand release branches.We don't want to run this in develop, master or release branch, since there is no point on re-running the new/changed tests there, as the changes are already in
developbranch. This could add up more credits and also slow down ci in those branches.Context: in
developbranch the current quality gate fails, since thediffResultis empty, and we were throwing the following error as we were entering in the !diffResult condition.However, this PR fixes the issue on the higher level, by skipping entirely the git diff for the mentioned branches above.
Related issues
Fixes: current develop ci
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist