(5P) SPT add JS Functional tests to the plugin #35379

getdave · 2019-08-14T16:11:46Z

This PR aims to add a JS unit test framework to the SPT Plugin via @wordpress/scripts.

However, as Gutenberg Core is considering dropping Enzyme the tests are written in React Testing Library. There is also a custom Jest config to allow us to overide the default settings for our Plugin as required.

All components are now covered by a test. Note that some components do not have their own individual test because they are covered by the parent component as we no longer use shallow rendering.

Please note

Some devDependencies have been introduced. These are entirely necessary and have been agreed with @sirreal (see comments below) as being valid.
The number of lines of code change figure is high. This is however, due to package-lock.json and @wordpress/scripts dependency. Don't panic.

Testing instructions

npm install within the apps/full-site-editing dir
Stay within the apps/full-site-editing dir
Run npm run test:unit:watch
See some example tests pass.
Write some more tests to see if they get picked up.

matticbot · 2019-08-14T16:11:48Z

Test live: https://calypso.live/?branch=try/spt-add-unit-tests

obenland · 2019-08-14T16:15:46Z

@getdave You're adding 12k lines of code? :)

getdave · 2019-08-14T16:18:10Z

Wowzers. Why is package-lock.json so large? That's the culprit.

matticbot · 2019-08-14T16:21:06Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

gwwar · 2019-08-14T18:04:28Z

Did we commit the package.lock? The dependencies in package.json can likely be removed in a janiortorial all of our dependencies are externalized

getdave · 2019-08-21T15:47:22Z

~~@obenland @gwwar Looks like a good 'ol rebase sorted it~~ Scratch that it didn't. package-lock.json is massive. I have no idea why although "@wordpress/scripts" probably has a lot of deps.

getdave · 2019-08-23T09:44:00Z

Did we commit the package.lock

To be clear @gwwar @obenland we are supposed to commit the package-lock file. I believe it is large due to @wordpress/scripts dep being added.

mmtr · 2019-08-24T02:06:43Z

Note that we're removing the package-lock.json file in #35729

apps/full-site-editing/package.json

…stead This is required because of the Calypso monorepo relying on root dependencies. We don’t want to install @wordpress/scripts in Calypso root so npx is the best interim soluton. See #35379 (comment)

getdave · 2019-08-27T13:11:18Z

When I remove @wordpress/scripts from the package.json I get the

Preset @wordpress/jest-preset-default is invalid:

@sirreal I'm going to install @wordpress/scripts in the root package.json as discussed.

getdave · 2019-08-29T15:55:09Z

I've refactored to use React Testing Library because Enzyme is being "deprecated" by Gutenberg Core

See WordPress/gutenberg#17249

…stead This is required because of the Calypso monorepo relying on root dependencies. We don’t want to install @wordpress/scripts in Calypso root so npx is the best interim soluton. See #35379 (comment)

getdave · 2019-08-30T11:46:05Z

@obenland @gwwar I think this is now ready to go. I'd love if you could give it another go and a 👍

@sirreal Can you confirm you're happy with my devDependencies. Had to add a couple more to get Jest config to work. Unavoidable I'm afraid.

sirreal · 2019-08-30T13:00:47Z

@sirreal Can you confirm you're happy with my devDependencies. Had to add a couple more to get Jest config to work. Unavoidable I'm afraid.

Given this is operating as a fairly isolated project, and I don't think there's a good way around that now, no problem from me 👍

…stead This is required because of the Calypso monorepo relying on root dependencies. We don’t want to install @wordpress/scripts in Calypso root so npx is the best interim soluton. See #35379 (comment)

…encies and causes test runner errors

…Gutenberg Core Rewrote tests using RTL and it is much nicer and avoids testing implementation details. I have much more confident in my tests.

…iner

@wordpress/jest-preset-default is required in order to extend the @wordpress/scripts jest config

