Document the new test setup with Jest #2451

kumar303 · 2017-05-25T20:04:03Z

tofumatt

Looks good. r+wc

I have some thoughts about yarn dev and questions about the "defaults" for the tests, but everything looks good and I like the usage of yarn everywhere!

Maybe see about making jest test o the actual default and addressing calling it the default in the docs? As I don't think it truly is.

tofumatt · 2017-05-25T21:55:33Z

README.md

-| 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


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.

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.

tofumatt · 2017-05-25T21:57:45Z

README.md

+  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


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.

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.

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.

tofumatt · 2017-05-25T21:58:48Z

README.md


-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


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

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.

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

tofumatt · 2017-05-25T21:59:38Z

README.md

+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:
+````


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

tofumatt · 2017-05-25T22:00:40Z

README.md

+The uncovered lines will be shown in the right column but you can open
+the full report in a browser:
+
+    open coverage/lcov-report/index.html


This only works on Mac OS, for what it's worth. Also might be nice to be consistent with block code style as the other ones use the tilde.

FWIW looks like a "open coverage" key option will be available directly from jest in the not too distant.

tofumatt · 2017-05-25T22:01:24Z

README.md

 };
 ````

 When you start up your front-end discover pane server, it will now apply


This should be "front-end Discovery Pane" instead of "discover pane".

tofumatt · 2017-05-25T22:03:48Z

README.md

 * Universal rendering via node
 * Unit tests with high coverage (aiming for 100%)
+
+[jest]: http://facebook.github.io/jest/docs/en/getting-started.html


Should be HTTPS.

Seems to be only available under http

Huh, that's weird, GitHub pages I thought had HTTPS 😞

Huh, that's weird, GitHub pages I thought had HTTPS 😞

Ah no you were right! For some reason I tried https://facebook.github.io/jest/docs/en/getting-started.html and got the http version, maybe that was the awesome bar correcting me if I'd been linked to that in the past? ¯\_(ツ)_/¯

An https link works for me. Sure, I can change it.

tofumatt · 2017-05-25T22:05:11Z

docs/adding-a-page.md

 ### 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


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!

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.

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.

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!

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

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

muffinresearch · 2017-05-26T08:20:16Z

README.md

-* 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.
+* To make life easier, also install [yarn](https://yarnpkg.com/en/).


Since we're pushing yarn as the command runner and dep installer it would make sense to make it sound a bit less optional (or alternatively clarify how to run commands with npm run in its place)

tofumatt · 2017-05-26T14:56:59Z

ahhhh, cool :-)

tofumatt · 2017-05-26T15:01:10Z

Oh, yeah, it’ll do that.

Document the new test setup with Jest

63a92f1

kumar303 added the state:pull_request_ready label May 25, 2017

kumar303 added 4 commits May 25, 2017 15:07

typo

76e1a46

Added watchman caveat

a113fb1

Document test matching a little bit

b560415

use a more future proof example

ad0145c

kumar303 requested review from tofumatt and muffinresearch May 25, 2017 20:24

tofumatt approved these changes May 25, 2017

View reviewed changes

muffinresearch reviewed May 26, 2017

View reviewed changes

kumar303 added 2 commits May 26, 2017 10:06

revisions from feedback

0cf333f

Merge branch 'master' into jest-docs-iss2450

d97574f

kumar303 merged commit d7563f0 into mozilla:master May 26, 2017

kumar303 deleted the jest-docs-iss2450 branch May 26, 2017 20:45

kumar303 removed the state:pull_request_ready label May 26, 2017

Document the new test setup with Jest #2451

Document the new test setup with Jest #2451

Uh oh!

Conversation

kumar303 commented May 25, 2017

Uh oh!

tofumatt left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

muffinresearch May 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tofumatt commented May 26, 2017 via email

Uh oh!

tofumatt commented May 26, 2017 via email

Uh oh!

Uh oh!

tofumatt left a comment •

edited

Loading

muffinresearch May 26, 2017 •

edited

Loading