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

Upgrade Puppeteer to the latest version (1.19.0) #16875

Merged
merged 15 commits into from Aug 7, 2019

Conversation

@ellatrix
Copy link
Member

commented Aug 2, 2019

Description

Supersedes and closes #14986.

This PR tries to upgrade Puppeteer and fix failing tests.

Updates puppeteer, expect-puppeteer, and jest-puppeteer.
Updates RichText to set internal selection at the earliest occasion.
Updates some e2e tests, some updates are takes from #14986.
Adds an e2e test for #16857.

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.
@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2019

@gziolo I think there is a big change that this PR will fix intermittent failures.

@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2019

Let's see what Travis thinks.

@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2019

Whyyyyyyy does Travis fail now? It passes locally!

@gziolo

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

Whyyyyyyy does Travis fail now? It passes locally!

It looks like one of those errors we encounter in Puppeteer upgrade branch. For what it's worth, this is a good sign :)

Kudos for working on those tests 🥇

@ellatrix ellatrix requested a review from nosolosw as a code owner Aug 2, 2019

@ellatrix ellatrix force-pushed the fix/paste-after-focus-e2e branch from 2805384 to 9bcb247 Aug 2, 2019

@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2019

@gziolo Any idea why Travis runs into Error: Cannot find module 'puppeteer' when upgrading it?

@ellatrix ellatrix force-pushed the fix/paste-after-focus-e2e branch from 9bcb247 to 8eb6500 Aug 2, 2019

@gziolo

This comment has been minimized.

Copy link
Member

commented Aug 5, 2019

I pushed 19eddc1 which hopefully will resolve issues with the npm dependencies setup.

@gziolo

This comment has been minimized.

Copy link
Member

commented Aug 5, 2019

@ellatrix - I see some failure but I guess most of them can be fixed with cherry-picking fixes from my branch with Puppeteer upgrade. It looks like you managed to fix the issues with intermittent test failures for e2e tests related to writing flow 🎉

I hope I don't celebrate prematurely :)

@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2019

@gziolo Thanks! I'm going to work on this in just a bit. :)

@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2019

@gziolo How did you fix the package lock file?

@gziolo gziolo force-pushed the fix/paste-after-focus-e2e branch from 7a23869 to bb406e6 Aug 6, 2019

@gziolo

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

This PR started as an attempt to write an e2e test for #16857, but I couldn't get it to pass on Travis. I'll try it again separately at some point.

I tested paste from outside of the browser. With this branch, I can see the issue again where RichText content gets duplicated.

@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

@gziolo Which steps do you use to reproduce the issue? Are you sure this branch is checked out? For me it seems to work correctly.

@ellatrix ellatrix force-pushed the fix/paste-after-focus-e2e branch from ce4e9f5 to c7dc0ae Aug 6, 2019

@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

Added a new, simplified e2e test that fails before the #16857 fix was merged, and passes locally. Let's see what Travis thinks.

@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

The latest e2e test should prove that #15724 is still fixed in this branch. The test would fail when run before the fix was merged into master.

@ellatrix ellatrix added this to the Gutenberg 6.3 milestone Aug 6, 2019

@gziolo

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

This is exactly what I reported from Gutenberg 6.2. It was fixed in master. Now, I see it again:

copy-paste-issue

You can also replicate by going to a different tab in Chrome and copying some text.

@gziolo

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

Other than that this branch is ready to go

@ellatrix ellatrix force-pushed the fix/paste-after-focus-e2e branch from 6f69301 to 260a79a Aug 6, 2019

@gziolo
gziolo approved these changes Aug 7, 2019
Copy link
Member

left a comment

I can confirm that copy&paste works properly with the last commit applied.

This PR is ready to go. Huge props for making this happen! 🥇

@gziolo gziolo merged commit 005e030 into master Aug 7, 2019

1 of 7 checks passed

Filter opened Filter opened
Details
Filter opened Filter opened
Details
Filter opened Filter opened
Details
Filter opened Filter opened
Details
Milestone It Milestone It
Details
Milestone It Milestone It
Details
Travis CI - Pull Request Build Passed
Details

@gziolo gziolo deleted the fix/paste-after-focus-e2e branch Aug 7, 2019

@gziolo gziolo changed the title Upgrade Puppeteer Upgrade Puppeteer to the latest version (1.19.0) Aug 7, 2019

@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2019

Thanks for reviewing!

gziolo added a commit that referenced this pull request Aug 29, 2019
Upgrade Puppeteer (#16875)
* Update demo test to make it work with Vimeo mock

* Add e2e test

* Update selection on mouseup/keyup/touchend

* Try to fix e2e tests

* Attempt to upgrade Puppeteer

* Fix package-lock.json file

* Fix preview e2e

* Update snapshot

* Stabilise block hierarchy tests

* Remove new e2e test

* Simplify loop

* Remove puppeteer from devDependencies

* Remove cancelAnimationFrame

* Add e2e test for #16857

* Restore animation frame fix
gziolo added a commit that referenced this pull request Aug 29, 2019
Upgrade Puppeteer (#16875)
* Update demo test to make it work with Vimeo mock

* Add e2e test

* Update selection on mouseup/keyup/touchend

* Try to fix e2e tests

* Attempt to upgrade Puppeteer

* Fix package-lock.json file

* Fix preview e2e

* Update snapshot

* Stabilise block hierarchy tests

* Remove new e2e test

* Simplify loop

* Remove puppeteer from devDependencies

* Remove cancelAnimationFrame

* Add e2e test for #16857

* Restore animation frame fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.