-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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.
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
[ci-jobs] Add support for github event and include HPOS disabled e2e tests in ci #46922
Conversation
…d-support-for-events-in-ci-jobs
Hi @rrennick, @woocommerce/vortex Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
…d-support-for-events-in-ci-jobs # Conflicts: # tools/monorepo-utils/dist/index.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pnpm utils ci-jobs should return all configured jobs without the 5 shards of "Core e2e tests - HPOS disabled"
The response includes the 5 HPOS disabled tests for me. Also, this PR only includes @woocommerce/plugin-woocommerce - Core API tests - HPOS disabled
when the other HPOS disabled tests are included. The PR testing instructions should be updated to reflect that.
I'm leaving this as a comment to not hang the PR on my review as I'll be AFK next week.
@@ -23,11 +23,24 @@ const program = new Command( 'ci-jobs' ) | |||
'Base ref to compare the current ref against for change detection. If not specified, all projects will be considered changed.', | |||
'' | |||
) | |||
.option( | |||
'-e --event <event>', | |||
'Github event for which to run the jobs. If not specified, all projects will with changes be considered.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wording here is unclear. Do you mean If not specified, all events will be considered.
? The current code already considers all packages with changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was struggling to get the right wording here. Your suggestion is much better. Updated with 5008e80
…d-support-for-events-in-ci-jobs
I fixed the testing instructions. That command should return the HPOS disabled tests, it worked as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…tests in ci (#46922) * Update env setup script to disable hpos * Use DISABLE_HPOS instead of ENABLE_HPOS * Log HPOS state * Configure project for core e2e with HPOS disabled * Add event argument for ci-jobs util * Add event option for test jobs * Add support for github events * Add changelog * Configure HPOS tests to run on pull_request * Fix utils tests for undefined commandVars * Added some tests for the event configuration * Revert event for HPOS e2e to push * Use matrix name in artifact name to avoid duplication * Test with pull_request event * Use job-index for unique artifacts names * Revert event for HPOS e2e to push * Add api tests for HPOS disabled * Use unique artifact name for api tests * Revert event for HPOS disabled api tests to push * Rebuild monorepo utils to fix a merge conflict * Updated wording
…tests in ci (#46922) * Update env setup script to disable hpos * Use DISABLE_HPOS instead of ENABLE_HPOS * Log HPOS state * Configure project for core e2e with HPOS disabled * Add event argument for ci-jobs util * Add event option for test jobs * Add support for github events * Add changelog * Configure HPOS tests to run on pull_request * Fix utils tests for undefined commandVars * Added some tests for the event configuration * Revert event for HPOS e2e to push * Use matrix name in artifact name to avoid duplication * Test with pull_request event * Use job-index for unique artifacts names * Revert event for HPOS e2e to push * Add api tests for HPOS disabled * Use unique artifact name for api tests * Revert event for HPOS disabled api tests to push * Rebuild monorepo utils to fix a merge conflict * Updated wording
Submission Review Guidelines:
Changes proposed in this Pull Request:
Closes #46925
Closes #46924
Adds support for Github events in ci-jobs, so we can only run jobs for specific events, like only for
push
.events
as an array of strings, which should contain a list of events for which the job should run. If no events list is defined or the list is empty the job will run for any event.ci-jobs
cli:--event
or-e
. If no argument is passed the tool will consider all jobs and do not filter by event.events: [push]
configuration. This should only run forpush
events and not run forpull_request
env:test:non-hpos
command and the test env setup script to disable the HPOS feature. Also added a print for the HPOS value in the global setup to easily spot if the expected value is set.How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Run pnpm utils ci-jobs with different arguments and check the output.
pnpm utils ci-jobs --event push
should return all configured jobs, including the HPOS disabled ones (5 e2e and 1 API)pnpm utils ci-jobs --event pull_request
should return all configured jobs without the 5 shards of "Core e2e tests - HPOS disabled" and without theCore API tests - HPOS disabled
pnpm utils ci-jobs
should return all configured jobs including the HPOS disabled ones.pnpm utils ci-jobs
with the right event argument.pull_request
event configured were triggered: https://github.com/woocommerce/woocommerce/actions/runs/8838048787?pr=46922push
event only (default value) and they were not triggered: https://github.com/woocommerce/woocommerce/actions/runs/8838873869/job/24271155448?pr=46922