Testing: Fix randomly failing E2E test#28073
Merged
noisysocks merged 2 commits intomasterfrom Jan 11, 2021
Merged
Conversation
…e or not an HTMLElement`
rafaelgallani
requested review from
ajitbohra,
nerrad,
ntwb and
talldan
as code owners
rafaelgallani
force-pushed
the
fix/e2e-testing-multi-entity-edit-error
branch
from
2c247e6 to
a932bfd
Compare
|
Size Change: +22 B (0%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
gziolo
added
the
[Type] Automated Testing
Member
|
Hey thanks for this! Do you know why the network request causes a focus loss? Is that a bug that should be fixed? |
Member
|
A possibly less fragile approach here would be to use |
noisysocks
approved these changes
Jan 11, 2021
Member
|
Let's merge this because it fixes an infuriating intermittent test failure 🙂 I think that we need to check why focus is lost when the network request completes, though. I worry that there's an actual bug there. |
Contributor
Author
@noisysocks You're welcome! I'm not sure why the request is causing the focus loss, I'll try to analyze it later. 😸
Yes! I tried waiting for the Thanks for the feedback! 😀 |
Contributor
|
Awesome work here and I'm very happy that this intermittent failure is fixed. It seems to me that this test is written in such a way that consequential tests depend on each other (which is not great). Ideally we'd start a new page/load screen in |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Description
The E2E tests for the
multi-entity-editing.test.jsfile were failing very often, in a random sort (like here, here, and here). When testing locally, I found that the issue was happening due to the tested HTML element's focus being lost. This PR fixes this error.For better understanding, check the "How has this been tested?" section 😸.
How has this been tested?
In short - this test consists of editing the entities and asserting whether they have been correctly dirtied, so, the basic flow for each closure of the test is:
editthe target element;assertobtained results;savechanges (afterEachmethod call).The error was happening due to a bug between the
assertand thesavestep:After asserting the results for a previous test closure, the
savestep calls thesaveAllEntitiesfunction:gutenberg/packages/e2e-tests/specs/experiments/multi-entity-editing.test.js
Lines 202 to 205 in a7fb6bb
Which would be basically clicking the "Save" button on the tested page:
gutenberg/packages/e2e-tests/specs/experiments/multi-entity-editing.test.js
Lines 68 to 72 in a7fb6bb
The problem: calling
awaiton the button click just waits for the returned click promise to be resolved; it doesn't wait for the whole save process to be completed.Therefore, sometimes, when the save process was not yet completed and the test continued, after the save request completed, the
editstep would try to change the element value - but it would fail, due to the focus change that would happen in the page. I find this very hard to explain only using words, so maybe these screenshots might help:This is the exact moment where the error happens: right after saving the changes, about to focus on the next tested element.
I simulated the behavior in the console:
I made a
setIntervalcall to keep logging the active element (the one currently focused) - and right after, the default test behavior: click the save button and start the next test closure. As shown, the block element would lose its focus right after the save request completed, causing the error.In addition, I ran the test 60 times, comparing the outputs before and after the changes:
Before - Failing 70% of the time (42 failures out of 60 runs) - failures related to the error that was being fixed (
Node is not visible/Node is detached from document).After - No failures. 🥳🎉🎉🎉🎉😸
Types of changes
Bugfix to the E2E tests.
Checklist: