Feature/tests #74
Feature/tests #74
Conversation
|
Hold on a sec, can you go into the impact of the downgrade? What bugfixes and or features are missing in the newer point releases? |
|
https://github.com/joyent/node/wiki/API-changes-between-v0.10-and-v0.12 describes the changes in the node versions. There's nothing in there that we directly use, and I haven't encountered any problems using 0.10 locally. jestjs/jest#243 has some more details about jest and node 0.12. Basically, jest officially supports node 0.8.x and 0.10.x. Jest depends on an old version of jsdom which supports node 0.10 but not node 0.12. And they can't exactly just upgrade jsdom since jsdom dropped support for node entirely in favor of io.js. Looks like FB themselves primarily use node 0.10.x for React and Jest, and that is by far the simplest working configuration. |
|
I know this will force a restart, but can we avoid this by selecting a different test harness? I'd like to avoid the node/io stuff while they sort out getting back together. Got a suggestion that still uses Jasmine: https://github.com/tommyh/jasmine-react |
|
another test harness may avoid the node and io's relationship drama, but frankly jest's automatic mocking is nice. Like really nice. And dealing with the downgrade is way simpler and easier than reworking the tests without jest. Everything works just dandy with the latest stable 0.10.x. |
|
and since most of the application logic is in models that are based on flux stores, good mocking of the "backend" classes (https://facebook.github.io/react/blog/2014/09/24/testing-flux-applications.html) is more important than utilities for testing React view components |
|
I get it, and I would be with you except that it's not just downgrading...it's getting stuck on a particular version of node until the project either opts to replace a component or waiting until node/io work things out. Both are long timescales that WFTDA will have to deal with long after you've made this decision now. |
|
That's true of depending on node 0.12 as well. We can't really account for future releases of node supporting every dependency of our application. No matter what we're stuck targeting a specific engine right now, and potentially upgrading to new releases of node would be a maintenance effort regardless. I don't think there's a substantial difference between targeting node 0.10 and node 0.12 right now given that the API changes between them don't directly affect any of our application code. |
|
@kweerious Matching versions that work is just part of dependency management and a necessary evil in all software projects. |
|
@gjacobrobertson, @thephw as a developer I'm well aware of dependency management and what it means to add outside things to your app. Every piece we add to BoutTime is something we will manage in the future, and every piece is something that will affect the ability to progress forward. That is a software reality. If you want to explain or argue about that some more, we can Slack about it. As for this known issue, since I think we should focus on the things we know are problems right now, I believe this isn't something we can just ignore. I was well aware of that open issue in Jest when I commented earlier. That issue has been open since Feb and only grows in pain and frustration for the people commenting on that ticket. What would it take to choose a different test harness? |
|
well like you've said it would force a restart on our test code. I've got a fair bit more that will be ready to push shortly that depends on jest. That jasmine-react module looks to have some helpers for testing react components, but more importantly we would need to deal with mocking our models and supporting classes such as clocks. Other testing frameworks have some utilities for making mocks manually but nothing comes close to Jest's auto-mocking capabilities. Dealing with manually mocking everything would greatly increase the development time and complexity of our tests. Another issue is testing timers. We need to be able to simulate time advancement for efficient testing of timer-related code without relying on real time to elapse. Jest provides built in timer mocks and functions simulating time passage, so we would either need to write similar capabilities ourselves or find another testing dependency for dealing with timers. furthermore, looking at jasmine-react, it hasn't been updated in nearly a year and doesn't appear to be getting a lot of use. Also, It relies on tests being run in-browser, presumably though jasmine's spec runner on a document with specific markup. It's quite ideal to simulate the dom in js rather than relying on a browser. In fact our CI system depends on exactly that, using So basically, switching test harnesses would be more painful and more expensive than just dealing with the node 0.10 dependency, which as @thephw pointed out is stable and in lifecycle. |
|
Do you have an estimate for something like that? I don't care if it's jasmine-react or something else, I just want all the facts. And if we need to use something like phantomjs that is fine. I'm sorry if you are frustrated. Can we raise and discuss these issues earlier, then WIP code would not pile up. |
|
Please STOP working on this until this is sorted. We are trying to carefully allot the hours we have to pay for thank you. |
|
@kweerious Getting the updated test framework in place is important toward moving forward in a timely manner without introducing regressions. We can try alternatives, but they will lack the features of Jest over vanilla jasmine and it will take more time to implement the tests (see: Why not just use vanilla Jasmine? https://facebook.github.io/jest). Help me understand the concern that we would be addressing by replacing the testing framework. Is the concern that we should be switching to io.js to get a later version or that using node v0.10 introduces meaningful regressions to the project? Inevitably both parties are coming back together under one project in the long-term. It seems the sensible short-term solution would be to version manage until they reconcile projects and merge. |
|
Note for the record: issue handled offline, PR can continue as currently planned, sprint planning meetings scheduled for the future. |
|
Thanks for dropping that heads up here, for those of us following along at On Tue, Jun 2, 2015 at 3:06 PM, dedi hubbard [email protected]
|
|
Test coverage is incomplete but I think this PR is ready to be merged as a baseline for more tests to be written as part of issue PRs |
This PR introduces unit tests using the Jest framework.
Jest is incompatible with node 0.12, so after checking out or merging this branch, you will need to downgrade node to 0.10. This can be accomplished with a tool such as nvm. After switching to node 0.10, cleanly rebuild the node dependencies with
rm -rf node_modules & npm install