WIP: Test tweaks

[nickserv]

Mostly just adjusting syntax and tooling based on some ideas I had in #52 and #53.

• Replace callback tests with Promises
• Move test files to the same directories with.test.js extensions
• Eslint config in package.json only
• Simplify Coveralls usage
• Review package.json
• Change validate to test so Travis runs the same command as local development
• Move test files to a __tests__ directory per source directory instead
• One test file per source file
• Unit test for listen (it's no longer used in the other tests)

[nickserv]

@nfantone Would you agree with having one test file per file in lib with a .test.js extension in the same directory? This is what Jest generally recommends and I like the approach, though since there's so much functionality in index.js we can keep the middleware tests in separate files if you want.

[nfantone]

@nickmccurdy I'd say normally, in backend projects, tests are kept separate in their own directory. We could, however, mirror the lib/ folder structure under test/.

[nickserv]

I know that's typical, Jest supports both conventions. The nice thing about *.test.js is your tests are first class citizens right next to your source files, it's easy to see if tests are missing, what you have tested, and it's easier to do relative imports when they're in the same directory. Though if you think them being in the same directory is too messy we can have a __tests__ directory in each source directory with a mirrored file structure. Thoughts?

[nfantone]

My vote is on keeping them under test/, frankly. In the long run, this seems like the most sane and maintainable approach, from my experience. New front-end projects tend to follow a side-by-side structure because testing components is usually mow straight forward, more often than not doesn't require external resources and it serves as a sort of documentation for its usage.

The again, I'm not completely against to the idea of having sources and tests together if that that we decide to follow.

[nickserv]

Besides *.test.js, Jest also supports __tests__ directories. There are two options: one monolithic directory in the root of the project (what we're doing) and one __tests__ directory inside each source directory. The reason for the latter is that you don't have to duplicate your top level directory structure and you get simpler imports, similar to the advantages of *.tests.js but you don't pollute the source directory with test files directory. How do you feel about this? I think it's the best of both worlds personally.

[nfantone]

@nickmccurdy On a related note, I'm also not too fond of moving configuration to package.json (not only eslint, but any kind).

• package.json is a common source of conflicts. Limiting what we include, reduces those.
• Moving non-npm specific configuration to it, makes it less portable.
• It leaks your configuration while installing the package to other packages.
• You lose the cascading effect of having nested configuration (like one specific to test/).

But again, this is just my opinion 🐐

[nickserv]

I don't think conflicts would be an issue since eslint has its own section, the conflicts would likely be above the eslint config and still happen either way. I believe npm supports adding arbitrary fields with no issues, either way because the dependencies that use those fields are installed with the same versions in the package lock, it should be fine. Also it seems like more an more packages are moving config to package.json, for example it's the only place (besides the CLI) that Jest supports configuration. Also eslint still cascades from package.json, you can continue to have other .eslintrc files in subdirectories that override it.

I'm still open to suggestions about everything, I'm mostly experimenting with stuff. Also I'll try to make small commits so if we disagree on something later we can revert it

[doug-wade]

Does this still log errors without the done(err)? Or are they swallowed?

[nickserv]

Yes, because if supertest throws an error the promise will reject, and since it's returned to Jest it will be automatically handled and the test will error. One of the nice things about promises is you only have to set up a handler once per chain, and for things like Jest it's handled automatically.

[doug-wade]

ooo this was a tricky bit of hackery. Glad we don't need it anymore.

[nickserv]

Whoops looks like I broke coverage on the listen function, I'll write a small test for it and wrap this up reorganizing the test files soon.

[nickserv]

I fixed the directory structure, I'll ask for another review after I add the missing app.listen() test.