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

Discussion: Testing #35

Closed
2 tasks
kenwheeler opened this issue Jan 19, 2017 · 12 comments
Closed
2 tasks

Discussion: Testing #35

kenwheeler opened this issue Jan 19, 2017 · 12 comments

Comments

@kenwheeler
Copy link

Lets use this ticket to discuss:

  • Which test lib to use
  • Spec naming/location convention
@evenstensberg
Copy link
Member

I'd find it appropriate to be under the specific files they originate from.

lib/prompt/prompt.js - together with lib/prompt/prompt.spec|test.js

@DanielaValero
Copy link
Contributor

DanielaValero commented Jan 23, 2017

About the lib:

For what we spoke in the last meeting, my impression was that most of us had more experience with Mocha, plus that webpack is also using it. Both might be a big plus when it comes to deciding for a testing tool.

In general I'd tend to say that we should better go for Mocha, as would be easier for most of us to write the tests, and also would be better when it comes to getting some support from devs from webpack who have done already contributions for testing in webpack such as alistairjcbrown, mentioned by @TheLarkInn

We mentioned that Jest can be faster than Mocha, and also the Mocha can be easier to debug with node.

About the location and naming convention:

I'd say that we stick to what the convention is for the testing tool that we decide.

@pksjce you had some good points about jest, would you share them?

@alistairjcbrown
Copy link

About the lib:

I’ve only really used Jest for react testing and that does seem to be a focus of their docs - it’s probably more than this, but to me it’s essentially Jasmine with react testing (in memory DOM, tree snapshotting) built in. So I'd s/jest/jasmine/g for this discussion.

For Jasmine vs Mocha, Mocha’s just the test framework - you bring in your assertion library, spys, test sandboxing, etc. on top Jasmine has a bit more built in - comes with built in expect, basic spys, test contexts are automatically sandboxed (so you can happily set values on this knowing they won’t leak) (edited)

If the aim is to standardise approaches across a number of repos, having a single dependency of Jasmine might be nicer than co-ordinating mocha + chai + sinon. However, as mentioned above, mocha + chai + sinon is used in the webpack main repo, so alignment with that might be a benefit especially if, in the future, functionality were split out into new plugins.

I'm not overly opinionated either way, and if most of the devs familiar with the code have used mocha, then that makes sense as the setup to use.

About the location and naming convention:

I'm a big fan of pod structures, keeping a modules tests (and related code) in the one directory. Most testing frameworks are pretty unopinionated about where the test files are kept and, as mentioned above, if the tests are suffixed .spec.js or .test.js (which I'm +1 for too), then globbing those files becomes very easy.
The advantages are code that's more modular - everything is encapsulated in one directory and all internal paths are relative, eg. your tests only have to reach up a directory rather than a full path from root down.

You'd end up with something like:

my-module
├── helper.js
├── index.js
└── test
    ├── helper.spec.js
    └── index.spec.js

So for an example in this repo:

parser
├── index.js
└── test
    └── index.spec.js
└── resolve-packages
    ├── index.js
    └── test
        └── index.spec.js
     ...

@okonet
Copy link
Contributor

okonet commented Jan 28, 2017

Jest is much more than Jasmine. Actually they completely removed the Jasmine dependency recently so right now it's basically the same as mocha + built-in assertions. You still can use other assertion libraries like chai, but would you when you have a really good built-ins? Also Jest will switch to expect assertions soon.

The pros of Jest is no-config setup, coverage out of the box, speed, support for snapshots (I think this latter is very powerful feature that I'd like to use for jscodeshift tests for instance).

I've moved almost all my repos to Jest because of nice UX without any issues. I've used Jest with Sinon and chai, but the best experience is with the built-in assertions.

I'm +1 for putting test files along with source files. No need for that extra directory. It only complicates the structure without any obvious benefit.

To me Jest best benefit is no need to agree on something since it already has all sane defaults built in. I can imagine every developer has its own opinion on dir structure and naming, assertion lib etc. but in the end it all doesn't matter. What matters is how fast we can write and run tests. And Jest helps here a lot with snapshots and coverage reports.

@CrlsMrls
Copy link

I'm also +1 for putting test files along with source files. No need for that extra directory.

Following the same example as @alistairjcbrown suggested, directories would look like:

parser
├── index.js
├── index.spec.js
└── resolve-packages
    ├── index.js
    └── index.spec.test
     ...

@okonet
Copy link
Contributor

okonet commented Feb 1, 2017

One more argument in favor of Jest: after a discussion about how to approach testing for transformations with @pksjce we'd like to use snapshotting to generate output fixtures automatically since current approach (manually update output files) is error-prone and time-consuming.

@evenstensberg
Copy link
Member

@okonet +1, was thinking about using it for the same

@DanielaValero
Copy link
Contributor

I find quite valid the argumentation towards Jest. So far, the discussion is pointing towards the next decisions:

  • Keep Jest
  • Place the tests next to the source code.

What I still find open would be:

  • Directory name for tests: So far we've got: __test__ and __fixtures__
  • Sufix for files: .test.js or .spec.js. In the project we've got already .test.js

@evenstensberg
Copy link
Member

I'm in favor for spec and also removing directories with test, replacing them with inline-files. Easy to do with jest configuration in package.json as well as perhaps an own mock directory somewhere.

@okonet
Copy link
Contributor

okonet commented Feb 2, 2017

Easy to do with jest configuration in package.json

I think it will work without the config for this case as well.

@DanielaValero
Copy link
Contributor

Cool, so to summarize, our decisions are:

I find quite valid the argumentation towards Jest. So far, the discussion is pointing towards the next decisions:

  • Keep Jest
  • Place the tests next to the source code, without any sub folder for test.
  • We just use .spec.js

When everyone is ok with this, by doing a +1 reaction in my comment, then we can close this discussion as completed.

@evenstensberg
Copy link
Member

Closing, resolved.

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

No branches or pull requests

6 participants