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

E2E Tests: Fix Meta Boxes Test #26440

Closed
wants to merge 1 commit into from
Closed

E2E Tests: Fix Meta Boxes Test #26440

wants to merge 1 commit into from

Conversation

jeyip
Copy link
Contributor

@jeyip jeyip commented Oct 26, 2020

Description

It looks like the removal of this shim in lib/compat.php in this PR is causing an e2e spec failure.

Meta boxes › Should render dynamic blocks when the meta box uses the excerpt for front end rendering

As @talldan mentions here, the latest posts block no longer has a .wp-block-latest-posts classname on its wrapper. What's odd is that, although the classname doesn't exist in an e2e testing environment, it does appear in the development environment. I'm unsure of the cause, but while we search for the cause, I attempt to introduce this temporary fix.

The intention of the failing spec is to ensure that the latest posts block is rendered at all, so I modify the selector that identifies the block (because it is, after all, still being rendered).

Other alternatives seem to be:

  • reverting @nosolosw 's PR
  • temporarily disabling the spec
  • reintroducing the shim
  • addressing the underlying issue (I'm still not certain of the cause)

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.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@jeyip jeyip added [Type] Bug An existing feature does not function as intended [Package] E2E Tests /packages/e2e-tests labels Oct 26, 2020
@jeyip jeyip self-assigned this Oct 26, 2020
@@ -60,7 +60,7 @@ describe( 'Meta boxes', () => {
await page.waitForNavigation();

// Check the the dynamic block appears.
await page.waitForSelector( '.wp-block-latest-posts' );
await page.waitForSelector( '.wp-block-latest-posts__list' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like the absence of the .wp-block-latest-posts class is an actual bug that the failing test has caught rather than a problem with the test itself, but I could be wrong. I don't really know the background of the change that caused it.

Copy link
Contributor Author

@jeyip jeyip Oct 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just updated the description of the PR with my reasoning (sorry I jumped the gun a bit and have been adding details after the fact!).

I feel like the absence of the .wp-block-latest-posts class is an actual bug that the failing test has caught rather than a problem with the test itself

That's what I thought at first as well, but I could only reproduce the problem in the e2e testing environment, and not the dev environment. I have a hunch it might be a false positive, and thought it might be worthwhile to have a temporary fix in the meantime.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for following up on it, definitely better to be proactive 😄

I've made an issue for the bug #26441

Copy link
Contributor Author

@jeyip jeyip Oct 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick replies Daniel. Let me know if we think it's better to close this PR for now 🙂

@jeyip jeyip requested a review from oandregal October 26, 2020 06:05
@jeyip jeyip changed the title E2E Tests: Meta Boxes E2E Tests: Fix Meta Boxes Test Oct 26, 2020
@tellthemachines
Copy link
Contributor

What's odd is that, although the classname doesn't exist in an e2e testing environment, it does appear in the development environment.

Are both environments running on the same version of Core? Because the PR that caused these tests to fail introduced a bug that manifests on 5.5.x but not on 5.6.

@jeyip
Copy link
Contributor Author

jeyip commented Oct 26, 2020

Are both environments running on the same version of Core?

That's a great point @tellthemachines ! I just checked, and they're referencing the same version of core 5.6-beta1-49280.

@oandregal
Copy link
Member

Thanks for the quick replies Daniel. Let me know if we think it's better to close this PR for now slightly_smiling_face

Yep, we can close this. This issue affects WP 5.6 beta1 and will be fixed by landing WordPress/wordpress-develop#640

@oandregal oandregal closed this Oct 26, 2020
@oandregal oandregal deleted the fix/meta-boxes-e2e-test branch October 26, 2020 08:25
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 [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants