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

E2E: Fix artifacts handling in CI #61338

Merged
merged 4 commits into from
May 7, 2024
Merged

E2E: Fix artifacts handling in CI #61338

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b0a8c65
Fix artifacts handling in CI
WunderBart May 2, 2024
5e27003
tmp: fake flaky tests to verify artifacts merging
WunderBart May 3, 2024
1b18873
Revert "tmp: fake flaky tests to verify artifacts merging"
WunderBart May 7, 2024
9b55c2f
Fix boolean output reference
WunderBart May 7, 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
43 changes: 33 additions & 10 deletions .github/workflows/end2end-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,24 +53,51 @@ jobs:

- name: Archive debug artifacts (screenshots, traces)
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
if: always()
if: ${{ !cancelled() }}
Copy link
Member

Choose a reason for hiding this comment

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

What's the difference here?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

@Mamaduka Mamaduka May 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the clarification!

with:
name: failures-artifacts
name: failures-artifacts--${{ matrix.part }}
path: artifacts/test-results
if-no-files-found: ignore

- name: Archive flaky tests report
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
if: always()
if: ${{ !cancelled() }}
with:
name: flaky-tests-report
name: flaky-tests-report--${{ matrix.part }}
path: flaky-tests
if-no-files-found: ignore

merge-artifacts:
name: Merge Artifacts
if: ${{ !cancelled() }}
needs: [e2e-playwright]
runs-on: ubuntu-latest
outputs:
has-flaky-test-report: ${{ !!steps.merge-flaky-tests-reports.outputs.artifact-id }}
steps:
- name: Merge failures artifacts
uses: actions/upload-artifact/merge@v4
# Don't fail the job if there aren't any artifacts to merge.
continue-on-error: true
with:
name: failures-artifacts
# Retain the merged artifacts in case of a rerun.
pattern: failures-artifacts*
delete-merged: true

- name: Merge flaky tests reports
id: merge-flaky-tests-reports
uses: actions/upload-artifact/merge@v4
continue-on-error: true
with:
name: flaky-tests-report
pattern: flaky-tests-report*
delete-merged: true

report-to-issues:
name: Report to GitHub
needs: [e2e-playwright]
if: ${{ always() }}
needs: [merge-artifacts]
if: ${{ needs.merge-artifacts.outputs.has-flaky-test-report }}
runs-on: ubuntu-latest
steps:
# Checkout defaults to using the branch which triggered the event, which
Expand All @@ -81,24 +108,20 @@ jobs:
show-progress: ${{ runner.debug == '1' && 'true' || 'false' }}

- uses: actions/download-artifact@v4.1.7
id: download_artifact
# Don't fail the job if there isn't any flaky tests report.
continue-on-error: true
with:
name: flaky-tests-report
path: flaky-tests

- name: Setup Node.js and install dependencies
if: ${{ steps.download_artifact.outcome == 'success' }}
uses: ./.github/setup-node

- name: Npm build
if: ${{ steps.download_artifact.outcome == 'success' }}
# TODO: We don't have to build the entire project, just the action itself.
run: npm run build:packages

- name: Report flaky tests
if: ${{ steps.download_artifact.outcome == 'success' }}
uses: ./packages/report-flaky-tests
with:
repo-token: '${{ secrets.GITHUB_TOKEN }}'
Expand Down
4 changes: 3 additions & 1 deletion test/e2e/specs/editor/blocks/avatar.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
test( 'should change image when user is changed', async ( {
editor,
page,
} ) => {
}, testInfo ) => {
// Inserting a quote block
await editor.insertBlock( {
name: 'core/avatar',
Expand Down Expand Up @@ -71,5 +71,7 @@
const newSrc = await updatedAvatarImage.getAttribute( 'src' );

expect( newSrc ).not.toBe( originalSrc );

expect( testInfo.retry ).toBeTruthy();

Check failure on line 75 in test/e2e/specs/editor/blocks/avatar.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 1

[chromium] › editor/blocks/avatar.spec.js:29:2 › Avatar › should change image when user is changed 

  1) [chromium] › editor/blocks/avatar.spec.js:29:2 › Avatar › should change image when user is changed 
    Error: expect(received).toBeTruthy()

    Received: 0

      73 | 		expect( newSrc ).not.toBe( originalSrc );
      74 |
    > 75 | 		expect( testInfo.retry ).toBeTruthy();
         | 		                         ^
      76 | 	} );
      77 | } );
      78 |

        at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/blocks/avatar.spec.js:75:28
} );
} );
4 changes: 3 additions & 1 deletion test/e2e/specs/editor/blocks/navigation-colors.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
editor,
page,
colorControl,
} ) => {
}, testInfo ) => {
const expectedNavigationColors = {
textColor: colorControl.black,
backgroundColor: colorControl.transparent, // There should be no background color set even though the body background is black.
Expand All @@ -73,6 +73,8 @@
await page.goto( `/?p=${ postId }` );

await colorControl.testFrontendColors( expectedNavigationColors );

expect( testInfo.retry ).toBeTruthy();

Check failure on line 77 in test/e2e/specs/editor/blocks/navigation-colors.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 2

[chromium] › editor/blocks/navigation-colors.spec.js:57:2 › Navigation colors › All navigation links should default to the body color and submenus and mobile overlay should default to a white background with black text 

  1) [chromium] › editor/blocks/navigation-colors.spec.js:57:2 › Navigation colors › All navigation links should default to the body color and submenus and mobile overlay should default to a white background with black text 
    Error: expect(received).toBeTruthy()

    Received: 0

      75 | 		await colorControl.testFrontendColors( expectedNavigationColors );
      76 |
    > 77 | 		expect( testInfo.retry ).toBeTruthy();
         | 		                         ^
      78 | 	} );
      79 |
      80 | 	test( 'Top level navigation links inherit the text color from the theme/group but do not apply to the submenu or overlay text', async ( {

        at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/blocks/navigation-colors.spec.js:77:28
} );

test( 'Top level navigation links inherit the text color from the theme/group but do not apply to the submenu or overlay text', async ( {
Expand Down
Loading