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

Reusable blocks e2e tests fail if starting from new post #10267

Closed
wants to merge 2 commits into from

Conversation

ellatrix
Copy link
Member

Description

Sparked by #9962. These tests should still pass and demonstrate some issues starting from a new post.

How has this been tested?

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.

@ellatrix
Copy link
Member Author

ellatrix commented Sep 30, 2018

See #9962 (comment).

@aduth aduth added this to the 4.1 milestone Oct 5, 2018
@aduth aduth added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) labels Oct 5, 2018
@youknowriad youknowriad modified the milestones: 4.2, WordPress 5.0 Oct 27, 2018
@gziolo gziolo modified the milestones: WordPress 5.0, 4.4 Nov 5, 2018
@aduth
Copy link
Member

aduth commented Nov 9, 2018

cc @noisysocks

@noisysocks noisysocks force-pushed the try/reusable-block-e2e-test branch 2 times, most recently from 7a00d03 to 98227ef Compare November 12, 2018 05:08
@noisysocks
Copy link
Member

I rebased this and pushed up some changes which should hopefully fix the failing tests here.

There were three problems:

1) A reusable block's Name field would get selected after a fetchReusableBlocks()

After a successful fetch request, RECEIVE_REUSABLE_BLOCKS was dispatched which would then in turn dispatch RECEIVE_BLOCKS. This meant that there was a brief moment (after the first dispatch but before the second dispatch) where getReusableBlock( ref ) would return a reusable block that referenced a block that didn't exist in the store. This caused the block to render its Block has been deleted or is unavailable message, which in turn caused ReusableBlockEditPanel to remount, which in turn caused the Name field to be select()ed.

I fixed this by removing the RECEIVE_REUSABLE_BLOCKS effect handler and instead making the reducer handle RECEIVE_REUSABLE_BLOCKS directly.

2) Race condition between deleting a reusable block and fetching reusable blocks

After clicking Remove from Reusable Blocks on a new post with a reusable block in it, both deleteReusableBlock() and fetchReusableBlocks() would be dispatched at the same time. This was because the cursor would be placed into a Paragraph block which then activates blocksAutocompleter. These two HTTP requests would race and, often, the response to fetchReusableBlocks() would come back before the block had been deleted, causing Gutenberg to mistakenly show the deleted block in the inserter.

I fixed this by moving fetchReusableBlocks() from blocksAutocompleter and into SETUP_EDITOR.

3) E2E test not able to click on a reusable block in the inserter

Since the tests are now always starting from a new post, we now need to wait for fetchReusableBlocks() to finish before we can click on the reusable block in the inserter.

I fixed this by using waitForSelector().

@noisysocks
Copy link
Member

Hm, damn, still some failures on Travis CI. I'm not sure if they're related.

@youknowriad youknowriad modified the milestones: 4.4, 4.5 Nov 15, 2018
@gziolo gziolo added the [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later label Nov 19, 2018
@gziolo
Copy link
Member

gziolo commented Nov 19, 2018

@iseulde can you rebase with master and see if e2e failures were unrelated to this PR?

@catehstn
Copy link

Moving this to 4.6, no rush to get it in today.

@catehstn catehstn modified the milestones: 4.5, 4.6 Nov 19, 2018
@mtias mtias modified the milestones: 4.6, 4.7 Nov 22, 2018
@youknowriad youknowriad removed this from the 4.7 milestone Dec 3, 2018
@gziolo gziolo added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Feb 7, 2019
@gziolo
Copy link
Member

gziolo commented Feb 7, 2019

@noisysocks and @iseulde - this one needs to be refreshed. Do you plan to get back to this PR? I'm marking it as Stale to make triaging and reviewing PR easier.

@ellatrix
Copy link
Member Author

ellatrix commented Feb 7, 2019

I don't. Looked at this at a meetup quick.

@noisysocks
Copy link
Member

I don't have the bandwidth to do a lot here, but I can rebase it and maybe that will fix the test failures I was getting in #10267 (comment).

@ellatrix ellatrix closed this Mar 28, 2019
@ellatrix ellatrix deleted the try/reusable-block-e2e-test branch March 28, 2019 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants