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

Add In-Browser Unit Testing #5703

Closed
sebmarkbage opened this Issue Dec 21, 2015 · 30 comments

Comments

Projects
None yet
@sebmarkbage
Copy link
Member

sebmarkbage commented Dec 21, 2015

We currently don't run the unit test suite in browsers in open source. We only run the internal Web Driver suites at Facebook, however, that isn't very helpful to external contributors nor is it helpful to browser vendors since they can't run these tests against their own builds.

We disabled it in #4393 but if we can solve those issues or replace it with a new runner that would be great for the community.

@sebmarkbage

This comment has been minimized.

Copy link
Member Author

sebmarkbage commented Dec 21, 2015

We also want public regression benchmarks, tracked here #2154 but that is a separate issue from the unit test support.

@oliverwoodings

This comment has been minimized.

Copy link

oliverwoodings commented Dec 21, 2015

@sebmarkbage just to be clear, are you looking to run the Jest tests in the browser, or are you looking to write new/separate unit tests?

@sebmarkbage

This comment has been minimized.

Copy link
Member Author

sebmarkbage commented Dec 21, 2015

Ideally we would be running the existing tests since jest tests is likely what we will keep writing for.

@oliverwoodings

This comment has been minimized.

Copy link

oliverwoodings commented Dec 21, 2015

Maybe the simplest solution (and the one that would require the least amount of changes to React itself) would be to make some kind of generic browser-capable version of the Jest runner?

@mjackson

This comment has been minimized.

Copy link
Contributor

mjackson commented Dec 21, 2015

IMO the easiest way to get your test suite running in real browsers would be to adopt karma. We use karma + BrowserStack (free for OSS) on both the router and history projects and it works well most of the time. It still requires a bit of tinkering to get it all working correctly and reliably, and fails every once in a while, but in-browser ci testing is a complicated beast.

@oliverwoodings Isn't jest just a tool that you use to organize your tests? If so, wouldn't the easiest thing be to adopt a mature test runner like karma?

@oliverwoodings

This comment has been minimized.

Copy link

oliverwoodings commented Dec 21, 2015

@mjackson jest doesn't just run the tests, it handles mocking/stubbing of commonjs modules as well. It ties you to running your tests on node with jsdom (since it just overrides the global require function).

@mjackson

This comment has been minimized.

Copy link
Contributor

mjackson commented Dec 21, 2015

@oliverwoodings Ah, I see. Thanks for the explanation. I'm used to using a separate bundler (webpack/browserify) that resolves all of my require statements for me so my test framework doesn't actually have to know anything about it.

@cpojer

This comment has been minimized.

Copy link
Contributor

cpojer commented Dec 21, 2015

No one is owning this for Jest but the idea would be to turn Jest into a server that can do all of its resolution on the server and send modules to the client. On the client it would require isolated environments to match jest's behavior. It can be done and I'd be happy to mentor anyone who'd be willing to work on this. We don't really need this at FB but it would be pretty good for the open source community, so I'm supportive.

@gaearon

This comment has been minimized.

Copy link
Member

gaearon commented Dec 21, 2015

Why is impractical for Jest to feed its information to Browserify/Webpack/FB-specific bundler? Even if it is just a server, can't it just deal with module resolution and mocking and leave the bundling to a tool specifically built for this?

Edit: despite the new Collaborator badge I have very little idea about how Jest works with or without React, as I only worked with Mocha and Karma before. I'll say stupid things here!

@cpojer

This comment has been minimized.

Copy link
Contributor

cpojer commented Dec 21, 2015

This Jest PR is also focused on making module resolution more modular: facebook/jest#599 Once that's done we can think of how to provide other module resolution implementations and how they may ship JS to the browser if we end up building this.

@oliverwoodings

This comment has been minimized.

Copy link

oliverwoodings commented Dec 21, 2015

@gaearon I'm thinking the same thing - why cant Jest be a core with plugins for different bundlers? This would make stuff like babel-jest unnecessary and let the user write their tests in whatever flavour of JS is currently popular.

