I'm not super convinced this actually reduces flakey tests. Are there examples where restoring checking immediately would cause a flaky test? Generally when changes are made to the editor, it should be immediately reflected in the resulting blocks. If not, we might have a bug.

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: ellatrix <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

I think it might actually do some damage: if there's failing tests, all these tests together will now take a lot longer to complete because each of the failing tests will be waiting a (few?) seconds until returning the failure

Size Change: 0 B

Total Size: 2.57 MB

_{compressed-size-action}

I'm not super convinced this actually reduces flakey tests.

In this case it's more like a safeguard/best practice changes.

Generally when changes are made to the editor, it should be immediately reflected in the resulting blocks. If not, we might have a bug.

True, but that's not always the case for the machinces.

I think it might actually do some damage: if there's failing tests, all these tests together will now take a lot longer to complete because each of the failing tests will be waiting a (few?) seconds until returning the failure

Let's see how this affects current runs. We've been using await expect.poll( editor.getBlocks ) in more tests than I'm chaing now. Additionally, we've been recommending this pattern in our docs; it seems odd not to use it ourselves - https://developer.wordpress.org/block-editor/contributors/code/testing-overview/e2e/overusing-snapshots/#the-solution.

Let's see how this affects current runs. We've been using await expect.poll( editor.getBlocks ) in more tests than I'm chaing now.

I know, it has been affecting runs I think. When there's a lot of failures, tests take way longer to complete because of the polling. It's not just polling for getBlocks of course, but it will contribute.

True, but that's not always the case for the machinces.

I'm just curious when this would happen. Usually, even before Playwright, when there's a flaky test, it's because of our code (time out, animation request...), not because of the machine doing something weird. So I'm highly skeptical that polling for blocks makes any difference with flaky tests. It would be great to see a past example where this happened for getBlocks. If it did, I suspect it may be because of an issue in our code.

I know, it has been affecting runs I think. When there's a lot of failures, tests take way longer to complete because of the polling. It's not just polling for getBlocks of course, but it will contribute.

I think the problem here is the increased number of flaky tests, not Playwright's auto-retries. Unfortunately, the flaky test reports are often ignored these days, and as you mentioned, they might be hiding the reason for issues with our codebase.

It would be great to see a past example where this happened for getBlocks. If it did, I suspect it may be because of an issue in our code.

I can't remember anything at the moment, will share if I recall one.

P.S. We do something similar when rendering values for users in React. We rely on useSelect to poll latest state from the store. We don't expect selectors to return the correct/latest state immediately. I understand it's not exactly a 1-1 comparison, but this is why I always viewed similar assertions.

Going to close this. We can change assertions as needed.