Exists as a pass through component to simplying testing components which consume `BlockPreview` from `@wordpress/block-editor`. This is because jest cannot mock node modules that are not part of the root node modules. Due to the way this projects dependencies are defined `@wordpress/block-editor` does not exist within `node_modules` and it is there impossible to mock it without providing a wrapping component to act as a pass though. See https://jestjs.io/docs/en/manual-mocks

Previously adding conditionals in tests had removed the ability to provide a “blank” value in the Templates. Now add this as an explicit assertion across multiple tests to ensure it isn’t lost again.

...iting-plugin/starter-page-templates/page-template-modal/components/template-selector-item.js

...starter-page-templates/page-template-modal/components/test/template-selector-control-test.js

frontdevde

Thanks for the inline comments @getdave, that made it easier to follow what your thought process behind certain decisions was e.g. the pass-through component. Testing it locally, the tests are passing as expected. With the comments and existing discussions in mind, nothing immediately jumps out to me right now. Given that it shouldn't affect Calypso but just our plugin, I'd be OK with getting this in and continue iterating from there 👍

retrofox

The changes purposed here don’t affect the current functionality of the plugin, and the tester framework looks pretty good.
I suggest 🚢 this tool since it will helpful for our development flow, considering that we can rollback if we find out another testing approach.
Thanks for taking over of this task, Dave.

getdave · 2019-09-20T15:21:17Z

Thanks @retrofox and @frontdevde. Appreciate your feedback and 👍 on this PR. Let's hope it helps us 🚢 with greater confidence.

Whilst it is important to test for the presence of `blocks` it does not hurt to provide a default value as this ensures the “blank” state can be provided when in dynamic mode.

getdave requested a review from a team August 14, 2019 16:11

getdave requested review from a team as code owners August 14, 2019 16:11

getdave self-assigned this Aug 14, 2019

obenland added this to the Ajax: Iteration 7 milestone Aug 14, 2019

getdave changed the title ~~SPT add JS unit tests to the plugin~~ SPT add JS Functional tests to the plugin Aug 20, 2019

obenland removed this from the Ajax: Iteration 7 milestone Aug 20, 2019

getdave force-pushed the try/spt-add-unit-tests branch from a41405b to 7bea67a Compare August 21, 2019 15:46

obenland changed the title ~~SPT add JS Functional tests to the plugin~~ (?P) SPT add JS Functional tests to the plugin Aug 21, 2019

getdave changed the title ~~(?P) SPT add JS Functional tests to the plugin~~ (5P) SPT add JS Functional tests to the plugin Aug 22, 2019

getdave force-pushed the try/spt-add-unit-tests branch from 7bea67a to 26e2cf3 Compare August 22, 2019 13:55

sirreal reviewed Aug 26, 2019

View reviewed changes

apps/full-site-editing/package.json Show resolved Hide resolved

getdave force-pushed the try/spt-add-unit-tests branch from afc1745 to 9c3e5c0 Compare August 27, 2019 09:26

getdave force-pushed the try/spt-add-unit-tests branch from 2221ee1 to 01a5d35 Compare August 30, 2019 11:39

getdave requested review from sirreal and obenland August 30, 2019 11:46

getdave added 17 commits September 20, 2019 10:15

Removes redunant aria prop

ec66133

Adds tests for Dynamic Preview mode

b608dd2

Refactor to DRY up test fixture data into helper file

1277131

Update snapshot

e70f679

Move dev deps to correct location in package json

0329b7c

Adds tests for TemplateSelectorPreview component

35674b9

Add tests and improve fixture data

5feeb58

Updates to remove @wordpress/scripts dep and utilise npx for tests in…

3d8f08e

…stead This is required because of the Calypso monorepo relying on root dependencies. We don’t want to install @wordpress/scripts in Calypso root so npx is the best interim soluton. See #35379 (comment)

Restore @wordpress/scripts to devDeps as npx doesn’t cover all depend…

39ecf6c

…encies and causes test runner errors

Refactor to React Testing Library as Enzyme is not being prefered by …

c6d4019

…Gutenberg Core Rewrote tests using RTL and it is much nicer and avoids testing implementation details. I have much more confident in my tests.

Rewrite Template Preview tests using RTL

f08e4b7

Update to local usage of queryByText to avoid unecessary ref to conta…

7bd37fd

…iner

Remove unecessary test

44174a7

Update to use custom Jest config to enable one line test setup imports

ce5af92

Update to remove need to manually import test setup

7a5d8b4

Include only necessary deps

71ad1bc

@wordpress/jest-preset-default is required in order to extend the @wordpress/scripts jest config

Resolve errors introduced due to rebase

6589029

getdave force-pushed the try/spt-add-unit-tests branch from 01a5d35 to 6589029 Compare September 20, 2019 09:18

getdave added 2 commits September 20, 2019 11:03

Fix to ensure Blank mode is preserved

77b2386

Previously adding conditionals in tests had removed the ability to provide a “blank” value in the Templates. Now add this as an explicit assertion across multiple tests to ensure it isn’t lost again.

retrofox reviewed Sep 20, 2019

View reviewed changes

...iting-plugin/starter-page-templates/page-template-modal/components/template-selector-item.js Show resolved Hide resolved

frontdevde reviewed Sep 20, 2019

View reviewed changes

...starter-page-templates/page-template-modal/components/test/template-selector-control-test.js Show resolved Hide resolved

frontdevde approved these changes Sep 20, 2019

View reviewed changes

retrofox approved these changes Sep 20, 2019

View reviewed changes

Reinstate missing default value

ad7b6ae

Whilst it is important to test for the presence of `blocks` it does not hurt to provide a default value as this ensures the “blank” state can be provided when in dynamic mode.

getdave merged commit 61d4c56 into master Sep 20, 2019

getdave deleted the try/spt-add-unit-tests branch September 20, 2019 15:43

obenland added this to the Ajax: Iteration 9 milestone Sep 23, 2019

tn3rb mentioned this pull request Dec 20, 2019

Bump @wordpress/jest-preset-default from 5.1.1 to 5.3.0 eventespresso/event-espresso-core#2072

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(5P) SPT add JS Functional tests to the plugin #35379

(5P) SPT add JS Functional tests to the plugin #35379

getdave commented Aug 14, 2019 •

edited

Loading

matticbot commented Aug 14, 2019

obenland commented Aug 14, 2019

getdave commented Aug 14, 2019

matticbot commented Aug 14, 2019 •

edited

Loading

gwwar commented Aug 14, 2019

getdave commented Aug 21, 2019 •

edited

Loading

getdave commented Aug 23, 2019

mmtr commented Aug 24, 2019

getdave commented Aug 27, 2019 •

edited

Loading

getdave commented Aug 29, 2019

getdave commented Aug 30, 2019

sirreal commented Aug 30, 2019

frontdevde left a comment

retrofox left a comment

getdave commented Sep 20, 2019

(5P) SPT add JS Functional tests to the plugin #35379

(5P) SPT add JS Functional tests to the plugin #35379

Conversation

getdave commented Aug 14, 2019 • edited Loading

Please note

Testing instructions

matticbot commented Aug 14, 2019

obenland commented Aug 14, 2019

getdave commented Aug 14, 2019

matticbot commented Aug 14, 2019 • edited Loading

gwwar commented Aug 14, 2019

getdave commented Aug 21, 2019 • edited Loading

getdave commented Aug 23, 2019

mmtr commented Aug 24, 2019

getdave commented Aug 27, 2019 • edited Loading

getdave commented Aug 29, 2019

getdave commented Aug 30, 2019

sirreal commented Aug 30, 2019

frontdevde left a comment

Choose a reason for hiding this comment

retrofox left a comment

Choose a reason for hiding this comment

getdave commented Sep 20, 2019

getdave commented Aug 14, 2019 •

edited

Loading

matticbot commented Aug 14, 2019 •

edited

Loading

getdave commented Aug 21, 2019 •

edited

Loading

getdave commented Aug 27, 2019 •

edited

Loading