@cpojer

This comment has been minimized.

Copy link
Contributor

cpojer commented Dec 21, 2015

This kind of behavior for Jest could never work for us at FB. We have thousands of test files and tens of thousands of modules and our bundler is completely separate from our JavaScript code. It is incredibly complex, unpredictable for humans and uses machine learning to optimize packages. Using this system to run unit tests would be incredibly slow and is reserved for integration tests that actually open up a web browser to load individual web sites and interact with them.

That being said and given what I said before, I'm completely open to any and all help if anyone wants to spent some time building this. Jest was designed to be modular but we never did a great job of presenting it that way. The PR mentioned above is going to help a lot and I'm going to focus more on making it actually modular and add documentation in the future to highlight all of this. I realize that a lot of projects aren't at Facebook scale and they can benefit from these features.

@oliverwoodings

This comment has been minimized.

Copy link

oliverwoodings commented Dec 21, 2015

@cpojer surely it could be done in a way that made the module loader optional? I.e. the 'default' loader would just be the same behaviour Jest has now.

@sebmarkbage

This comment has been minimized.

Copy link
Member Author

sebmarkbage commented Dec 21, 2015

How will changing the module resolution in jest solve the browser tests? Is this off-topic?

Seems like changing the runner would be much more viable, but it doesn't let us integrate the test into other FB infra and run them in React Native, so if we can avoid it and use a browser version of the jest runner, that would be ideal.

We could also carve out a space for a separate set of DOM related tests since most tests are generic enough that they don't add any additional test of the DOM. That makes contributing a bit of a pain since you need to learn two test runners.

@oliverwoodings

This comment has been minimized.

Copy link

oliverwoodings commented Dec 21, 2015

@sebmarkbage right now we can't use Jest in the browser because of two things:

  • Code is not bundled, meaning it can't be executed easily in the browser. This is especially relevant if the source code needs some kind of pre-compile step (e.g. babel)
  • Even if the code was bundled, changing the module resolution is necessary because right now Jest's module resolution is very specific to node; it overrides require and does the resolution from the filesystem itself in order to mock modules.

Making the module resolution pluggable would mean that any kind of bundler could be used, including ones that target the browser instead of Node.

@sebmarkbage

This comment has been minimized.

Copy link
Member Author

sebmarkbage commented Dec 21, 2015

Wouldn't that be the opposite of a pluggable module resolution in jest? It would mean that you would want to use jest's module resolution in a different bundler, no?

@cpojer

This comment has been minimized.

Copy link
Contributor

cpojer commented Dec 21, 2015

An in-browser runner would most certainly have to ship with its own require implementation. I'm quite convinced that you'd want Jest to play a part in the bundling for test runs. Unit testing generally doesn't want the same kind of bundling (or bundling at all) or the same set of transforms as prod. Please proof me wrong though.

@oliverwoodings

This comment has been minimized.

Copy link

oliverwoodings commented Dec 21, 2015

@cpojer wouldn't that create a restriction that your production code can only use whatever transformation tools are supported by Jest? I get that Jest needs to play some part in the bundling process, but maybe it should just be instructing a loader on what to bundle, rather than doing the actual bundling and transforming itself? Articles like this one are an example of how Jest can make it hard to write production code with newer JS features that aren't supported by Jest.

@oliverwoodings

This comment has been minimized.

Copy link

oliverwoodings commented Dec 21, 2015

@sebmarkbage pluggable in that the transforming and resolution of files can be done by the bundlers - @jsdf seems to be proposing a similar thing in facebook/jest/pull/599

@cpojer

This comment has been minimized.

Copy link
Contributor

cpojer commented Dec 21, 2015

@oliverwoodings oh yes, I agree with that. Part of that PR is to make the transform function async, so that other transformers can be injected more easily.

@oliverwoodings

This comment has been minimized.

Copy link

oliverwoodings commented Dec 21, 2015

I'm confused then by what you mean by an in browser runner having to ship with its own require implementation. Surely this would be provided by browserify/webpack/whatever bundler you are using and not jest? (please excuse me if this is actually what you mean)

@cpojer

This comment has been minimized.

Copy link
Contributor

cpojer commented Dec 22, 2015

Jest is like a VM and virtualizes the JavaScript that is run inside of it. The require implementation can be found here: https://github.com/facebook/jest/blob/master/src/HasteModuleLoader/HasteModuleLoader.js#L510 ( https://github.com/cpojer/jest/blob/haste2/src/HasteModuleLoader/HasteModuleLoader.js#L538 contains a cleaner version and is from the haste2 PR) and is a large part of the module loader. require in jest does not mean entirely the same thing that it does in CommonJS. It has additional features, like mocking and allows to wipe away the cache more easily to be able to build tests that don't carry over state from one test to the next.

@gfogle

This comment has been minimized.

Copy link

gfogle commented Dec 30, 2015

@cpojer - I'd be interested in having a look. I've been working through having better perf tools with React as well (DefaultPerf wasn't providing what I would like) so this would be a nice pairing to perf in multiple browsers as well.

@cpojer

This comment has been minimized.

Copy link
Contributor

cpojer commented Jan 4, 2016

Feel free to join us on discord (see http://facebook.github.io/jest/support.html ) if you want to discuss potential implementations.

@milesj

This comment has been minimized.

Copy link
Contributor

milesj commented Jan 6, 2016

For reference, I currently use Karma (with Browserify and Babelify plugins) + Jasmine + Enzyme to run my React unit tests. This solves the bundling process of source and tests files -- all that's left is updating Jest to work in the browser.

Implementation can be found here: https://github.com/titon/toolkit

@gaearon

This comment has been minimized.

Copy link
Member

gaearon commented Jul 12, 2017

After living with this problem for a while we discovered that Node tests cover our internal logic really well, and browser tests, even if we had them, don’t give us enough confidence. Many browser issues and regressions that we have had are unobservable from the code, and need actual human interaction. They are also very tedious to write and understand.

Instead, we went with a set of manual DOM fixtures. Since React operates on DOM in a very constrained way, and we don’t change this logic often, it is feasible for contributors to go through a manual test suite and verify their changes work as intended in different browsers. This has already caught several regressions, and helped move bugfixes through without too much review bureaucracy.

I think at this point we won’t be investing into running our unit tests in the browser. The combination of Node unit tests and manual DOM tests seems to be working well, and further work might include making DOM fixtures easier to access (e.g. by hosting a link to them from a CI task for every PR). I’ll close this.

@gaearon gaearon closed this Jul 12, 2017

@jdalton

This comment has been minimized.

Copy link
Contributor

jdalton commented Jul 12, 2017

DOM fixtures are great! Thank you!

@vvo

This comment has been minimized.

Copy link

vvo commented Jul 17, 2017

I'd love to know more about this DOM fixtures thing and how it's part of your testing suite. How other communities could benefit from it too (other libraries with DOM tests).

Thanks for the heads up!

@nhunzaker

This comment has been minimized.

Copy link
Collaborator

nhunzaker commented Jul 17, 2017

Hey @vvo!

Basically, JSDOM, and even the JavaScript interface to the DOM, can't accurately reproduce every edge case across every browser. Some things just take manual user input. So we've made a little app to host test cases for manual testing:

https://github.com/facebook/react/tree/master/fixtures/dom

I'd eventually like to host this more publicly. I have a prototype up here:
https://react-dom-fixtures.surge.sh/

I think it would be great if we could share what we've learned with other libraries, and the community at large. I also have a massive backlog of things I need to file on MDN...

There's not much else to it :) We just need to run some stuff in a real browser, using a real user. I would love to hear your thoughts on how we can share this with the community.

@vvo

This comment has been minimized.

Copy link

vvo commented Jul 17, 2017

I would love to hear your thoughts on how we can share this with the community.

I think everyone would expect a library like React to be tested in every browser ETC so the learning and the dom fixtures project might be a good talk subject for the community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment