Skip to content
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

Fix 'Multi entity saving -> site editor' e2e failures. #26371

Merged

Conversation

Addison-Stavlo
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo commented Oct 21, 2020

Description

Should fix the e2e failures in multi-entity-saving tests. Oddly insertBlock( 'Template Part' ) doesn't seem to work consistently anymore (not sure why, manual flow seems to work fine still). Since we just needed to add something/anything to dirty the editor, changing this to a paragraph block seems to fix the test.

How has this been tested?

Ensure CI e2e checks don't fail on multi-entity-saving tests.

Run npm run test-e2e:watch locally and run the multi-entity-saving file. Verify tests pass.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions
Copy link

Size Change: 0 B

Total Size: 1.2 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.54 kB 0 B
build/api-fetch/index.js 3.35 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 668 B 0 B
build/block-directory/index.js 8.59 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 130 kB 0 B
build/block-editor/style-rtl.css 11 kB 0 B
build/block-editor/style.css 11 kB 0 B
build/block-library/editor-rtl.css 8.93 kB 0 B
build/block-library/editor.css 8.93 kB 0 B
build/block-library/index.js 145 kB 0 B
build/block-library/style-rtl.css 7.75 kB 0 B
build/block-library/style.css 7.75 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.77 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.6 kB 0 B
build/components/index.js 170 kB 0 B
build/components/style-rtl.css 15.4 kB 0 B
build/components/style.css 15.4 kB 0 B
build/compose/index.js 9.63 kB 0 B
build/core-data/index.js 12.1 kB 0 B
build/data-controls/index.js 684 B 0 B
build/data/index.js 8.63 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 4.43 kB 0 B
build/edit-navigation/index.js 10.8 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.37 kB 0 B
build/edit-post/style.css 6.35 kB 0 B
build/edit-site/index.js 21.7 kB 0 B
build/edit-site/style-rtl.css 3.79 kB 0 B
build/edit-site/style.css 3.79 kB 0 B
build/edit-widgets/index.js 26.6 kB 0 B
build/edit-widgets/style-rtl.css 3.09 kB 0 B
build/edit-widgets/style.css 3.09 kB 0 B
build/editor/editor-styles-rtl.css 480 B 0 B
build/editor/editor-styles.css 482 B 0 B
build/editor/index.js 42.6 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.84 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.49 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.54 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.45 kB 0 B
build/primitives/index.js 1.35 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/reusable-blocks/index.js 3.06 kB 0 B
build/rich-text/index.js 13 kB 0 B
build/server-side-render/index.js 2.61 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.75 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.23 kB 0 B

compressed-size-action

@jeyip
Copy link
Contributor

jeyip commented Oct 21, 2020

I came to the exact same conclusions as you have @Addison-Stavlo while investigating failures in one of @david-szabo97 's recent tickets. What a strange issue 😕

I was reluctant to open a PR to fix the problem by replacing the block we insert, but now that I think more about it, I agree that this is necessary, especially to unblock e2e tests.

I'm still concerned about the template part insertion failure. I think it would be worthwhile to create a separate issue (happy to handle that), but I'm curious what your thoughts are.

@Addison-Stavlo Addison-Stavlo merged commit 79e57cc into master Oct 21, 2020
@Addison-Stavlo Addison-Stavlo deleted the fix/multi-entity-save-flow-site-editor-e2e-failures branch October 21, 2020 19:33
@github-actions github-actions bot added this to the Gutenberg 9.3 milestone Oct 21, 2020
@Addison-Stavlo
Copy link
Contributor Author

Addison-Stavlo commented Oct 21, 2020

I'm still concerned about the template part insertion failure. I think it would be worthwhile to create a separate issue

Yeah, im a bit confused by it as well. The template-part tests that go through inserting seem to be working still. 🤔 Luckily for this test specifically there is no reason to use a template part.

@ZebulanStanphill ZebulanStanphill added the [Package] E2E Tests /packages/e2e-tests label Oct 21, 2020
@noahtallen
Copy link
Member

It's possible that we used a second template part block to test nested template part block insertion & dirty states, but I kinda forget :)

@talldan
Copy link
Contributor

talldan commented Oct 22, 2020

Something that might have made the results in this PR confusing is that I'd already made some changes to the test in #26341 which was merged before this. It seems to have been passing ok since #26341 in commits to master so the change here might not have made much difference.

I definitely observed the same thing, that Template Parts couldn't be inserted. I changed the test to first close the navigation panel (seems unusual, but it wouldn't work without this), and also click a block in the editor before inserting. My feeling was that the inserter was possibly attempting to insert the Template Part somewhere it couldn't be, but when trying out things manually it always worked.

Still, I don't really understand the FSE flow that much yet having not worked on it, so happy for the tests to be changed to something more logical.

@Addison-Stavlo
Copy link
Contributor Author

It seems to have been passing ok since #26341 in commits to master so the change here might not have made much difference.

Oh interesting! I noticed they weren't failing on github, but running locally after that change I was still seeing them fail at the block insertion step. 🤷‍♀️ It seems difficult to get local e2e to give the same results as the CI checks sometimes... 🤔

tellthemachines added a commit that referenced this pull request Oct 27, 2020
* Fix drop zone indicators for non blocks (#25986)

* Remove isDraggingBlocks check

* Clean up drag and drop data if the user presses escape when dragging

* Add comment to ensure avoidance of dragend

* Switch to using dragend

* Fix archives block render function (#26309)

* Code block: preserve indentation on paste (#26347)

* Turn off autocomplete for token input. (#26427)

* Fix parent post selector: ensure initial value available, search performed, all results shown. (#26397)

Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com>

* [RNMobile] Fix CI issues during `npm ci` (#26455)

* Run mobile tests on master

* Debug CI

* Revert "Debug CI"

This reverts commit dec1ebe.

* Update react-native-screens ref, pin @react-navigation/native version

* Pin @react-navigation/core,router versions

* Fix composer test failures due to invalid lock (#26472)

* Fix gallery block undo issue (#26377)

* Mark change in attributes when gallery mounts as not persistant

* Fix typo

* Fix package lock changes

* Fix some Twenty Twenty One related e2e test failures (#26341)

* Account for unknown number of controls in block inspector

* Ensure selection is on-screen by using a group block instead of cover, which takes up less space on screen

* Fix typewriter test

* Use correct name for patterns test

* Revert change to .wp-env.json

* Fix multi-entity side editor saving test

* Remove blank line

* insert paragraph instead of tempalte part (#26371)

* Fix autosave e2e tests (#26416)

* Update how block to render is tracked (#26356)

Co-authored-by: Daniel Richards <daniel.richards@automattic.com>
Co-authored-by: Riad Benguella <benguella@gmail.com>
Co-authored-by: Ella van Durpe <wp@iseulde.com>
Co-authored-by: Adam Silverstein <adamsilverstein@earthboundhosting.com>
Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com>
Co-authored-by: Ceyhun Ozugur <ceyhunozugur@gmail.com>
Co-authored-by: Marcus Kazmierczak <marcus@mkaz.com>
Co-authored-by: Addison Stavlo <Stavz01@gmail.com>
Co-authored-by: O André <nosolosw@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] E2E Tests /packages/e2e-tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants