-
Notifications
You must be signed in to change notification settings - Fork 383
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
JavaScript Tests, Take 1. #2176
Conversation
580a55a
to
59797b1
Compare
page is a globa variable exposed by jest-puppeteer
There's some progress upstream! WordPress/gutenberg#15246 has been merged, and WordPress/gutenberg#15139 is close to being merged as well. Once that is done, and new package versions have been released, there should be no more transpiler errors when running tests. |
Webpack maps these to the global objects already
Webpack maps these to the global objects already
Webpack maps these to the global objects already
Webpack maps these to the global objects already
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks Good
Hi @swissspidy,
This looks good, as far as I know about JavaScript testing (much less than you) 😄
The results are similar to the ones a few days ago, though like you mentioned, I didn't run npm link
to use local packages.
Wow, 90 seconds is a lot 😬 It should actually be more like 1/10th of that... That sounds like Jest does transpile all the Gutenberg packages when running the tests or something. Hope that is just because of the |
Unless there are any objections, I am going to merge this later the day. |
@swissspidy Just for information that my computer has been having some serious thinking (and other) issues lately so it's likely my local issue only if it works OK for you -- I'll get a new one soon and then can check again. |
This PR aims to make the current JavaScript code base more testable, while also adding some initial tests to prove that the setup is working. There's still lots to do. Mainly:
This PR also affects non-Stories components, so we need to double-check whether it works and makes sense.
Ideally, shared components like the featured image notice stuff is in a common directory, instead of in only
amp-stories
orblock-editor
. See also Improve featured image notices #2038.There are currently some issues, see Update to Babel 7.4 and core-js 3 WordPress/gutenberg#15139, https://wordpress.slack.com/archives/C02QB2JS7/p1556031641343900, Include examples of tests WordPress/gutenberg-examples#26.
Might need a second pair of eyes from Gutenberg devs.
Testing Instructions
The two main things to test here are:
npm run test:js
)Note: right now, the unit test part is a bit more complex at the moment because the new Gutenberg packages haven't been published on npm yet.
In order to prevent any errors and make sure that all the tests run smoothly, you need to perform the following tasks:
Clone https://github.com/WordPress/gutenberg/ and build it.
Go to each of these folders and run
npm link
inside them:packages/babel-preset-default
packages/block-editor
packages/blocks
packages/compose
packages/data
packages/element
packages/hooks
packages/i18n
packages/wordcount
So basically this:
(cd packages/compose && npm link); (cd packages/blocks && npm link); ...
)Check out this PR branch locally
Run
npm link
for all these packages.Basically this:
npm link @wordpress/compose; npm link @wordpress/blocks; npm link @wordpress/i18n; ...
Finally, run
npm run test:js
The result should look like this:
(Yes, there are currently some failing tests; see below for the reason)
To do (ideally in some follow-up PRs)
select()
calls in tests as the current approach does not seem to be working in thetest/reducer.js
.This is more of an ongoing process
Once all tests are green, otherwise it's just gonna be a sad experience 🙂
See #2121.