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

Consider allowing custom test preprocessors #229

Closed
billyjanitsch opened this issue Nov 17, 2015 · 16 comments
Closed

Consider allowing custom test preprocessors #229

billyjanitsch opened this issue Nov 17, 2015 · 16 comments
Labels
enhancement new functionality future we'll get back to this eventually low priority

Comments

@billyjanitsch
Copy link
Contributor

I understand the philosophy of keeping the test environment separate from the lib environment. If I'm writing a lib in ES5 but want to write tests in ES6, it makes sense, and I think it should be kept as a default behavior. However, there are cases where AVA's custom transpiling and non-polluting require are more of a hinderance than a benefit.

Suppose I'm already transpiling my lib with a transpiler more advanced than AVA's (e.g. ES6/7 stage 0 + JSX + additional babel plugins). I probably want to transpile my tests with the same transpiler to let them make use of new language features, and for the sake of consistent style and linting across my project. I also probably want my tests' dependencies transpiled automatically. #202 and #111 would help, but only if I'm using babel.

Suppose I'm using webpack instead (with babel-loader, but also css-loader to handle requiring CSS modules). I can't compile my build before running tests without losing test access to all of my lib's non-exported internals, and there's no import 'babel-core/register'-type hook I can include in each of my test files. A custom compiler for my tests' dependencies as suggested in #111 would be inefficient, since if a test imported more than one file, webpack would have to compile them separately.

