Scripts: Install Chromium on demand together with test-e2e
#20215
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gziolo
requested review from
ajitbohra,
nerrad,
ntwb,
talldan and
youknowriad
as code owners
gziolo
added
[Tool] WP Scripts
gziolo
force-pushed
the
update/puppeteer-core
branch
from
662ee6a
to
7190f36
Compare
gziolo
force-pushed
the
update/puppeteer-core
branch
2 times, most recently
from
a931f10
to
8b1523f
Compare
noahtallen
requested changes
Feb 13, 2020
|
One thing that I don't quite understand is why I see the following diff after: $ rm -rf node_modules
$ npm idiff --git a/package-lock.json b/package-lock.json
index b4c56d406..0c6cfb355 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -9625,12 +9625,6 @@
"integrity": "sha512-Il2DtDVRGDcqjDtE+rF8iqg1CArehSK84HZJCT7AMITlyXRBpuPhqGLDQMowraqqu1coEaimg4ZOqggt6L6L+A==",
"dev": true
},
- "@types/mime-types": {
- "version": "2.1.0",
- "resolved": "https://registry.npmjs.org/@types/mime-types/-/mime-types-2.1.0.tgz",
- "integrity": "sha1-nKUs2jY/aZxpRmwqbM2q2RPqenM=",
- "dev": true
- },
"@types/minimatch": {
"version": "3.0.3",
"resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-3.0.3.tgz",
@@ -10785,7 +10779,6 @@
"minimist": "^1.2.0",
"npm-package-json-lint": "^4.0.3",
"prettier": "npm:[email protected]",
- "puppeteer": "npm:[email protected]",
"read-pkg-up": "^1.0.1",
"request": "^2.88.0",
"resolve-bin": "^0.4.0",
@@ -18775,29 +18768,6 @@
}
}
},
- "extract-zip": {
- "version": "1.6.7",
- "resolved": "https://registry.npmjs.org/extract-zip/-/extract-zip-1.6.7.tgz",
- "integrity": "sha1-qEC0uK9kAyZMjbV/Txp0Mz74H+k=",
- "dev": true,
- "requires": {
- "concat-stream": "1.6.2",
- "debug": "2.6.9",
- "mkdirp": "0.5.1",
- "yauzl": "2.4.1"
- },
- "dependencies": {
- "debug": {
- "version": "2.6.9",
- "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz",
- "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==",
- "dev": true,
- "requires": {
- "ms": "2.0.0"
- }
- }
- }
- },
"extsprintf": {
"version": "1.3.0",
"resolved": "https://registry.npmjs.org/extsprintf/-/extsprintf-1.3.0.tgz",
@@ -18959,15 +18929,6 @@
}
}
},
- "fd-slicer": {
- "version": "1.0.1",
- "resolved": "https://registry.npmjs.org/fd-slicer/-/fd-slicer-1.0.1.tgz",
- "integrity": "sha1-i1vL2ewyfFBBv5qwI/1nUPEXfmU=",
- "dev": true,
- "requires": {
- "pend": "~1.2.0"
- }
- },
"figgy-pudding": {
"version": "3.5.1",
"resolved": "https://registry.npmjs.org/figgy-pudding/-/figgy-pudding-3.5.1.tgz",
@@ -30599,12 +30560,6 @@
"resolved": "https://registry.npmjs.org/pegjs/-/pegjs-0.10.0.tgz",
"integrity": "sha1-z4uvrm7d/0tafvsYUmnqr0YQ3b0="
},
- "pend": {
- "version": "1.2.0",
- "resolved": "https://registry.npmjs.org/pend/-/pend-1.2.0.tgz",
- "integrity": "sha1-elfrVQpng/kRUzH89GY9XI4AelA=",
- "dev": true
- },
"performance-now": {
"version": "2.1.0",
"resolved": "https://registry.npmjs.org/performance-now/-/performance-now-2.1.0.tgz",
@@ -32019,12 +31974,6 @@
"ipaddr.js": "1.8.0"
}
},
- "proxy-from-env": {
- "version": "1.0.0",
- "resolved": "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-1.0.0.tgz",
- "integrity": "sha1-M8UDmPcOp+uW0h97gXYwpVeRx+4=",
- "dev": true
- },
"prr": {
"version": "1.0.1",
"resolved": "https://registry.npmjs.org/prr/-/prr-1.0.1.tgz",
@@ -32257,81 +32206,6 @@
"integrity": "sha512-XRsRjdf+j5ml+y/6GKHPZbrF/8p2Yga0JPtdqTIY2Xe5ohJPD9saDJJLPvp9+NSBprVvevdXZybnj2cv8OEd0A==",
"dev": true
},
- "puppeteer": {
- "version": "npm:[email protected]",
- "resolved": "https://registry.npmjs.org/puppeteer-core/-/puppeteer-core-2.1.1.tgz",
- "integrity": "sha512-n13AWriBMPYxnpbb6bnaY5YoY6rGj8vPLrz6CZF3o0qJNEwlcfJVxBzYZ0NJsQ21UbdJoijPCDrM++SUVEz7+w==",
- "dev": true,
- "requires": {
- "@types/mime-types": "^2.1.0",
- "debug": "^4.1.0",
- "extract-zip": "^1.6.6",
- "https-proxy-agent": "^4.0.0",
- "mime": "^2.0.3",
- "mime-types": "^2.1.25",
- "progress": "^2.0.1",
- "proxy-from-env": "^1.0.0",
- "rimraf": "^2.6.1",
- "ws": "^6.1.0"
- },
- "dependencies": {
- "agent-base": {
- "version": "5.1.1",
- "resolved": "https://registry.npmjs.org/agent-base/-/agent-base-5.1.1.tgz",
- "integrity": "sha512-TMeqbNl2fMW0nMjTEPOwe3J/PRFP4vqeoNuQMG0HlMrtm5QxKqdvAkZ1pRBQ/ulIyDD5Yq0nJ7YbdD8ey0TO3g==",
- "dev": true
- },
- "debug": {
- "version": "4.1.1",
- "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.1.tgz",
- "integrity": "sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw==",
- "dev": true,
- "requires": {
- "ms": "^2.1.1"
- }
- },
- "https-proxy-agent": {
- "version": "4.0.0",
- "resolved": "https://registry.npmjs.org/https-proxy-agent/-/https-proxy-agent-4.0.0.tgz",
- "integrity": "sha512-zoDhWrkR3of1l9QAL8/scJZyLu8j/gBkcwcaQOZh7Gyh/+uJQzGVETdgT30akuwkpL8HTRfssqI3BZuV18teDg==",
- "dev": true,
- "requires": {
- "agent-base": "5",
- "debug": "4"
- }
- },
- "mime-db": {
- "version": "1.43.0",
- "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.43.0.tgz",
- "integrity": "sha512-+5dsGEEovYbT8UY9yD7eE4XTc4UwJ1jBYlgaQQF38ENsKR3wj/8q8RFZrF9WIZpB2V1ArTVFUva8sAul1NzRzQ==",
- "dev": true
- },
- "mime-types": {
- "version": "2.1.26",
- "resolved": "https://registry.npmjs.org/mime-types/-/mime-types-2.1.26.tgz",
- "integrity": "sha512-01paPWYgLrkqAyrlDorC1uDwl2p3qZT7yl806vW7DvDoxwXi46jsjFbg+WdwotBIk6/MbEhO/dh5aZ5sNj/dWQ==",
- "dev": true,
- "requires": {
- "mime-db": "1.43.0"
- }
- },
- "ms": {
- "version": "2.1.2",
- "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz",
- "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==",
- "dev": true
- },
- "ws": {
- "version": "6.2.1",
- "resolved": "https://registry.npmjs.org/ws/-/ws-6.2.1.tgz",
- "integrity": "sha512-GIyAXC2cB7LjvpgMt9EKS2ldqr0MTrORaleiOno6TweZ6r3TKtoFQWay/2PceJ3RuBasOHzXNn5Lrw1X0bEjqA==",
- "dev": true,
- "requires": {
- "async-limiter": "~1.0.0"
- }
- }
- }
- },
"q": {
"version": "1.5.1",
"resolved": "https://registry.npmjs.org/q/-/q-1.5.1.tgz",
@@ -40738,15 +40612,6 @@
}
}
},
- "yauzl": {
- "version": "2.4.1",
- "resolved": "https://registry.npmjs.org/yauzl/-/yauzl-2.4.1.tgz",
- "integrity": "sha1-lSj0QtqxsihOWLQ3m7GU4i4MQAU=",
- "dev": true,
- "requires": {
- "fd-slicer": "~1.0.1"
- }
- },
"zwitch": {
"version": "1.0.4",
"resolved": "https://registry.npmjs.org/zwitch/-/zwitch-1.0.4.tgz",When I run @jsnajdr, did you encounter something similar when aliasing |
|
I'll take this comment out of the resolved thread so it has more visibility now that the older issue was solved :) I see this issue running the script locally. I wonder if @wordpress/scripts might be calling puppeteer expecting a certain API, but puppeteer-core has a different API? > npx wp-scripts test-e2e --wordpress-base-url='http://localhost:4013'
Chromium downloaded to /Users/noah.allen/source/gutenberg/packages/scripts/node_modules/puppeteer/.local-chromium/mac-722234
● Validation Error:
Preset jest-puppeteer is invalid:
The "id" argument must be of type string. Received type object
TypeError [ERR_INVALID_ARG_TYPE]: The "id" argument must be of type string. Received type object
at validateString (internal/validators.js:112:11)
at Module.require (internal/modules/cjs/loader.js:841:3)
at require (internal/modules/cjs/helpers.js:74:18)
at setupPreset (/Users/noah.allen/source/gutenberg/packages/scripts/node_modules/jest-config/build/normalize.js:230:14)
at normalize (/Users/noah.allen/source/gutenberg/packages/scripts/node_modules/jest-config/build/normalize.js:554:15)
at readConfig (/Users/noah.allen/source/gutenberg/packages/scripts/node_modules/jest-config/build/index.js:163:46)
at readConfigs (/Users/noah.allen/source/gutenberg/packages/scripts/node_modules/jest-config/build/index.js:373:26)
at /Users/noah.allen/source/gutenberg/packages/scripts/node_modules/@jest/core/build/cli/index.js:155:58
at Generator.next (<anonymous>)
at asyncGeneratorStep (/Users/noah.allen/source/gutenberg/packages/scripts/node_modules/@jest/core/build/cli/index.js:108:24)
Configuration Documentation:
https://jestjs.io/docs/configuration.html |
|
@noahtallen - the difference between However, there are some other checks in the code that I need to double-check now. In addition, I plan to bump Update: I found other places where https://github.com/puppeteer/puppeteer/search?q=isPuppeteerCore&unscoped_q=isPuppeteerCore The most important part is here: I don't think we use those env variables. |
aduth
reviewed
Feb 14, 2020
|
I opened #20268 to perform Puppeteer upgrade in smaller steps. |
This was referenced Feb 28, 2020
Co-Authored-By: Andrew Duthie <[email protected]>
gziolo
force-pushed
the
update/puppeteer-core
branch
from
d080809
to
fca0094
Compare
|
Size Change: +1.02 kB (0%) Total Size: 841 kB
ℹ️ View Unchanged
|
gziolo
commented
Apr 17, 2020
| @@ -60,7 +60,7 @@ | |||
| "minimist": "^1.2.0", | |||
| "npm-package-json-lint": "^5.0.0", | |||
| "prettier": "npm:[email protected]", | |||
| "puppeteer": "^2.0.0", | |||
| "puppeteer": "npm:puppeteer-core@3.0.0", | |||
There was a problem hiding this comment.
It looks like aliasing is required only temporary until a new version of jest-environment-puppeteer is published:
argos-ci/jest-puppeteer#315
There was a problem hiding this comment.
The e2e tests are failing in WordPress/wordpress-develop#347. Does this have something to do with it?
|
There is still one test suite failing with the following error:
I have no idea where it comes from. It works with the same file uploaded using the Classic Media modal. I'm able to upload the same file manually when using Puppeteer in the interactive mode 🤷♂️ |
|
I tried different file types: I recreated the scenario where we upload files through Media modal and it worked just fine. I tried to apply the same attributes ( Should I skip those tests for now and proceed? I don't know why the type of image gets set to undefined... |
|
This fixes this issue: diff --git a/packages/media-utils/src/utils/upload-media.js b/packages/media-utils/src/utils/upload-media.js
index fb31af14cf..b80f2e85b5 100644
--- a/packages/media-utils/src/utils/upload-media.js
+++ b/packages/media-utils/src/utils/upload-media.js
@@ -121,6 +121,7 @@ export async function uploadMedia( {
// verify if user is allowed to upload this mime type
if (
allowedMimeTypesForUser &&
+ mediaFile.type &&
! isAllowedMimeTypeForUser( mediaFile.type )
) {
triggerError( {
@@ -134,7 +135,7 @@ export async function uploadMedia( {
}
// Check if the block supports this mime type
- if ( ! isAllowedType( mediaFile.type ) ) {
+ if ( mediaFile.type && ! isAllowedType( mediaFile.type ) ) {
triggerError( {
code: 'MIME_TYPE_NOT_SUPPORTED',
message: __( 'Sorry, this file type is not supported here.' ),It looks like I also see the following note in bold at https://developer.mozilla.org/en-US/docs/Web/API/File/type
|
noahtallen
approved these changes
Apr 17, 2020
There was a problem hiding this comment.
The code makes sense to me here! 🚀
Giving a plus one because:
- ✅ Gutenberg e2e tests are passing (🎉)
- ✅ Not seeing chromium install in Gutenberg on
npm iin gutenberg
Additionally, I tested this (via a file ref with this branch checked out) in a 3rd party plugin I develop, and:
- ✅ No chromium install on
npm i - ✅ When I run test e2e there, chromium is installed and I can see its progress indicator
- ✅ The e2e test that I have completes successfully
|
strangely, I'm seeing this error again in my plugin: #20215 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #15667.
This change landed upstream and we can now explore ways to improve the flow in Gutenberg.
The bundled
puppeteerdependency in version^2.0.0has been replaced withpuppeteer-corerequiring3.0.0. It allowed preventing Chromium installation together with@wordpress/scripts. It happens now on-demand when runningtest-e2escript and only when a new version is required.Implementation notes
I had to use a similar trick that we used for
puppeteerwith npm aliases to ensure thatpuppeteer-coreintegrates properly withjest-puppeteer. I'm still not sure whether it's necessary to explore ways of integratingpuppeteer-coredirectly intojest-puppeteeras it isn't the most common use case.I tried first to require
puppeteer/installbecause it is a regular Node script. However, it downloads Chromium using Promises so I had to spawn the process to ensure that the process finishes first.I don't know if there is a better way to do it so I'm happy to improve the logic :)