skip navigation block e2e tests #43571
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
MaggieCabrera
requested review from
ockham,
Mamaduka,
getdave,
scruffian and
draganescu
ockham
approved these changes
Aug 24, 2022
There was a problem hiding this comment.
Thanks a lot, Maggie!
Let's merge this to get things un-stuck 👍
FWIW, without this change -- i.e. on current trunk -- I just tried running npm run test:e2e -- packages/e2e-tests/specs/editor/blocks/navigation.test.js locally. It looks stuck with no sign of life for about 10 minutes, and then fails with a timeout after 691 seconds.
Test Suites: 1 failed, 1 total
Tests: 15 failed, 3 skipped, 12 passed, 30 total
Snapshots: 2 passed, 2 total
Time: 691.628 s
|
Thanks, @MaggieCabrera. We'll probably have to re-enable Navigation tests shortly. It's probably one of the complex blocks in Block Library and easy to miss regressions. But considering all the flakiness, we might be already missing some 😅 |
|
Size Change: 0 B Total Size: 1.24 MB ℹ️ View Unchanged
|
|
I looks like #42735 introduced more flakiness - and that PR only does one major change, using |
|
Ultimately I thought it'd be possible to collaborate on the Playwright PR, but I don't think I took the right approach. I should've just merged a subset of tests when I had them passing. Let me see if I can get the ones that I have written passing again, and then I'll merge them if I can (removing the puppeteer versions). |
This was harder than I thought. The block has been changed quite a lot, and the Puppeteer tests have become very disorganised and hard to migrate. I actually think it may be worth scrapping the Puppeteer tests. They can run on CI if they can be made stable, but I don't think it's worth migrating the current state directly over to Playwright, it'd be better to restructure everything. One of the problems is that the Puppeteer tests don't really tell a story. Ideally the very first test case would start from the first thing a user sees, and then the resulting test cases are ordered and structured in a logical way that matches the user's journey through the block. I think it would be worth writing some user stories, and then writing new test cases around those. |
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?
This disables the Navigation block e2e tests
Why?
Looks like these tests are failing consistently and they need to be reworked anyway. They are causing things like the RC build action to get stuck because we are triggering the repo limit. Here is the discussion about this.
How?
Skipping the tests :D
Testing Instructions
Locally you can run
npm run test:e2e -- packages/e2e-tests/specs/editor/blocks/navigation.test.jsand will see the tests being skippedScreenshots or screencast