-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Improve image caret e2e-test #32832
Improve image caret e2e-test #32832
Conversation
Press Enter after adding intial caption will add a new paragraph, allowing skipping the insertBlock() with the same functionality. Local tests runs show saving ~5 seconds on normal test run and >10 seconds when using THROTTLE_CPU=4 SLOW_NETWORK=true
Size Change: 0 B Total Size: 1.04 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just spent a bit of time looking at this as well, as it's blocking a PR.
It's unusual. I think it's a latency issue, using the inserter seems to cause a bit of additional typing latency. I changed the test to the following:
it( 'should place caret at end of caption after merging empty paragraph', async () => {
await insertBlock( 'Image' );
const fileName = await upload( '.wp-block-image input[type="file"]' );
await waitForImage( fileName );
await page.keyboard.type( '1' );
await insertBlock( 'Paragraph' );
expect(
await page.evaluate( () => document.activeElement.nodeName )
).toBe( 'P' );
await page.keyboard.press( 'Backspace' );
expect(
await page.evaluate( () => document.activeElement.nodeName )
).toBe( 'FIGCAPTION' );
await page.keyboard.type( '2' );
expect(
await page.evaluate( () => document.activeElement.innerHTML )
).toBe( '12' );
} );
The test only ever fails on the last expectation, which indicates to me that the test is typing 2
faster than the UI can handle. When I added the following:
expect(
await page.evaluate( () => document.activeElement.nodeName )
).toBe( 'FIGCAPTION' );
the test started passing more regularly, I guess because of the additional time between pressing backspace
and 2
. 🤷
Anyway, that was a long story. Lets get this in and see how it goes. It seems more in the spirit of the test to use the keyboard instead of the inserter, so I don't see an issue.
The same test failed on the trunk commit: So we're still looking for a solution to this one. |
Proposing that it's skipped in #32847 |
Description
This PR aims to improve the image e2e test that checks the scenario
The change switches from using inserter to insert block to simply pressing
enter which default adds a paragraph block, without having to invole the
inserter, search, and the additional time involved which is not the use cae
being testing.
Local tests runs show saving ~5 seconds on normal test run and
How has this been tested?
Run test and confirm works as expected:
npm run test-e2e packages/e2e-tests/specs/editor/blocks/image.test.js
You can also simulate a slower connection which I was able to fail locally before the change, and passes after this change:
THROTTLE_CPU=4 SLOW_NETWORK=true npm run test-e2e packages/e2e-tests/specs/editor/blocks/image.test.js
If you want to watch what puppeteer is doing use
--puppeteer-interactive
npm run test-e2e -- --puppeteer-interactive packages/e2e-tests/specs/editor/blocks/image.test.js
Types of changes
Switches insertBlock to pressing enter in e2e test.