react-webpack-generators / react-webpack-template

Simple react webpack template
MIT License
82 stars 44 forks source link

V2 -- Split test directory in unit and functional tests #45

Open sthzg opened 8 years ago

sthzg commented 8 years ago

What is your opinion on splitting the root test directory into

test/
  func/
    ... everything that is currently under test/
  unit/
    ... root directory for unit tests run by mocha

Currently I've added a mocha run script to package.json.

"mocha": "./node_modules/mocha/bin/mocha --require babel-core/register"

and run some unit tests by:

npm run mocha -- test/fixtures/qaFixturesTest.js --watch

That's a quite manual way and it would probably be nicer api for our users if there were run scripts like npm run test:unit, npm run test:func. npm run test would run both of them.

weblogixx commented 8 years ago

I am sorry that I do not completely understand the reason for this. If you have unit tests (e.g. for utility methods...), why dont we just add them into the current test structure? In this way you would in fact use them as more functional tests, that is true. But things like coverage information etc. are a lot harder to do if you have two test runners. Are there any benefits that I currently do not see?

I had the whole test setup working without karma, too. But it just felt incorrect as you have to monkey-patch webpacks require calls. You may see the last approaches of this when checking out https://github.com/react-webpack-generators/react-webpack-template/commit/bff1648815210.

Maybe this is something that we should move for instead of karma? All tests will run in a speed not compareable to the webpack based tests currently used...

sthzg commented 8 years ago

Generating test reports with multiple test runners is a fair point, haven't thought about that. Shipping a ready-to-go Karma integration seems very valuable to me, as in combination with Enzyme it can be used for advanced tests not possible otherwise.

A bulk of simpler tests is fine (and much faster) with Mocha and outside of Webpack (utility logic, business logic, etc.). Also if I am not mistaken FB will publish a new test helper that can do simple assertions on rendered components based purely on the node runtime, so asserting some classes and the like could become cheaper outside of Karma.

Most of all I am -1 for dropping Karma, but my initial pros for a separate unit test runner were

Maybe it is an option to let the Karma test runner process all tests, but have a purely node based test runner configured to run a subset of them (is this what you meant with adding them in the current test structure?). At least this matches my workflow in most projects, where I have unit tests evaluating all the time, and the more expensive functional tests only on commit.

weblogixx commented 8 years ago

You may even run the tests with mocha alone, which is really blazing fast (and running with the "native" mocha/chai pair, including all available plugins and stuff there like). The test framework is really fat as it is configured currently. The question is, if you need functional tests (in the way books describe them), would it not be better to use something like selenium or webdriver.io for this and use regular unit tests the way it is meant to be? It would really speed up if we would use mocha instead of karma out of the box. There are just some edge cases (e.g. hard integration of cookies, localStorage or websocket connections) that cannot be handled via jsdom.

Have to think about this closer I think.

sthzg commented 8 years ago

I'd argue that Karma also offers a lot in the area of functional testing (e.g. capturing tests from different browsers / although you are right that it's still more on the unit testing side, just in different browser-envs). That's why I enjoy that the basic configuration for it is available out of the box.

However, the argument that the tests that we ship with the generator don't require Karma, but run perfectly fine on Mocha and wo/ taking so much time to launch is absolutely valid.

Would it be an option for you to think in a direction where we refactor the main testing facilities to use mocha cli as test runner, but also keep a configuration and command for Karma tests? Another option would be to create a sub generator that injects a karma setup for people who like to opt in.

sthzg commented 8 years ago

I had the whole test setup working without karma, too. But it just felt incorrect as you have to monkey-patch webpacks require calls.

The one thing I've noticed is that it will obviously stumble over resolving webpack aliases (as I define imports with relative paths most of the time that wasn't too bad for me). Is that an aspect of the monkey-patching you refer to or are there more issues that require monkey-patching?

weblogixx commented 8 years ago

Yes, that is what I meant. Also, you have to add each extension that has a loader for webpack to the mocha exclude pseudo null loader that is patched. You can see this in https://github.com/react-webpack-generators/react-webpack-template/blob/208de3d08d546e80c1f6333bfefd09d5dd46573d/conf/mocha/webpackexclude.js#L25. That means whenever a user adds something to the webpack config (or we do), we will also have to add it to this extension array.

The webpack-alias may also be another problem a user could run into, but as I opted to just return nothing (for completely skipped stuff) or the filename (for images, svgs and the like) that was not the problem as nothing will be loaded by babel anyways.

About react testing: This seems to be in flux currently. In one of the core meeting notes it is said that the core React-Devs are looking into dropping the built in test utils (and jest?) completely in favour of enzyme. Enzyme can be used either via shallow rendering (without any dom) or with fakedom (when using render). Both work out of the box currently, on karma as well as on mocha.

Lets look at it like this:

Karma:

Mocha:

I have to say I am a bit torn here :).

sthzg commented 8 years ago

that's a great response, thanks! Just for reference: with react testing I was referring to the react-test-renderer which was released just the other day. There seems to be a problem when using it in the same file with Enzyme test currently (see issue no. 7386 in the react issue queue). In no way this is comprehensible to what Enzyme has to offer, but I'd at least expect people to ask about using it.

sthzg commented 8 years ago

FWIW, starting up the Karma runner to get the first results takes a little over 2 minutes on a fairly moderate project right now (limited hardware plus the need for the webpack build chain + spinning up a browser).

The tests themselves and subsequent runs in watch mode run super quick, but the startup time was the initial reason I started running many of the tests with Mocha only. I will have tests that need Karma's advanced features (or as you wrote something different in that direction like Selenium), so I wouldn't kick its config out of my project.

To solve the reporting problem I simply have Karma run all of my tests, whereas the Mocha runner runs a subset of them. I understand that this might not be desirable as a default, but in the current use case it does its job. 😄