Skip to content
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.

Sign up for GitHub

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

Jump to bottom

chore: exclude running git diff job for the e2e quality gate in develop, master and release branches #25605

Merged
merged 14 commits into from
Jul 2, 2024
Merged

chore: exclude running git diff job for the e2e quality gate in develop, master and release branches #25605

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
59ce1be
ignore quality gate for develop, master and rc branches
seaona Jul 1, 2024
afa75f9
add requires
seaona Jul 1, 2024
37a05ec
fix develop git diff
seaona Jul 1, 2024
cdf6025
add check for ci branch
seaona Jul 1, 2024
8aac280
remove error condition with git diff and just return the output
seaona Jul 1, 2024
9e19f5b
do not run git diff in develop, master and release branches
seaona Jul 1, 2024
cfc8f72
exclude develop master and rc for git diff
seaona Jul 1, 2024
0ada3e8
Merge branch 'develop' into chore-quality-gate-ignore-branches
seaona Jul 1, 2024
82ef0f0
Merge branch 'develop' into chore-quality-gate-ignore-branches
seaona Jul 1, 2024
3783ebf
return empty array for empty file
seaona Jul 1, 2024
726f86b
clean up
seaona Jul 2, 2024
b3f4510
fix requires job
seaona Jul 2, 2024
e697d96
do not run in test-e2e-firefox-confirmation-redesign and firefox-flask
seaona Jul 2, 2024
7d28ce5
apply filter on workflow level
seaona Jul 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ workflows:
<<: *develop_master_rc_only
requires:
- prep-build-confirmation-redesign-test-mv2
- get-changed-files-with-git-diff
- test-e2e-chrome-rpc:
requires:
- prep-build-test
Expand All @@ -223,7 +222,6 @@ workflows:
<<: *develop_master_rc_only
requires:
- prep-build-test-flask-mv2
- get-changed-files-with-git-diff
- test-e2e-chrome-mmi:
requires:
- prep-build-test-mmi
Expand All @@ -243,7 +241,6 @@ workflows:
- /^Version-v(\d+)[.](\d+)[.](\d+)/
requires:
- prep-build
- get-changed-files-with-git-diff
Copy link
Contributor Author

@seaona seaona Jul 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a great point! Removed too 👍

- test-unit-jest-main:
requires:
- prep-deps
Expand Down Expand Up @@ -488,6 +485,7 @@ jobs:

# This job is used for the e2e quality gate.
# It must be run before any job which uses the run-all.js script.
# The job is skipped in develop, master or RC branches.
get-changed-files-with-git-diff:
executor: node-browsers-small
steps:
Expand All @@ -497,7 +495,15 @@ jobs:
at: .
- run:
name: Get changed files with git diff
command: npx tsx .circleci/scripts/git-diff-develop.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!!

command: |
# Check if the branch is excluded by the filter
if [[ $(git branch --show-current) == *"develop"* || $(git branch --show-current) == *"master"* || $(git branch --show-current) =~ ^Version-v(\d+)\.(\d+)\.(\d+)$ ]]; then
# Branch is excluded, skip actual execution and exit with success code
echo "Branch excluded by filter, skipping get-changed-files."
exit 0
fi
# If not excluded, proceed with the original git-diff command
npx tsx .circleci/scripts/git-diff-develop.ts
- persist_to_workspace:
root: .
paths:
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/changedFilesUtil.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ async function readChangedFiles() {
return changedFiles;
} catch (error) {
console.error('Error reading from file:', error);
return '';
return [];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

returning an empty array, in order to handle properly the subsequent filter, when there is no file (in develop, master and RC branch)

}
}

Expand Down