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: Cypress not using type() properly on contenteditable elements #4089

Closed
mikeyarce opened this issue Dec 19, 2017 · 7 comments
Closed
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.

Comments

@mikeyarce
Copy link
Contributor

mikeyarce commented Dec 19, 2017

Update: see the latest comment.

Issue Overview

When running our e2e tests to create blocks, the <br data-mce-bogus="1"> is not being removed from the Block content, so our blocks are not created with proper formatting.

Steps to Reproduce (for bugs)

  1. Run the 002-adding-blocks.js test from /test/e2e/integration/
  2. It creates a few blocks, and it uses the type() function to type into the blocks
  3. TinyMCE isn't recognizing this as typing, so it never removes the <br data-mce-bogus="1"> from the blocks

Chrome 63, also tried using Electron on Chromium 58
Latest master of Gutenberg

Expected Behavior

TinyMCE adds this <br> as a placeholder, and when you start typing it's removed. I'm guesing it checks isEmpty to know when to keep it there and then when it's no longer empty, it removes it. I would expect this to also happen when running an e2e test.

Current Behavior

<br data-mce-bogus="1"> not removed

Possible Solution

  1. Trigger each block properly so that the elements are removed in the tests
  2. Change the check for empty blocks so that they also are aware of our e2e tests (not as good as option 1)

Screenshots / Video

tinymce-e2e

@gziolo gziolo added [Component] TinyMCE [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. labels Dec 19, 2017
@gziolo
Copy link
Member

gziolo commented Dec 19, 2017

@karmatosed can we ask someone from TinyMCE for help here?

@mikeyarce mikeyarce changed the title TinyMCE: e2e tests not removing <br data-mce-bogus="1"> TinyMCE: e2e tests not removing <br data-mce-bogus="1"> Dec 19, 2017
@mikeyarce
Copy link
Contributor Author

I found a workaround for removing the <br data-mce-bogus="1"> - you have to invoke text into the field first like this: .invoke('text', '/quote)`

This does bring a different problem though, as it doesn't move the cursor with the text, even if you use type() after, so any returns you do will actually create blocks ABOVE the current block being tested.

block2

@mikeyarce
Copy link
Contributor Author

Update: I found a bug in Cypress which is only causing this caret/cursor issue for contenteditable elements. It's not a Gutenberg problem or a React issue, just how our e2e tester is firing change on this element.

You can see cypress-io/cypress#1108 for more details.

@mikeyarce mikeyarce reopened this Dec 20, 2017
@mikeyarce mikeyarce changed the title TinyMCE: e2e tests not removing <br data-mce-bogus="1"> E2E tests: Cypress not using type() properly on contenteditable elements Dec 20, 2017
@BoardJames
Copy link

Typing in cypress seems to be all simulated events, it does not update the window selection so anything that tracks the cursor (like the quick inserter) does not work. I have a branch where I implemented selenium tests which work correctly:
#3664

So far I have had nothing but push-back on Selenium testing though (despite it being the defacto standard).

@mikeyarce
Copy link
Contributor Author

Thanks for your input @EphoxJames !

Typing in cypress seems to be all simulated events, it does not update the window selection so anything that tracks the cursor (like the quick inserter) does not work. I have a branch where I implemented selenium tests which work correctly:

In my Cypress tests, typing works well in many blocks, specifically any that use an input element. The issue right now is only when using a contenteditable type of element and the type() command. That's when the caret doesn't move and can cause inconsistencies in the tests.

I agree Selenium is the current defacto standard, but having compared it to Cypress, I prefer Cypress because of the ease of use, the code reads better, it's easier to learn (better documentation), and it has some advanced features like the DOM timeline inspector that is really nice for debugging issues especially with things like Gutenberg. Cypress will be the testing of the future, so I think it makes sense to start there and help it grow alongside Gutenberg.

That's just my personal opinion! If Cypress doesn't fix the current issues, we will have to look at something like Selenium or rely on more manual methods for now. I'm curious to hear from others over their preferences for E2E testing and what they think.

@BoardJames
Copy link

The selenium docs for javascript aren't that bad but strangely they are very hard to find:
https://seleniumhq.github.io/selenium/docs/api/javascript/index.html

@gziolo
Copy link
Member

gziolo commented Mar 23, 2018

We decided to discontinue our journey with Cypress.io and start a closer relationship with Puppeteer. See #5618 :)

@gziolo gziolo closed this as completed Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

No branches or pull requests

3 participants