[SITE-1585] Move check-commits into GitHub actions #146
Conversation
This file contains 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
this may or may not work depending on how GHA sources files, but we'll start here
jazzsequence
changed the title
don't run playwright tests if ci failed
| @@ -35,7 +35,7 @@ function identify_commit_type() { | |||
| # Verifies that the given commit does not contain forbidden files. | |||
| function only_allowed_files() { | |||
| local commit=$1 | |||
| local forbidden_files=("composer.lock" ".github/workflows/ci.yml") | |||
| local forbidden_files=("composer.lock" "package-lock.json") | |||
There was a problem hiding this comment.
Why remove .github/workflows/ci.yml?
There was a problem hiding this comment.
For the immediate need, it was blocking the workflow from running on this PR. However, I tried to figure out why we added it in the first place and it looked like a "¯_(ツ)_/¯ maybe this will fix something" addition. There was no ticket associated with it and I don't really know why we would be disallowing changes to the workflow (they're non-release changes anyway).
jazzsequence
added a commit
that referenced
this pull request
Jul 31, 2024
* just add a step that runs the script this may or may not work depending on how GHA sources files, but we'll start here * specify the path * run a git fetch before running the script * check out default branch so we have a local copy * check out a local copy of default * don't warn about detached head * allow changes to ci.yml * remove `variable` * create a failure status artifact don't run playwright tests if ci failed * don't create a status file if it already exists * create an aggregate status file * use the same php-version matrix * remove aggregate status * pull and aggregate all statuses * update perms and add logging * trigger on workflow dispatch * remove status checks * trigger workflow on success * add token * re-add the status artifacts * go back to running on PR * wait for the status artifacts and check them before running the tests * remove test job * add missing done * add some debugging code * make the files unique * allow gh run download to fail * upload the artifacts * add a comment when we're done checking the linting status * comment on the PR * update permissions * exclude status check files
jazzsequence
added a commit
that referenced
this pull request
Aug 1, 2024
* just add a step that runs the script this may or may not work depending on how GHA sources files, but we'll start here * specify the path * run a git fetch before running the script * check out default branch so we have a local copy * check out a local copy of default * don't warn about detached head * allow changes to ci.yml * remove `variable` * create a failure status artifact don't run playwright tests if ci failed * don't create a status file if it already exists * create an aggregate status file * use the same php-version matrix * remove aggregate status * pull and aggregate all statuses * update perms and add logging * trigger on workflow dispatch * remove status checks * trigger workflow on success * add token * re-add the status artifacts * go back to running on PR * wait for the status artifacts and check them before running the tests * remove test job * add missing done * add some debugging code * make the files unique * allow gh run download to fail * upload the artifacts * add a comment when we're done checking the linting status * comment on the PR * update permissions * exclude status check files
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR moves the
check-commitsscript into a GitHub action workflow. It does not remove it from the deploy workflow, but adding it to a GHA workflow means that if we would mix non-release and release commits in a single PR, we are alerted early because the PR will fail.This PR also creates a relationship between the Playwright (functional) tests and the linting tests and commit checks. If anything in the linting tests (
ci.yml) fails, it prevents the Playwright tests from executing.If the check-commits script fails -- and finds mixed commits to release and non-release files in the same PR -- a message is added to the PR and the workflow fails.
This does not change or fix the issues in the upstream or solve the issue described in SITE-1585, but it will (attempt to) prevent issues that currently exist in the upstream from happening again by failing the PR early.