Skip to content
This repository has been archived by the owner on Jul 9, 2018. It is now read-only.

Jest preset: Add default jest preset for WordPress development #74

Merged
merged 2 commits into from
Feb 1, 2018

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Jan 27, 2018

This PR introduces Jest preset to be shared between @wordpress/packages and Gutenberg. In the future, it can be also consumed by the projects created by plugin or theme developers. This implementation is fully based on the existing setup of Gutenberg which can be found in the jest section of the package.json file: https://github.com/WordPress/gutenberg/blob/master/package.json#L105. I moved everything that isn't very specific to Gutenberg's configuration. We might further discuss some things included in this PR, but I expect that most of the things included here should be left as they are. Maybe jQuery stub is something we don't want to keep. Not sure really :)

There aren't many Jest preset published to npm so it doesn't seem to be a popular technique, but in our case, it should remove some code duplication and provide reasonable default settings.

The last step is to add changes which will detect if the project consuming scripts contains Babel or Jest config and use our presets as fallbacks. That should conclude all the work required to make unit testing easy to start within all WordPress projects.

I'm not quite sure if we should include docs about the recommended Jest setup here, inside scripts package or in Gutenberg's handbook? I guess it can be a follow-up task once we have all the pieces working together.

Testing

It's pretty annoying that I couldn't start this PR in a shape that allows to work seamlessly after it's going to be published. The thing is that all the files referenced inside the preset file need to use paths relative to the root folder of the project that is including the preset :( I will comment inline to highlight what needs to be changed before publishing to npm. What we have now allows to validate it as it is now because it works properly with all test commands:

  • npm test
  • npm run test:coverage
  • npm run test:coverage-ci - which Travis uses
  • npm run test:watch

package.json Outdated
],
"coverageDirectory": "coverage",
"setupTestFrameworkScriptFile": "./packages/jest-console/build/index.js"
"preset": "./packages/jest-preset-default/jest-preset.json"
Copy link
Member Author

Choose a reason for hiding this comment

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

We will have to include @wordpress/jest-preset-default in devDependencies and replace this line with:

"preset": "@wordpress/jest-preset-default"

"<rootDir>/.*/build.*"
],
"moduleNameMapper": {
"\\.(scss|css)$": "<rootDir>/packages/jest-preset-default/scripts/style-mock.js"
Copy link
Member Author

Choose a reason for hiding this comment

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

All occurences of <rootDir>/packages/jest-preset-default will have to be replaced with:
<rootDir>/node_modules/@wordpress/jest-preset-default

@gziolo gziolo force-pushed the add/jest-preset branch 3 times, most recently from 2fc3d1b to 6bcb0f3 Compare January 27, 2018 18:24
@gziolo
Copy link
Member Author

gziolo commented Jan 27, 2018

I'm not quite sure if we should include docs about the recommended Jest setup here, inside scripts package or in Gutenberg's handbook? I guess it can be a follow-up task once we have all the pieces working together.

I included detailed documentation about all config options used in this preset. I think it is enough to proceed.

@ntwb
Copy link
Member

ntwb commented Jan 30, 2018

Why name it jest-config-default? I think we should name it jest-config and the current default configuration be the default, this would allow us in the future to include alternate configurations without having to infer that it is a "default" configuration.

For example, we could add a configuration for "themes" and "plugins" and these would be jest-config/themes and jest-config/plugins` respectively.

I also think this change should also be implemented for babel-preset-default would become, babel-preset and alternate configurations would be for example babel-preset/themes and babel-preset/plugins

Likewise with broswerslist-config, alternate configurations would be for example broswerslist-config/themes and broswerslist-config/plugins

The -default suffix is superfluous in my opinion, what do you think @gziolo?

@gziolo
Copy link
Member Author

gziolo commented Jan 31, 2018

Why name it jest-config-default? I think we should name it jest-config and the current default configuration be the default, this would allow us in the future to include alternate configurations without having to infer that it is a "default" configuration.

I wanted to follow what I did for Babel... I'm fine with removing -default part here. I will do it tomorrow.

For example, we could add a configuration for "themes" and "plugins" and these would be jest-config/themes and jest-config/plugins` respectively.

I wouldn't worry as much about this at the moment :)

I also think this change should also be implemented for babel-preset-default would become, babel-preset and alternate configurations would be for example babel-preset/themes and babel-preset/plugins

I wanted to use shorthand for Babel as described here, but we can also remove -default for consistency. Will you open PR and publish an updated package to npm?

Copy link
Member

@ntwb ntwb left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@gziolo gziolo changed the title Jest preset: Add default jest preset for WordPress developmnent Jest preset: Add default jest preset for WordPress development Feb 1, 2018
@gziolo gziolo changed the base branch from master to eslint February 1, 2018 20:16
@gziolo gziolo changed the base branch from eslint to master February 1, 2018 20:16
@gziolo gziolo merged commit 4466f21 into master Feb 1, 2018
@gziolo gziolo deleted the add/jest-preset branch February 1, 2018 20:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants