Skip to content

Please do not recommend browser-env for browser testing #1092

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

Closed
domenic opened this issue Oct 23, 2016 · 9 comments
Closed

Please do not recommend browser-env for browser testing #1092

domenic opened this issue Oct 23, 2016 · 9 comments

Comments

@domenic
Copy link

domenic commented Oct 23, 2016

That package uses an explicitly unsupported jsdom anti-pattern and is very bad practice to recommend to consumers of ava.

@sindresorhus
Copy link
Member

// @lukechilds

@lukechilds
Copy link
Contributor

lukechilds commented Oct 23, 2016

Regarding the two main points in that link, the docs do recommend only requiring the properties needed to prevent unnecessary globals and I don't think attaching them to global is as big of an issue as it normally would be due to the way AVA runs each test file in a separate process.

That said, I'm aware this isn't the cleanest solution in the world. To match the example I think we would have create a new jsdom instance inside every test because they all run async in AVA. It would also require that the module is first built for the browser so it can be injected in via a script tag.

I feel like the current solution gets you 99% of the way there in one function call and makes sense until AVA is running natively in the browser.

@domenic
Copy link
Author

domenic commented Oct 23, 2016

It would also require that the module is first built for the browser so it can be injected in via a script tag.

Yes. That is exactly the point! You need to test the same scenario as you will be deploying to your users.

@lukechilds
Copy link
Contributor

I understand your point. I'm not quite sure how you would get the react example working in the current recipe. It loads extra modules for testing which would also need to be built for browsers.

I don't disagree with what you're saying, I still think this method is ok to easily test simple modules, but maybe it would be better to not recommend browser-env because more complex modules may end up passing tests by using Node.js functionality that wouldn't be available in a sandboxed jsdom environment.

@Akkuma
Copy link

Akkuma commented Oct 24, 2016

To defend browser-env. I feel it does exactly what it should/could do. I'm using it for testing a browser web extension, which is inherently even more difficult to test. I already have browser automation tests to guarantee if X is done Y happens. However, I can see the need for it to go away once AVA supports running in a real browser.

@domenic
Copy link
Author

domenic commented Oct 24, 2016

I understand that some people want to abuse jsdom in ways it's not meant to be used and that are not supported. I especially understand if the author of a module geared around doing so wants to do that. But I don't think a widely-used test-runner such as ava should be recommending such practices.

@lukechilds
Copy link
Contributor

lukechilds commented Oct 24, 2016

I can see your concern. Maybe we should add a disclaimer in the browser recipe that makes it clear that this is not recommended and is a temporary workaround until AVA is supported in the browser.

I've just added a disclaimer to the browser-env repo linking to the above wiki page: https://github.com/lukechilds/browser-env

@novemberborn
Copy link
Member

Assuming your browser code runs under Node.js without compilation then IMHO browser-env is still a good way of getting enough globals and DOM in place to exercise that code. AVA at least isolates different test files from each other. Our current browser testing recipe focuses on these kinds of tests.

Ideally though you'd compile your browser code first and test that in a less "hybrid franken-environment".

I'd like to see the recipe address this distinction. I'm curious how people exercise their browser code though, there may be quite a habit of running it directly in Node.js rather than properly isolated in jsdom.

@novemberborn
Copy link
Member

Addressed with #1355.

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

5 participants