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

Try: Add very basic unit tests to validate it works with the repository at the global level #84

Merged
merged 1 commit into from
Aug 29, 2019

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Aug 13, 2019

Initial try to address #26.

This adds very basic unit tests on the repository level. In the long term, we should rather have unit tests for blocks which use ESNext. I think @Shelob9 has some good ideas about what to test which I'm going to explore later. As far as I remember, it's relatively easy to cover save implementation with snapshot tests. In case of edit implementation, it seems much easier to run e2e tests once we figure out how to set up a local instance of WordPress with the latest Gutenberg and this plugin installed and working with Travis.

Testing

npm test

local:gutenberg-examples gziolo$ npm run test

> [email protected] test /Users/gziolo/PhpstormProjects/gutenberg-examples
> wp-scripts test-unit-js

 PASS  test/examples.js
  ✓ adds 1 + 2 to equal 3 (2ms)
  ✓ Button renders (2ms)

 › 1 snapshot written.
Snapshot Summary
 › 1 snapshot written from 1 test suite.

Test Suites: 1 passed, 1 total
Tests:       2 passed, 2 total
Snapshots:   1 written, 1 total
Time:        1.58s
Ran all test suites.

Known Issues

When running npm run lint-js I see the following errors:

/Users/gziolo/PhpstormProjects/gutenberg-examples/test/examples.js
  10:1  error  'test' is not defined    no-undef
  11:2  error  'expect' is not defined  no-undef
  14:1  error  'test' is not defined    no-undef
  15:2  error  'expect' is not defined  no-undef

✖ 4 problems (4 errors, 0 warnings)

To solve it I had to add:

+ /* global test, expect */

at the top of the file which should work out of the box. This should work out of the box when using @wordpress/scripts.

I also had to put:

+ /**
+  * WordPress dependencies
+  */
import { Button } from '@wordpress/components';

above the import statement to fix the following issue:

 1:1  error  Expected preceding "WordPress dependencies" comment block  @wordpress/dependency-group

which I don't think we want to enforce outside of Gutenberg.

/cc @iandunn

@gziolo gziolo force-pushed the try/unit-tests-integration branch 2 times, most recently from d8620eb to cdb737e Compare August 13, 2019 08:01
@gziolo gziolo force-pushed the try/unit-tests-integration branch from cdb737e to 716b945 Compare August 13, 2019 08:02
/**
* External dependencies
*/
import { create as createTestRenderer } from 'react-test-renderer';
Copy link
Member Author

@gziolo gziolo Aug 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like react-test-renderer is a very deep dependency of @wordpress/scripts. We should ensure it has tighter integration so it works out of the box in all cases.

/**
* WordPress dependencies
*/
import { Button } from '@wordpress/components';
Copy link
Member Author

@gziolo gziolo Aug 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that having @wordpress/components as a dependency brings @wordpress/element as well. Ideally, we bring it as part of @wordpress/babel-preset-default which is the default handler for React regardless.

@gziolo
Copy link
Member Author

gziolo commented Aug 13, 2019

Overall, I think we could merge this PR as is to give some idea about how to integrate unit tests in the repository. We can refine it later when all issues are resolved in Gutenberg: WordPress/gutenberg#17016.

@gziolo gziolo requested a review from a team August 13, 2019 08:28
@iandunn
Copy link
Member

iandunn commented Aug 13, 2019

❤️

Thanks! It looks good to me. I agree that this will be very helpful as a starting point, and can be iterated on to streamline it further.

Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good 👍, though I think it demonstrates how it's a little awkward that we have one package.json to service every example. I'd say it might be worth considering restructuring this project so that each example is its own plugin with its own package.json. That way, developers can copy and paste an example in its entirety to their project.

@gziolo
Copy link
Member Author

gziolo commented Aug 29, 2019

@noisysocks, totally agree. I came to the conclusion that it’s better to have any example than none of them 😎 This is also important to have this integration to have a way to ensure that wp-scripts works seamlessly. As it turned out it wasn’t so pleasant experience but it should get fixed soon.

Another option is to use Lerna and run tests from each folder of blocks which have test command configured.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants