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

Testing: Add E2E test for inserting reusable blocks by inserter after page refresh #9962

Closed
aduth opened this issue Sep 17, 2018 · 9 comments · Fixed by #9992 or #20605
Closed

Testing: Add E2E test for inserting reusable blocks by inserter after page refresh #9962

aduth opened this issue Sep 17, 2018 · 9 comments · Fixed by #9992 or #20605
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Status] In Progress Tracking issues with work in progress [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Milestone

Comments

@aduth
Copy link
Member

aduth commented Sep 17, 2018

To avoid repeated regressions, we should include an end-to-end test for inserting a reusable block by Inserter in a fresh editor page. See #9962 (comment) .

Original Regression:


Describe the bug

Reusable blocks are not shown in the Inserter.

To Reproduce

  1. (Prerequisite) Create a reusable block in a Gutenberg editor session
  2. Create a new post
  3. Try to insert the reusable block (either one preexisting, or the one created in Step 1) using the Inserter.
  4. Observe that the reusable block is not shown in the inserter.

Expected behavior

Reusable block is both shown and searchable.

Additional context

Slack mentions:

From @youknowriad:

I think we do have an e2e test, what's missing is an e2e test inserting a reusable block after refreshing the page.

@aduth aduth added [Type] Bug An existing feature does not function as intended [Feature] Inserter The main way to insert blocks using the + button in the editing interface [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) labels Sep 17, 2018
@aduth aduth added this to the 3.9 milestone Sep 17, 2018
@aduth aduth changed the title Inserter: Insert doesn't show reusable blocks Inserter: Inserter doesn't show reusable blocks Sep 17, 2018
@aduth aduth assigned aduth and unassigned aduth Sep 17, 2018
@aduth
Copy link
Member Author

aduth commented Sep 17, 2018

Workaround: The reusable blocks appear correctly when the Inserter is closed and reopened again. It appears to hint to a possible race condition on this logic:

// TODO: these are potentially undefined, this fix is in place
// until there is a filter to not use reusable blocks if undefined
const postType = await resolveSelector( 'core', 'getPostType', 'wp_block' );
if ( ! postType ) {
return;
}

cc @youknowriad Does any of this sound familiar? Perhaps related to recent work in #9732?

@aduth
Copy link
Member Author

aduth commented Sep 17, 2018

I'd suggest we approach this from a TDD angle, updating the E2E reusable block insertion case to a failing state and working backward to a resolution. Based on the above workaround, I expect it may start failing if we ensured a fresh post for each test case run, i.e. using beforeEach instead of the current beforeAll (cc @noisysocks).

@youknowriad
Copy link
Contributor

the recent changes have been merged after the 3.9 RC, so while it's related, I think the issue was there before.

@aduth
Copy link
Member Author

aduth commented Sep 17, 2018

I checked out a detached head at both 41d4223 and 41d4223~1 and it worked in both cases, which led me to believe that it might not have been directly related to #9732, but became introduced afterward.

@youknowriad
Copy link
Contributor

Oh sorry, I'm tired, I thought you were linking to the resolvers refactoring that was recently merged. I can take a look at this issue tomorrow hopefully.

@aduth
Copy link
Member Author

aduth commented Sep 18, 2018

Reopening as we should have an end-to-end test.

@aduth aduth reopened this Sep 18, 2018
@aduth aduth changed the title Inserter: Inserter doesn't show reusable blocks Testing: Add E2E test for inserting reusable blocks by inserter Sep 18, 2018
@youknowriad
Copy link
Contributor

I think we do have an e2e test, what's missing is an e2e test inserting a reusable block after refreshing the page.

@aduth
Copy link
Member Author

aduth commented Sep 18, 2018

Yes, the test was obviously not solid enough, based on the presence of the bug. I'd say in general we should have each and every test case working from a new post†.

† I think there's still some room to optimize here, if performance is a concern. Artificially resetting the editor state to its initial value without actually triggering a reload.

@ellatrix ellatrix self-assigned this Sep 30, 2018
@tellyworth
Copy link
Contributor

Based on the above workaround, I expect it may start failing if we ensured a fresh post for each test case run, i.e. using beforeEach instead of the current beforeAll (cc @noisysocks).

Experimenting with this, we've encountered some other issues to do with tab focusing that interfere with the tests. Tab behaviour is different when starting with a new post for unknown reasons. We're investigating that further.

@aduth aduth modified the milestones: 4.0, 4.1 Oct 5, 2018
@ellatrix ellatrix removed their assignment Oct 19, 2018
@youknowriad youknowriad removed this from the 4.2 milestone Oct 27, 2018
@youknowriad youknowriad added this to the WordPress 5.0 milestone Oct 27, 2018
@mtias mtias modified the milestones: WordPress 5.0 RC, Future: 5.1 Nov 12, 2018
@designsimply designsimply added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. Needs Tests labels Dec 17, 2018
@gziolo gziolo added Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts labels Apr 23, 2019
@gziolo gziolo changed the title Testing: Add E2E test for inserting reusable blocks by inserter Testing: Add E2E test for inserting reusable blocks by inserter after page refresh Apr 23, 2019
@youknowriad youknowriad removed the [Type] Bug An existing feature does not function as intended label Nov 18, 2019
@noisysocks noisysocks added the [Status] In Progress Tracking issues with work in progress label Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Status] In Progress Tracking issues with work in progress [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
8 participants