-
Notifications
You must be signed in to change notification settings - Fork 43
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
Comments
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... |
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. |
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. |
I'd argue that Karma also offers a lot in the area of 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. |
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? |
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
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 :). |
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. |
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. 😄 |
What is your opinion on splitting the root test directory into
Currently I've added a mocha run script to
package.json
.and run some unit tests by:
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.The text was updated successfully, but these errors were encountered: