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

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

Open
sthzg opened this issue Aug 8, 2016 · 8 comments
Open

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

sthzg opened this issue Aug 8, 2016 · 8 comments

Comments

@sthzg
Copy link
Member

sthzg commented Aug 8, 2016

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
Copy link
Member

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 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
Copy link
Member Author

sthzg commented Aug 8, 2016

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

  • speed of a test run (no expensive spin-up)
  • mocha cli options (mainly the grep and fgrep options to run particular tests)
  • IDE integration (although many IDEs support both, mocha and karma tests)

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
Copy link
Member

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
Copy link
Member Author

sthzg commented Aug 8, 2016

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
Copy link
Member Author

sthzg commented Aug 9, 2016

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
Copy link
Member

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

const completelySkippedExtensions = [
. 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:

  • + is already included
  • + setup is tried and true, runs nicely without any bigger issues
  • + has support for hot reloading out of the box (with current webpack setup)
  • + runs on a "real" browser (or more)
  • - Subsequent tests fail if one assertion fails (e.g. when testing stores)
  • - has much longer spinup times because of webpack + browser
  • - has many (many!) dependencies that affect the toolchains size currently (just look at all those karma-* packages)
  • - does not need any more configuration in enzyme / mocha / whatever directly (most stuff is done via plugins)

Mocha:

  • + is already included, albeit not as testrunner
  • + is much more lightweight as it does not need a browser or webpack at all
  • + makes it possible to grep (to be fair this is in fact also possible with karma, but you will still need to spin up the whole webpack stuff in it)
  • + "Real" Unittests, as each file is tested individually and does not affect the already built application
  • + less problems because no browser is needed (I once ran into a phantom memory leak which made phantomjs-2 unable to run a complete testsuite more than 3 or 4 times, suite had about 2000 tests and 6000 assertions)
  • - eventually missing support for inbrowser stuff (e.g. ajax, websocket, localStorage...). This means you WILL always need something like sinon to stub/mock those stuff. Can (but must not) be a problem when using flux with remote sources.
  • - we will likely have to maintain the fake webpack loader for it to work. I do not have any deeper insights into this, so it may be tricky to support.
  • - If you wanto to have integration or functional tests, you need to setup something like webdriver.io, karma or whatever yourself.

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

@sthzg
Copy link
Member Author

sthzg commented Aug 9, 2016

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
Copy link
Member Author

sthzg commented Aug 11, 2016

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

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

2 participants