Instead, I want an option to specify a custom pre-processor for my test files, overwriting the one that AVA provides by default. For example, I could pass babel-core/register (which would make my tests' dependencies compile automatically, using my own babel settings), or a custom one which handles webpack integration. This is what Mocha and Karma do, and I think it's a very clean solution.

@jamestalmage
Copy link
Contributor

A custom compiler for my tests' dependencies as suggested in #111 would be inefficient, since if a test imported more than one file, webpack would have to compile them separately.

Off topic here, but Babel suffers from the same issue across processes. Since AVA runs each test in a separate process, if you import it test-a.js, and test-b.js it compiles it once in each process. There is work on a solution (#189), that might be usable to help with that.

Right now, the intricacies of getting this to work well and work fast are challenging enough targeting Babel only.

Any custom pre-processor you wrote would have to handle the IPC protocol we already have in place, would have to apply the power-assert transform on its own. As well as integrate communication of uncaught exceptions, and unhandled promise rejections. All that is currently bundled in babel.js. Some would be fairly easy to extract and make generic, some would not.

An additional concern is that this project is currently moving REALLY fast. Maintaining multiple preprocessor solutions at this point will slow us down, which I think would be bad for the project overall.

That said, I think there is value to this at some point. I just don't think it should be a priority.

Me of today: 👎
Me of the future: 👍

@billyjanitsch
Copy link
Contributor Author

Thanks for your thoughts!

I think there is value to this at some point. I just don't think it should be a priority.

It depends on how you want to develop the project, of course, but FWIW, AVA is currently unable to test anything that uses webpack's require aliasing -- which includes a ton of projects these days. It would be great to at least think about a workaround for this case.

Any custom pre-processor you wrote would have to handle the IPC protocol we already have in place, would have to apply the power-assert transform on its own. As well as integrate communication of uncaught exceptions, and unhandled promise rejections. All that is currently bundled in babel.js. Some would be fairly easy to extract and make generic, some would not.

I agree that this would be nice, but I don't think it's immediately necessary. For now, I'd be fine with a simple layer between the tests and AVA which runs an (optional) pre-processor on each test before passing it off to the existing implementation. The assertion errors, etc. would all be on transpiled code, which certainly isn't ideal, but still a huge improvement over the current state.

A better implementation could be figured out once the project is a little more stable.

Maintaining multiple preprocessor solutions at this point will slow us down, which I think would be bad for the project overall.

True, but I wasn't thinking that AVA would actually implement any custom pre-processors. It would just support them if provided.

If you're at all convinced, I'm happy to work on a PR proposal.

@jamestalmage
Copy link
Contributor

If you're at all convinced, I'm happy to work on a PR proposal.

Wait for a project maintainer to chime in.

@sindresorhus
Copy link
Member

The main focus for AVA right now is testing Node.js things. We do intend to support browser use-cases too, but that takes the backseat for now.

Would exposing the Node.js binary --require flag while disabling our Babel hook solve your problem?
So you could for example do: $ ava --require=babel-core/register --require=jsx/register

@vdemedes Thoughts?

@vadimdemedes
Copy link
Contributor

Would exposing the Node.js binary --require flag while disabling our Babel hook solve your problem?

Perfect solution for this issue. I have nothing against this, should be clean to implement ;)

@jokeyrhyme
Copy link
Contributor

With #296 merged, it seems that we might be able to resolve this now...

@jamestalmage
Copy link
Contributor

It's almost there, we really need istanbuljs/nyc#90 to land for this to work reliably (which is itself waiting on istanbuljs/nyc#89). A single .js extension will probably work before then. Once it lands though, it will be a lot easier to integrate all this.

We may also be waiting for babel/babel#3139 before it all works the way I want it to in the end.

I have not looked at how webpack handles "require aliasing". Hopefully they extend require.extensions correctly.

We are getting there.

@novemberborn
Copy link
Member

#448 will let you customize pre-processing via Babel. Please file a new issue with some sample use cases for a non-Babel pre-processor.

@billyjanitsch
Copy link
Contributor Author

@novemberborn It's great that AVA allows custom Babel config now, but this issue was already specifically about custom non-Babel pre-processors. There are some sample use cases in my original post:

  • webpack with any non-standard loader, e.g. css-loader
  • a custom require hook like those allowed by Mocha (may include things like custom require aliasing, environment polyfills, global test environment behavior)

@vadimdemedes vadimdemedes reopened this Feb 29, 2016
@jokeyrhyme
Copy link
Contributor

@billyjanitsch custom require hooks are supported now via the --require CLI option.

@billyjanitsch
Copy link
Contributor Author

@jokeyrhyme See #296 (comment). Unless things have changed since that PR, --require hooks lib code imported by tests but not the test code itself.

Mocha's --compilers implements the behavior that I'm interested in: a custom pre-processor for the tests (which cascades into their imports)

@novemberborn
Copy link
Member

@billyjanitsch,

It's great that AVA allows custom Babel config now, but this issue was already specifically about custom non-Babel pre-processors.

Yes, fair enough. Apologies for closing.

There are some sample use cases in my original post:

webpack with any non-standard loader, e.g. css-loader

It'd be great if you could point us to such a test set up, with Mocha or similar. I'm still having a hard time understanding what would be necessary from AVA's side.

Unless things have changed since that PR, --require hooks lib code imported by tests but not the test code itself.

Any module you --require is loaded in the test process. Your example should work just fine.

@novemberborn novemberborn changed the title Consider allowing custom preprocessors Consider allowing custom test preprocessors Apr 5, 2016
@novemberborn
Copy link
Member

This would also enable AVA to execute test files written in CoffeeScript or TypeScript.

@jamestalmage
Copy link
Contributor

Closing in favor of #722 and #631 (comment).

Once we allow any extension for tests, this is already solved using --require.

#722 proposes a way to speed up alternate transpilers by allowing them to participate in the main thread and caching precompiler

@novemberborn
Copy link
Member

@jamestalmage I think we should keep this issue open and focused on supporting different precompilers for test files and related changes. #722 should focus on improving source file transpilation.

@novemberborn novemberborn reopened this Apr 6, 2016
@novemberborn novemberborn added the future we'll get back to this eventually label Apr 6, 2016
@novemberborn
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new functionality future we'll get back to this eventually low priority
Projects
None yet
Development

No branches or pull requests

6 participants