[E2E] Add regression E2E test for the Paragraph to List copy and paste failure #33080
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
|
Size Change: 0 B Total Size: 1.05 MB ℹ️ View Unchanged
|
ockham
reviewed
Jun 30, 2021
Comment on lines
+557
to
+560
| const content = await page.$eval( | ||
| '.wp-block-list', | ||
| ( el ) => el.innerHTML | ||
| ); |
There was a problem hiding this comment.
We tend to use getEditedPostContent() from @wordpress/e2e-test-utils for cases like these, I think. (Which would mean comparing to a whole snapshot, e.g. via expect( content ).toMatchInlineSnapshot( ... ).
If that seems to heavy-handed, maybe we can at least change the selector here: Overall, the rule of thumb is to use something "user-facing" in e2e tests, rather than a class name (like .wp-block-list), as the latter is considered an implementation detail (that might change over time).
Often, we tend to use aria-labels as selectors, e.g.
Suggested change
| const content = await page.$eval( | |
| '.wp-block-list', | |
| ( el ) => el.innerHTML | |
| ); | |
| const content = await page.$eval( | |
| '[aria-label="Block: List"]', | |
| ( el ) => el.innerHTML | |
| ); |
|
I wonder if we should put the test into a different place, since it seems to be somewhat block-agnostic, and more about generically copy-pasting from one block (or RichText component?) to another. I've noticed that we have https://github.com/WordPress/gutenberg/blob/79e02a2684abb53a6235818abcad5c045653fc7b/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js. Maybe we need something like cc/ @mcsf @youknowriad @ellatrix to weigh in, since I'm not that familiar with how we're implementing copy-pasting. |
Thanks for having a look! I also noticed we have https://github.com/WordPress/gutenberg/blob/trunk/packages/e2e-tests/specs/editor/various/rich-text.test.js, which was actually modified as part of #27967, and seems to test some clipboard aspects, too. I agree that if we can identify and test (move the case to a more general test suite) the issue at a lower-level, it'd be better. I'll wait for insights from @mcsf @youknowriad @ellatrix about that. |
|
I added e2e tests in #35416, which is now merged. Closing this. |
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.
Description
Adds regression test to cover #33078.
For now this is very specific to the paragraph -> list copy and paste scenario (see issue above). If we learn more about the root of the issue in #33078, we might add more scenarios or perhaps test it at a lower level (?), and or move it somewhere else (I'm open to suggestions!). For now, this should be enough to catch the issue though. Note that at the time of this writing,
trunkstill doesn't have a fix for it, so it's expected that this test is failing.How has this been tested?
Paragraph->Listdoesn't work (it copies and pastes by sending the actual key combos, refer to the test). I manually tested the positive scenario by pasting from some other source, and it passed.To run the test, run
npm install, make sure tounset PUPPETEER_SKIP_DOWNLOADjust in case and:npx wp-scripts test-e2e --config packages/e2e-tests/jest.config.js packages/e2e-tests/specs/editor/blocks/list.test.jsScreenshots
Types of changes
New regression E2E test.
Checklist:
*.native.jsfiles for terms that need renaming or removal).Resolves: #33073