E2E: Do not run page actions in beforeEach hook when using fixme #54065
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
added
the
[Type] Automated Testing
|
Size Change: +117 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
|
Passed without any hiccups 2 times in a row. I can keep rerunning, but it should be safe to merge at this point. |
|
Should this also revert #53905? |
|
Thanks for looking into it! It still bugs me why it suddenly appeared though. We haven't upgraded Playwright in a while, and the test was written 10 months ago, so there must be something else at play here 🤔. We can still try this if it fixes the issue and unblocks CI though!
Yes, I think we should! Increasing the global timeout doesn't seem to make sense IMO. |
This reverts commit 5a77e22.
@kevin940726 it doesn't seem to be that sudden, though - the first flake was registered almost 10 months ago, so around the time the test was written, if I'm looking correctly. |
|
Flaky tests detected in 5035fee. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6035871351
|
|
Oh I see 😆! Then why do we suddenly want to work on fixing this (given that it's not that flaky)? |
It's been failing quite persistently recently but not being registered as flaky at the same time AFAIU. Just yesterday, I needed to rerun 3 times to get that very test to pass, and 6 times 2 weeks ago. Is it possible that it's not being registered as flaky because it either passes on the first run or doesn't pass at all? |
|
Here's the recent failure on the trunk - https://github.com/WordPress/gutenberg/actions/runs/6036043724/job/16377594899. Creating a post in Should we discourage pushing the |
For the record, using ✅ GOOD - test.fixme("Help me I'm failing", () => {
// Fix me.
});❌ BAD - test("Help me I'm failing", () => {
test.fixme(({ browserName }) => browserName === 'firefox');
// Fix me.
}); |
Mamaduka
approved these changes
Aug 31, 2023
|
Okay so I think the errors are different than the one reported in #45522 then? Then it still feels pretty weird to me that it suddenly fails often for firefox. I wonder if upgrading Playwright (#54051) magically solves this 😅 . If it's a bug in Playwright then we would also like to report it upstream. I don't think it has a high priority though and I'm glad that if this fixes the issue 🙂 . |
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?
Another attempt to fix a flaky inserting blocks test + revert the previous attempt.
How?
There is an antipattern where we initialize a new post (via
createNewPost) in thebeforeEachhook and then mark some tests asfixme(aka skip). This causes tests not to be truly skipped because a new post gets created anyway, which initializes the timers and some handles. Skipping the test then causes the page to be (abruptly?) closed, likely causing the disconnecting race condition that makes these tests flaky.