Skip to content

Document the new test setup with Jest #2451

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
May 26, 2017
Merged

Document the new test setup with Jest #2451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
63a92f1
Document the new test setup with Jest
kumar303 May 25, 2017
76e1a46
typo
kumar303 May 25, 2017
a113fb1
Added watchman caveat
kumar303 May 25, 2017
b560415
Document test matching a little bit
kumar303 May 25, 2017
ad0145c
use a more future proof example
kumar303 May 25, 2017
0cf333f
revisions from feedback
kumar303 May 26, 2017
d97574f
Merge branch 'master' into jest-docs-iss2450
kumar303 May 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 78 additions & 60 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,68 +23,78 @@ Never submit security-related bugs through a Github Issue or by email.

## Requirements

* Node 6.x LTS
* You need [Node](https://nodejs.org/) 6.x which is the current
[LTS](https://github.com/nodejs/LTS) (long term support) release.
* Install [yarn](https://yarnpkg.com/en/) to manage dependencies
and run scripts.

The easiest way to manage multiple node versions in development is to use
[nvm](https://github.com/creationix/nvm).

## Get started

* npm install
* npm run dev

## NPM scripts for development

Generic scripts that don't need env vars. Use these for development:

| Script | Description |
|-------------------------|-------------------------------------------------------|
| npm run dev:admin | Starts the dev server (admin app) |
| npm run dev:amo | Starts the dev server and proxy (amo) |
| npm run dev:amo:no-proxy| Starts the dev server without proxy (amo) |
| npm run dev:disco | Starts the dev server (discovery pane) |
| npm run flow:check | Check for Flow errors and exit |
| npm run flow:dev | Continuously check for Flow errors |
| npm run eslint | Lints the JS |
| npm run stylelint | Lints the SCSS |
| npm run lint | Runs all the JS + SCSS linters |
| npm run version-check | Checks you have the minimum node + npm versions |
| npm test | Runs the unittest, servertests + lint |
| npm run unittest | Runs just the unittests |
| npm run unittest:dev | Runs the unittests and watches for changes |
| npm run unittest:server | Starts a unittest server for use with `unittest:run` |
| npm run unittest:run | Executes unittests (requires `unittest:server`) |
| npm run servertest | Runs the servertests |
* type `yarn` to install all dependencies
* type `yarn dev:amo` to start a dev server
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if yarn dev should be the shortcut to start the AMO dev server given it's the main app here and what I'd venture most devs spend most of their time on.

Sort of unrelated to this change, I know, but just thinking aloud.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's an interesting idea. I also wondered about us considering making the addons-frontend repo a single app and binning the separation between the disco pane and amo once admin is gone. It would remove a number of complications though we would need to retain them as separate entry points from a webpack perspective. That's a discussion for another time though.


## Development commands

Here are some commands you can run:

| Command | Description |
|----------------------|---------------------------------------------------------------------------------|
| yarn dev:admin | Starts the dev server (admin app) |
| yarn dev:amo | Starts the dev server and proxy (amo) |
| yarn dev:amo:no-proxy| Starts the dev server without proxy (amo) |
| yarn dev:disco | Starts the dev server (discovery pane) |
| yarn flow:check | Check for Flow errors and exit |
| yarn flow:dev | Continuously check for Flow errors |
| yarn eslint | Lints the JS |
| yarn stylelint | Lints the SCSS |
| yarn lint | Runs all the JS + SCSS linters |
| yarn version-check | Checks you have the required dependencies |
| yarn test | Enters [jest][] in watch mode |
| yarn test-ci | Runs all continuous integration checks. This is only meant to run on TravisCI. |
| yarn test-coverage | Runs all tests and reports code coverage |
| yarn test-once | Runs all tests with [jest][] and exits |

### Running tests

You can run the entire test suite with `npm test` but there are a few other ways
to run tests.
You can enter the interactive [jest][] mode by typing `yarn test`.
This is the easiest way to develop new features.

#### Run all unit tests in a loop
Here are a few tips:

You can use `npm run unittest:dev` to run all unit tests in a loop while you
edit the source code.
* When you start `yarn test`, you can switch to your code editor and begin
adding test files or changing existing code. As you save each file, [jest][]
will only run tests related to the code you change.
* If you had typed `a` when you first started then [jest][] will continue to
run the full suite even when you change specific files. Type `o` to switch
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to launch in o mode by default? That seems to be the best mode for development and it might be nice if yarn test or even yarn test:dev started in that mode without the interactive prompt.

Copy link
Contributor

Choose a reason for hiding this comment

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

It does sort of do that if you run watch without pressing anything, since it seems to look at what changed since the last commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a way to launch in o mode by default?

It does launch in this mode by default :) However, if you press the a key (which intuitively is what I thought to do) then it will switch you out of the o mode so just don't do that.

back to the mode of only running tests related to the files you are changing.
* If you see something like `Error watching file for changes: EMFILE` on Mac OS
then `brew install watchman` might fix it.
See https://github.com/facebook/jest/issues/1767

#### Run a subset of the unit tests
#### Run a subset of the tests

If you don't want to run the entire unit test suite, first you have to start a
unittest server:
By default, `yarn test` will only run a subset of tests that relate to the code
Copy link
Contributor

Choose a reason for hiding this comment

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

But this isn't really true, right? By default yarn test opens the prompt and doesn't really have a default.

Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned, that depends. It seems to default to running anything it finds since the last commit. But if there is nothing it just sits there waiting for a prompt.

Copy link
Contributor

Choose a reason for hiding this comment

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

Neat. I didn't realise that. Might be worth quickly pointing that out in the docs then, but that sounds good.

you are working on.

npm run unittest:server
To explicitly run a subset of tests, you can type `t` or `p` which are explained
in the [jest][] watch usage.

When you see "Connected on socket," the server has fully started.

Now you can execute a more specific [mocha](https://mochajs.org/) command,
such as using `--grep` to run only a few tests. Here is an example:

npm run unittest:run -- --grep=InfoDialog
Alternatively, you can start the test runner with a
[specific file or regular expression](https://facebook.github.io/jest/docs/en/cli.html#jest-regexfortestfiles),
like:
````
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought these were usually three tildes for a block code section, not four.

yarn test tests/client/amo/components/TestAddonDetail.js
````

This would run all tests that either fall under the `InfoDialog` description grouping
or have `InfoDialog` in their behavior text.
#### Run all tests

Any option after the double dash (`--`) gets sent to `mocha`. Check out
[mocha's usage](https://mochajs.org/#usage) for ideas.
If you want to run all tests and exit, type:
````
yarn test-once
````

### Flow

Expand All @@ -93,7 +103,7 @@ to check for problems in the source code.

To check for Flow issues during development while you edit files, run:

npm run flow:dev
yarn flow:dev

If you are new to working with Flow, here are some tips:
* Check out the [getting started](https://flow.org/en/docs/getting-started/) guide.
Expand Down Expand Up @@ -152,12 +162,18 @@ function getAllAddons({ categoryId }: GetAllAddonsParams = {}) {

### Code coverage

The `npm run unittest` command generates a report of how well the unit tests
covered each line of source code.
The continuous integration process will give you a link to view the report.
To see this report while running tests locally, type:
To see a report of code coverage, type:
````
yarn test-coverage
````

This will print a table of files showing the percentage of code coverage.
The uncovered lines will be shown in the right column but you can open
the full report in a browser:

open ./coverage/index.html
````
open coverage/lcov-report/index.html
````

### Running AMO for local development

Expand All @@ -174,8 +190,8 @@ it will not work when logging in from an addons-server page. See
information on fixing this.

If you would like to use `https://addons-dev.allizom.org` for the API you should use the
`npm run dev:amo:no-proxy` command with an `API_HOST` to start the server without the proxy. For
example: `API_HOST=https://addons-dev.allizom.org npm run dev:amo:no-proxy`.
`yarn dev:amo:no-proxy` command with an `API_HOST` to start the server without the proxy. For
example: `API_HOST=https://addons-dev.allizom.org yarn dev:amo:no-proxy`.

### Configuring for local development

Expand All @@ -199,10 +215,10 @@ module.exports = {
};
````

When you start up your front-end discover pane server, it will now apply
When you start up your front-end Discovery Pane server, it will now apply
overrides from your local configuration file:

npm run dev:disco
yarn dev:disco

Consult the
[config file loading order docs](https://github.com/lorenwest/node-config/wiki/Configuration-Files#file-load-order)
Expand Down Expand Up @@ -233,14 +249,14 @@ The env vars are:

| Script | Description |
|------------------------|-----------------------------------------------------|
| npm run start | Starts the express server (requires env vars) |
| npm run build | Builds the libs (all apps) (requires env vars) |
| yarn start | Starts the express server (requires env vars) |
| yarn build | Builds the libs (all apps) (requires env vars) |

Example: Building and running a production instance of the AMO app:

````
NODE_APP_INSTANCE=amo NODE_ENV=production npm run build
NODE_APP_INSTANCE=amo NODE_ENV=production npm run start
NODE_APP_INSTANCE=amo NODE_ENV=production yarn build
NODE_APP_INSTANCE=amo NODE_ENV=production yarn start
````

To run the app locally in production mode you'll need to create a config file
Expand Down Expand Up @@ -296,7 +312,7 @@ module.exports = {
};
````

After this, re-build and restart using `npm run build` and `npm run start`
After this, re-build and restart using `yarn build` and `yarn start`
as documented above.
If you have used `localhost` before with a different configuration,
be sure to clear your cookies.
Expand Down Expand Up @@ -350,3 +366,5 @@ still can.
* Code written in ES2015+
* Universal rendering via node
* Unit tests with high coverage (aiming for 100%)

[jest]: https://facebook.github.io/jest/docs/en/getting-started.html
2 changes: 1 addition & 1 deletion bin/extract-locales
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
#!/usr/bin/env sh

npm run extract-locales
yarn extract-locales
2 changes: 1 addition & 1 deletion docs/adding-a-page.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export default (

### Starting the server

Now that the component is setup we can run `npm start dev:search` and navigate to
Now that the component is setup we can run `yarn dev:search` and navigate to
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically this page is very out of date since it still references the search app which is now admin (which we're likely deleting). But c'est la vie!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I would remove any references to search (and admin) since it won't work the app is called admin, and the app is very close to being deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This whole guide is based around the search app. I'm not going to spend time on updating it for this patch. TBH I was just replacing all instances of npm run.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I just sort of was pointing it out to keep in mind... no need to update it right now, there are other issues for that.

Sorry for the noise!

Copy link
Contributor

Choose a reason for hiding this comment

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

Fwiw yarn dev:search won't even run though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/mozilla/addons-frontend/issues/2223 would be a good place to fix up this guide

[http://localhost:3000/user](http://localhost:3000/user) to see the page. Since our component is
wrapped in the `LoginRequired` component you will need to be logged in to see the page.

Expand Down
2 changes: 1 addition & 1 deletion docs/i18n.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ carried out via [Pontoon](https://pontoon.mozilla.org/).
Some commands wrap standard gettext tools. To run these commands you'll need
to ensure you have done the following steps:

* Run `npm install` to install all the project deps.
* Run `yarn` to install all the project deps.
* Install [gettext](https://www.gnu.org/software/gettext/) tools for your
platform.

Expand Down