E2E: Fix artifacts handling in CI #61338
Merged
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
WunderBart
force-pushed
the
fix/e2e-workflow-artifacts
branch
2 times, most recently
from
0171ff5 to
038e93e
Compare
|
Size Change: 0 B Total Size: 1.74 MB ℹ️ View Unchanged
|
WunderBart
force-pushed
the
fix/e2e-workflow-artifacts
branch
4 times, most recently
from
713455c to
f3a0e8d
Compare
|
Flaky tests detected in 5e27003. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8966643688
|
WunderBart
force-pushed
the
fix/e2e-workflow-artifacts
branch
7 times, most recently
from
8a68d89 to
45bc1f2
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
WunderBart
added
the
[Type] Automated Testing
WunderBart
force-pushed
the
fix/e2e-workflow-artifacts
branch
from
1b094d5 to
5e27003
Compare
Mamaduka
reviewed
May 7, 2024
| @@ -53,24 +53,51 @@ jobs: | |||
|
|
|||
| - name: Archive debug artifacts (screenshots, traces) | |||
| uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3 | |||
| if: always() | |||
| if: ${{ !cancelled() }} | |||
There was a problem hiding this comment.
always() causes the step to be always executed, even when the job is cancelled, which I don't think we need. Using !cancelled() will skip it if the job is cancelled.
There was a problem hiding this comment.
Thanks for the clarification!
Mamaduka
approved these changes
May 7, 2024
There was a problem hiding this comment.
Thank you, @WunderBart!
The generated artifacts and flaky test reports look good to me. Let's not forget to revert the temp commit before merging.
This reverts commit 5e27003.
Those values aren't actually booleans, so we need to check by string comparison.
|
I reverted the tmp commit, and everything seems to work as expected: Artifacts were not produced, so the |
|
@WunderBart, I noticed following errors in e2e test summary - https://github.com/WordPress/gutenberg/actions/runs/9042463717 Screenshot
|
I should have mentioned it in the original PR, but thankfully that's expected. It's because the merge action does not have an BTW, there's an open request to add |
|
Thanks for the clarification, @WunderBart! |
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.
What?
Resolves #58995.
Supersedes #57183.
Since the artifacts became immutable in v4 of actions/upload-artifact, we must create a separate artifact for each shard (both failures and flaky reports), or we'll get a conflict error (see the attached issue #58995). This is the main issue this PR addresses.
Enhancements
For convenience, this PR adds a step to merge artifacts into a single zip so we don't need to remember the shard number. The partial artifacts will be deleted after merging to reduce noise. In case of a rerun, the new artifacts (if any) will be merged with those from the previous run(s) so we don't lose any information. Lastly, unlike currently, the
Report to GitHubjob will only run if a flaky test report zip is produced.Testing Instructions
Since this is not manually testable, I've pushed a tmp commit that causes two tests to be flaky. Those tests are in two shards (no. 1 & 2) to verify that the artifact upload, merging, and flaky test reporting work as expected. Download the artifacts and confirm the content matches what's reported in CI. Check out the CI output and ensure everything looks as expected. Confirm the flaky report comment contains correct test reports.
The tmp commit will be reverted when this PR is approved. The reopened flaky report issues will also be closed.