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 - update site editor component waitForTemplateParts method. #56441

Merged

Conversation

Addison-Stavlo
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo commented Sep 21, 2021

Changes proposed in this Pull Request

Updates the waitForTemplateParts method to be more accurate to its purpose. Fixes tests specs/specs-wpcom/wp-calypso-gutenberg-site-editor-tracking-spec.js and specs/specs-wpcom/wp-site-editor-spec.js

Currently, this method waits for the following selector to not exist in the editor .wp-block-template-part .components-spinner. However, it is possible (and has happened with recent theme updates) that other blocks inside a template part may have their own spinner components.

Here, we update this to only be concerned with spinners that are direct children of the template part. As this is how gutenberg renders the template part loading state wp-block-template-part > .components-spinner.

We have also added another condition to ensure the template part is not in a warning state .wp-block-template-part > .block-editor-warning. Previously, when template parts were not resolved the spinner component would be in place perpetually. With more recent Gutenberg updates, when the template part cannot be resolved it renders a warning component. Thus this extra condition now helps ensure that the template part is properly loaded.

Testing instructions

  • Run the specs/specs-wpcom/wp-site-editor-spec.js test file and verify it no longer fails on the template part assertion.
  • Run the specs/specs-wpcom/wp-calypso-gutenberg-site-editor-tracking-spec.js and verify it no longer fails in the first test.

Related to #

@Addison-Stavlo Addison-Stavlo self-assigned this Sep 21, 2021
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 21, 2021
@github-actions
Copy link

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

dpasque
dpasque previously approved these changes Sep 23, 2021
Copy link
Contributor

@dpasque dpasque left a comment

Choose a reason for hiding this comment

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

Hrm, actually, now it looks like i'm getting a failure in the wp-site-editor test!

  [WPCOM] Site Editor (desktop) @parallel
    ✓ Can log in (16434ms)
    ✓ Can open site editor (547ms)
    ✓ Site editor opens (207ms)
    1) Template loads


  3 passing (48s)
  1 failing

  1) [WPCOM] Site Editor (desktop) @parallel
       Template loads:
     Error: TimeoutError: Waiting to be able to switch to frame
Wait timed out after 20109ms
      at Proxy.<anonymous> (lib/driver-manager.js:97:14)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)
      at async Proxy.waitUntilAbleToSwitchToFrame (lib/driver-helper.js:194:9)
      at async SiteEditorComponent.runInCanvas (lib/components/site-editor-component.js:17:3)
      at async SiteEditorComponent.waitForTemplateToLoad (lib/components/site-editor-component.js:37:3)
      at async Context.<anonymous> (specs/specs-wpcom/wp-site-editor-spec.js:37:3)

@dpasque dpasque dismissed their stale review September 23, 2021 15:44

Test is failing

@dpasque
Copy link
Contributor

dpasque commented Sep 23, 2021

Actually, both of these are failing on my machine on trunk... Hmm... I'm going to try to trigger this for this branch on TeamCity and see how it runs there!

@Addison-Stavlo
Copy link
Contributor Author

👋 @dpasque

Actually, both of these are failing on my machine on trunk

They should fail on trunk as the fix here is necessary.

it looks like i'm getting a failure in the wp-site-editor test!

It looks like im getting this failure now too. It seems like an issue with the test site as I see the site editor is not actually loading there now (gray blank editor screen). Investigating...

@dpasque
Copy link
Contributor

dpasque commented Sep 23, 2021

They should fail on trunk as the fix here is necessary.

Oh, duh! 🤦 I clearly didn't get enough sleep last night. 🙃

gray blank editor screen

That's what I was seeing too!

@Addison-Stavlo
Copy link
Contributor Author

Addison-Stavlo commented Sep 23, 2021

Its interesting, I cannot repro with my own test sites. But loading up the e2e sites for the site editor (both edge and non-edge) seem to do this.

@dpasque
Copy link
Contributor

dpasque commented Sep 23, 2021

Its interesting, I cannot repro with my own test sites. But loading up the e2e sites for the site editor (both edge and non-edge) seem to do this.

Weird! Should we spin up a separate issue for this you think?

I'm also good with this merging too! The code changes look fine and we can have the fixed tests then to validate this other issue once it's fixed!

Copy link
Contributor

@dpasque dpasque left a comment

Choose a reason for hiding this comment

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

Failure is unrelated, approving for now!

@Addison-Stavlo
Copy link
Contributor Author

Weird! Should we spin up a separate issue for this you think?

Yeah, il spin up an issue. I can find a couple of my own test sites that are like this as well (and others are fine! 😕 ). 😆

@Addison-Stavlo Addison-Stavlo merged commit ba7eb23 into trunk Sep 23, 2021
@Addison-Stavlo Addison-Stavlo deleted the fix/site-editor-component-wait-for-template-parts-method branch September 23, 2021 17:30
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 23, 2021
@Addison-Stavlo
Copy link
Contributor Author

Addison-Stavlo commented Sep 23, 2021

created an issue for the gray screen bug here - #56512

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants