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: Instability in "embedding content" tests #19033

Closed
aduth opened this issue Dec 10, 2019 · 2 comments · Fixed by #19042
Closed

E2E Tests: Instability in "embedding content" tests #19033

aduth opened this issue Dec 10, 2019 · 2 comments · Fixed by #19042
Assignees
Labels
[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. [Type] Bug An existing feature does not function as intended

Comments

@aduth
Copy link
Member

aduth commented Dec 10, 2019

A frequent build failure occurs due to a failing test in the "Embedding content" test suite:

FAIL packages/e2e-tests/specs/editor/various/embedding.test.js (30.078s)
  Embedding content
    ✕ should render embeds in the correct state (6576ms)

● Embedding content › should render embeds in the correct state
    Node is either not visible or not an HTMLElement
      29 |             _context.next = 2;
      30 |             return page.click('.block-editor-default-block-appender__content');
    > 31 | 
         | ^
      32 |           case 2:
      33 |           case "end":
      34 |             return _context.stop();
      at ElementHandle._clickablePoint (../../node_modules/puppeteer/lib/JSHandle.js:222:13)
          at runMicrotasks (<anonymous>)
      at ElementHandle.click (../../node_modules/puppeteer/lib/JSHandle.js:283:20)
      at DOMWorld.click (../../node_modules/puppeteer/lib/DOMWorld.js:367:5)
        -- ASYNC --
      at ElementHandle.<anonymous> (../../node_modules/puppeteer/lib/helper.js:111:15)
      at DOMWorld.click (../../node_modules/puppeteer/lib/DOMWorld.js:367:18)
          at runMicrotasks (<anonymous>)
        -- ASYNC --
      at Frame.<anonymous> (../../node_modules/puppeteer/lib/helper.js:111:15)
      at Page.click (../../node_modules/puppeteer/lib/Page.js:1067:29)
      at _callee$ (../e2e-test-utils/build/click-block-appender.js:31:25)
      at tryCatch (../../node_modules/regenerator-runtime/runtime.js:45:40)
      at Generator.invoke [as _invoke] (../../node_modules/regenerator-runtime/runtime.js:271:22)
      at Generator.prototype.<computed> [as next] (../../node_modules/regenerator-runtime/runtime.js:97:21)
      at asyncGeneratorStep (../../node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (../../node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
      at ../../node_modules/@babel/runtime/helpers/asyncToGenerator.js:32:7

Initial assessment:

Based on the error and relevant for some previous issues which have been encountered, it can sometimes be problematic to try to click an element which is "off-screen". The viewport for the E2E tests is quite small, so there's a high likelihood that inserting embeds could push the block appender down the page. That being said, page.click is documented as intending to bring the element on-screen, so it's not entirely clear that this should be necessary:

This method fetches an element with selector, scrolls it into view if needed, and then uses page.mouse to click in the center of the element. If there's no element matching selector, the method throws an error.

https://github.com/puppeteer/puppeteer/blob/v2.0.0/docs/api.md#pageclickselector-options

If there could be another way to remove or otherwise express this desire to add a new default block which does not depend as much on cursor or element positioning, it may be worth exploring as an alternative implementation of the shared clickBlockAppender utility function.

@aduth aduth added [Type] Bug An existing feature does not function as intended [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. labels Dec 10, 2019
@aduth
Copy link
Member Author

aduth commented Dec 10, 2019

That being said, page.click is documented as intending to bring the element on-screen, so it's not entirely clear that this should be necessary:

A suspicion I had here is whether this scrolling behavior would have difficulty with individual scrollable panels, such as the block list area of the editor. Based on the relevant implementation (source), I would expect that this should be working correctly (CodePen example).

My next instinct is: Embed content can load asynchronously, and the rendered preview is shown in a resizable iframe. It might be possible there's a race condition between when the "Block Appender" element is brought into view, vs. the resize of the iframe pushing the appender back out of view.

The behavior of embeds in E2E tests is intended that the loading of embeds should occur instantaneously via response mocking (source). There's of course a chance that this is not working as expected.

@aduth
Copy link
Member Author

aduth commented Dec 10, 2019

Another observation: The E2E viewport is so small that the "Common" block insertion options can overlap much of the default block appender.

image

Because the default behavior of page.click is to "click in the center of the element" (documentation), there's a chance that the click intended for the appender is actually being performed on one of the common block options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[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. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant