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.

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

E2E Tests: Fix and skip flakey tests, improve debug output #11782

Merged
merged 155 commits into from
Jul 27, 2022

Conversation

timarney
Copy link
Contributor

@timarney timarney commented Jun 23, 2022

Context

E2E test are having more test run failures. We need to stabilize tests where we can.

Example test run timing out after 30 min

https://github.com/GoogleForCreators/web-stories-wp/runs/6965907754?check_suite_focus=true

Summary

This PR addresses some of the issues with the flakey e2e tests

Generally seeing better test runs now -- if failures do occur re-running tests in that shard seem to pass.

Example run good run:
https://github.com/GoogleForCreators/web-stories-wp/actions/runs/2662886690

  • Adds notes to skipped tests in this PR
  • Updates shard count for WP 5.7 runs (speeds things up + help with test re-runs)
  • Removes runAxeTestsForStoriesEditor
  • Update Artifacts package to capture detailed stack track
  • Improves console output for unhandled errors

I would suggest merging this to move things forward and then re-looking at individual skipped tests.

Relevant Technical Choices

To-do

As a follow up we might want to look at automating failed steps with something like https://github.com/marketplace/actions/retry-step (to save manually re-running tests)

Notes

Errors
Stuck on Media and or Loading screen
#11782 (comment)

Error: net::ERR_SOCKET_NOT_CONNECTED
#11782 (comment)

Error: net::ERR_CONNECTION_RESET
#11782 (comment)

Artifact package
#11782 (comment)

Helpful for debugging:
Image capture using Base 64 -- was able to capture messages that didn't end up in the artifacts package
#11782 (comment)

User-facing changes

Testing Instructions

  • This is a non-user-facing change and requires no QA

This PR can be tested by following these steps:

Reviews

Does this PR have a security-related impact?

No

Does this PR change what data or activity we track or use?

No

Does this PR have a legal-related impact?

No

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Fixes #11803

@timarney timarney self-assigned this Jun 23, 2022
@timarney timarney changed the title Try/e2e test fixes e2e test updates Jun 24, 2022
@timarney timarney changed the title e2e test updates Fix flakey e2e tests Jun 27, 2022
@timarney timarney added the Type: Infrastructure Changes impacting testing infrastructure or build tooling label Jun 27, 2022
@swissspidy
Copy link
Collaborator

One failing test Shopify settings › should let me see and update shopping provider settings

Copy link
Collaborator

@swissspidy swissspidy left a comment

Choose a reason for hiding this comment

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

You seem to have added a git submodule here by accident

Screenshot 2022-07-25 at 12 07 44

packages/e2e-tests/src/specs/editor/pendingStories.js Outdated Show resolved Hide resolved
packages/e2e-tests/src/specs/wordpress/archive.js Outdated Show resolved Hide resolved
patches/puppeteer+15.4.1.patch Outdated Show resolved Hide resolved
@timarney
Copy link
Contributor Author

Running into percy issues.

[percy] Encountered an error uploading snapshot: Story Background Audio
[percy] Error: Cannot create snapshot, the build is already finalized. If this happens frequently in a parallel test suite, you may want to use a "finalize all" step in CI: 

@swissspidy
Copy link
Collaborator

This can happen when re-running failing tests multiple times, shouldn't cause issues

@timarney
Copy link
Contributor Author

Outside of the Percy noise I think all the skips etc... have issues assigned to them now.

@swissspidy
Copy link
Collaborator

I see two issues right now at first glance:

First of all, there are still some failing tests, see https://github.com/GoogleForCreators/web-stories-wp/runs/7520603772?check_suite_focus=true#step:10:165

Excerpt from logs:

FAIL packages/e2e-tests/src/specs/editor/prePublishChecklist/adminUser.js (22.544 s)
  ● Pre-Publish Checklist : Admin User › should focus on media button when poster image issue card is clicked

    expect(jest.fn()).not.toHaveErrored(expected)

    Expected mock function not to be called but it was called with:
    ["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"],["Failed to load resource: net::ERR_CONNECTION_CLOSED"]

      at Object.assertExpectedCalls (node_modules/@wordpress/jest-console/build/@wordpress/jest-console/src/index.js:36:4)
          at runMicrotasks (<anonymous>)

FAIL packages/e2e-tests/src/specs/editor/editor.js (23.118 s)
  ● Story Editor › should preview story with development mode

    TimeoutError: Text not found "/Debug/i"
    waiting for function failed: timeout 2000ms exceeded

      76 |
      77 |     await expect(previewPage).toMatch(/Preview/i);
    > 78 |     await expect(previewPage).toMatch(/Debug/i);
         |     ^
      79 |     await expect(previewPage).toMatch(/Add device/i);
      80 |
      81 |     await previewPage.waitForSelector('amp-story-player');

      at Object.<anonymous> (packages/e2e-tests/src/specs/editor/editor.js:78:5)

If these fail consistently we need to skip them.

Second, the sharding does not seem to work for Percy runs 😬 So basically every one of the chrome - WP latest (x/4) groups runs the full test suite. Not very efficient 🙃
Not a fault of this PR, but I just noticed it now.

Looks like it's because of how the arg is passed down to the command, see how the --shard arg is lost here:

Run npm run test:e2e:percy -- --shard=$SHARD

> test:e2e:percy
> percy exec --parallel --config=percy.config.yml -- npm run test:e2e "--shard=2/4"

[percy] Percy has started!
[percy] Running "npm run test:e2e --shard=2/4"

> test:e2e
> cross-env JEST_PUPPETEER_CONFIG=jest-puppeteer.config.cjs jest --runInBand --config=packages/e2e-tests/src/jest.config.js <--------------- missing shard

854c67b should fix it.

@swissspidy
Copy link
Collaborator

Look how fast this is now with proper sharding:

Screenshot 2022-07-26 at 17 07 42

We should be able to reduce to just 2 shards with similar execution time

This one is still failing:

FAIL packages/e2e-tests/src/specs/editor/editor.js (29.372 s)
  ● Story Editor › should preview story with development mode

    TimeoutError: Text not found "/Preview/i"
    waiting for function failed: timeout 2000ms exceeded

      75 |     await previewPage.waitForSelector('.i-amphtml-story-dev-tools-header');
      76 |
    > 77 |     await expect(previewPage).toMatch(/Preview/i);
         |     ^
      78 |     await expect(previewPage).toMatch(/Debug/i);
      79 |     await expect(previewPage).toMatch(/Add device/i);
      80 |

      at Object.<anonymous> (packages/e2e-tests/src/specs/editor/editor.js:77:5)

Let's disable it & create a ticket for it.

We'd want to merge this PR with passing tests.

@timarney
Copy link
Contributor Author

@swissspidy here's a passing run for the end to end tests with no retries

https://github.com/GoogleForCreators/web-stories-wp/runs/7528661369?check_suite_focus=true

Screen Shot 2022-07-26 at 5 13 23 PM

@swissspidy
Copy link
Collaborator

Still passing!

@timarney Kudos for all your hard work on this!! 👏 👏

Now on to the next challenge of addressing all these flakey tests...

@swissspidy swissspidy changed the title Fix flakey e2e tests E2E Tests: Fix and skip flakey tests, improve debug output Jul 27, 2022
@swissspidy swissspidy merged commit 3f9e79f into main Jul 27, 2022
@swissspidy swissspidy deleted the try/e2e-test-fixes branch July 27, 2022 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Infrastructure Changes impacting testing infrastructure or build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix flakey e2e tests
4 participants