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

Allow tests files to have any extension (i.e. .jsx, .ts) #631

Closed
dustinsanders opened this issue Mar 10, 2016 · 84 comments
Closed

Allow tests files to have any extension (i.e. .jsx, .ts) #631

dustinsanders opened this issue Mar 10, 2016 · 84 comments
Assignees
Labels

Comments

@dustinsanders
Copy link

dustinsanders commented Mar 10, 2016

With the latest release(0.13) and the ability to support jsx; I would like the ability to write my test that contains jsx with a jsx extension. However ava is filtering only for files with .js extensions. I'm a little confused as to why ava allows for a glob pattern in the cli but then only filters down to .js files.

I can make a PR if this is something worthwhile to the maintainers.


See conclusion here: #631 (comment)

@sotojuan
Copy link
Contributor

Personally I'd argue that there's no point in using .jsx files in general, but that's for another debate. This would only require changing one line, so I am for it.

@jamestalmage
Copy link
Contributor

Extensions provide hints to the IDE as to which language features should be enabled.

My question is, do we just make a special exception for .jsx, or parse glob strings for extensions?

ava test/*.{js,jsx}

Would enable js and jsx

@dustinsanders
Copy link
Author

I think ideally ava would accept any extension, as long as there is a register hook to get the file to vanilla javascript. I believe this would also fix issue #521.

@jamestalmage
Copy link
Contributor

I think ideally ava would accept any extension, as long as there is a register hook to get the file to vanilla javascript.

I like that thinking. The only difficulty is that we can't know the list of registered extensions in the parent process, we process require hooks in the child.

@novemberborn
Copy link
Member

I think we can support .jsx specifically. We could even add the Babel transform for .jsx files (though perhaps not depend on it).

Accepting any extension is trickier. We might have to swap out the compiler. #577 discusses this with regards to source files but the same would apply here. I suspect we might solve that with an explicit extension registry, not just accepting whatever is matched by the glob patterns.

@jamestalmage
Copy link
Contributor

Why is matching the glob string problematic? If they explicitly define an extension just trust the user will add the require hook themselves (perhaps a helpful message if they don't).

@sindresorhus
Copy link
Member

If they explicitly define an extension just trust the user will add the require hook themselves (perhaps a helpful message if they don't).

👍

@novemberborn
Copy link
Member

If they explicitly define an extension just trust the user will add the require hook themselves

Yes, but this is the part we're currently missing. You can't swap out the Babel precompiler. Adding .jsx support is easier because it uses Babel.

@vadimdemedes
Copy link
Contributor

So, we agreed to accept .jsx extensions in our API?

@vadimdemedes vadimdemedes added the enhancement new functionality label Mar 18, 2016
@jamestalmage
Copy link
Contributor

Yes, but this is the part we're currently missing. You can't swap out the Babel precompiler.

You can by combining babel:false and require: 'some-precompiler'. Let's take coffee-script support for example. A complete config should look like this:

{
  "ava": {
    "require": "coffee-script/register",
    "babel": false,
    "files":  "test/**/*.coffee"
  }
}

JSX should be like this:

{
  "ava": {
    "babel": "inherit" // with .jsx support setup correction in .babelrc,
    "files": "test/**/*.{js,jsx}"
  }
}

So, we agreed to accept .jsx extensions in our API?

I would just as soon not hard code non-standard extensions into AVA if we can avoid it (which I think we can fairly easily in this case).

@novemberborn
Copy link
Member

I would just as soon not hard code non-standard extensions into AVA if we can avoid it (which I think we can fairly easily in this case).

We are currently hard coding for .js so that would have to change.

@sindresorhus
Copy link
Member

We are currently hard coding for .js so that would have to change.

That's because we want $ ava some-directory to just work, without having to specify it like some-directory/**/*.{js,jsx}. We could still support both though, just that if you want other extensions that .js you need an explicit glob.

@novemberborn
Copy link
Member

@sindresorhus so remove code that restricts test files to .js extensions, but leave glob patterns as is? Presumably we're requiring .js extensions in case the glob pattern is too broad so that might become a problem.

@jamestalmage
Copy link
Contributor

I think what @sindresorhus is getting at is the following.

If the glob pattern matches a directory explicitly:

$ ava tests/some-dir

What the user almost certainly meant was:

$ ava test/some-dir/*.js

So we help them out and do that automagically.

If the users glob pattern is specific about extensions:

$ ava test/some-dir/*.jsx

Then we take them at their word. In the above example, .js files should be ruled out. If they want .js as well, they need to do:

$ ava test/some-dir/*.{js,jsx}

So basically, we should be using strict glob pattern matching except in the case where they explicitly match a directory.

@sindresorhus
Copy link
Member

Yes, that's what I meant, and much clearer :)

Nitpick, but it's recursive: $ ava test/some-dir/*.js => $ ava test/some-dir/**/*.js

@novemberborn
Copy link
Member

Without getting into #229 territory we can quite easily support .jsx test files and automatically include the react preset. I suspect this would make AVA much easier to use for React projects. @kentcdodds?

@kentcdodds
Copy link
Contributor

I know that some people use .jsx, but I know more people who just use .js. I think that both should be supported. But I don't know if it'll make it any easier for most people. There are already plenty of steps people must follow to get AVA testing their React stuff.

@novemberborn
Copy link
Member

@kentcdodds happy to accept a PR that adds React's test naming convention, if that removes a step.

I don't think we should apply the react transform to all .js files. Still we could find ways to make that work out of the box, perhaps #619.

@kentcdodds
Copy link
Contributor

React's test naming convention

There are many of these. Not really React specific. Definitely preference based.

I definitely don't believe that we should arbitrarily add plugins and presets to AVA's built-in config. I just mean that if there is an existing babel configuration, users of AVA are obviously transpiling with babel and will assume that the test files will be transpiled in the same way as their source files.

Maybe having an init with informative questions would be helpful?

@novemberborn
Copy link
Member

There are many of these. Not really React specific. Definitely preference based.

Well you got nyc to exclude some patterns (istanbuljs/nyc#199). Conversely maybe AVA should include them.


I definitely don't believe that we should arbitrarily add plugins and presets to AVA's built-in config.

Yea. Still, .jsx is pretty clear-cut IMO.

Maybe having an init with informative questions would be helpful?

👍

@kentcdodds
Copy link
Contributor

Well you got nyc to exclude some patterns (istanbuljs/nyc#199). Conversely maybe AVA should include them.

Ah, I think I misunderstood you. You mean so I don't have to do:

"test": "ava \"app/**/*.test.js\""

But instead I can:

"test": "ava"

That'd be awesome! I'll look into making a PR for that :-)

Yea. Still, .jsx is pretty clear-cut IMO.

Also misunderstood you on this one as well. You're totally right. If they're using .jsx, it would definitely make sense to slap the react preset on there 👍

@novemberborn
Copy link
Member

@sindresorhus @jamestalmage @vdemedes what's your stance on supporting .jsx test files out of the box, including the react transform?

@novemberborn
Copy link
Member

Also, while we could support alternative extensions they'll still be treated as JavaScript. IMO that has less value than supporting .jsx.

@sindresorhus
Copy link
Member

@novemberborn What about those using JSX and React with a .js extension? I guess we shouldn't run the React transform on everything.

@novemberborn novemberborn removed the blocked waiting on something else to be resolved first label May 4, 2017
@rauschma
Copy link

rauschma commented May 4, 2017

Awesome! If you do babel.extensions, I suspect you need to support babel.inherit (or similar), too, given that babel: "inherit" can’t be specified, anymore.

@novemberborn
Copy link
Member

Good point. We should make sure to retain compatibility with the current use of the babel options (until we change it all as part of RFC 001).

@rauschma
Copy link

rauschma commented May 4, 2017

An easy preliminary solution would be to introduce a property babel.value that takes over the role that babel had, previously.

MrBri pushed a commit to MrBri/dotfiles that referenced this issue May 11, 2017
@Jamesernator
Copy link

I'd like to see this so that @std/esm can be used to run ESM tests e.g.:

// test.mjs
import test from "ava"
import foo from "../esm.mjs"

test(t => {
    t.is(foo(), 2)
})

// esm.mjs
export default function foo() {
    return 2
}

// package.json
{
    "ava": {
        "require": ["@std/esm"]
    }
}

Given that .mjs is going to be the way to distinguish ESM from CommonJS in Node this should be supported even without a more generic way of supporting other extensions.

@novemberborn
Copy link
Member

@Jamesernator there's an ongoing discussion around @std/esm in #1512.

@cameron-martin
Copy link

This will become even more important once babel 7 is released, which has the ability to parse and remove the type annotations from Typescript files.

@vanga
Copy link

vanga commented Dec 28, 2017

Hello,

I want to use ava for server-side(Coffee-Script) testing, I want to write tests also in Coffee. Based on my searching around, I see that there are ways to achieve this. Some posts suggest to use --require, but that seems to have been deprecated.

{
"ava": {
"require": "coffee-script/register",
"babel": false,
"files": "test/**/*.coffee"
}
}

Is this still relevant? I tried to do something like this and was unsuccesful.

Can someone please tell me if what I am trying to do is possible and point me in the right direction if it is possible.

Thanks.

@novemberborn
Copy link
Member

@vanga AVA performs some transpilations on the test files. These assume the test files contain JavaScript. Until those transpilations can be disabled we cannot support arbitrary file extensions, or indeed support non-JavaScript test files.

@vanga
Copy link

vanga commented Jan 10, 2018

Thanks @novemberborn
I guess I will try having a wrapper that compiles coffee script test files to JS before starting ava tests. That's enough for me if it works.

@feross
Copy link

feross commented Jan 15, 2018

@vanga AVA performs some transpilations on the test files. These assume the test files contain JavaScript. Until those transpilations can be disabled we cannot support arbitrary file extensions, or indeed support non-JavaScript test files.

Makes sense. But why can't .mjs be treated as JavaScript? That extension will always contain JavaScript. ava won't run tests even when specifying an .mjs file explicitly, which is preventing the testing of ES Module code:

$ ava test/basic.mjs

  1 exception

  ✖ Couldn't find any files to test

cc @jdalton

@novemberborn
Copy link
Member

@feross I think we could recognize .mjs files, yes. We'd have to properly resolve imports and enforce strict mode though. I'd accept a PR for that, separate from completely disabling Babel and supporting an extensions option.

@feross
Copy link

feross commented Jan 15, 2018

@novemberborn That's great to hear! I was just investigating porting tests from tape to ava for a few minutes, so I'm not invested enough in ava yet to dig into the code to send a PR. Hopefully someone who's more familiar can jump in and try this?

@novemberborn
Copy link
Member

Our globbing is quite broken but after I land #1608 I intend to implement extensions for our Babel pipeline, and probably top-level too which would let you specify which file extensions should be recognized by AVA without Babel compilation. (If these are not JS files you'd have to disable the new compileEnhancements flag.)

@greym0uth
Copy link
Contributor

@novemberborn Do we have an update on this issue?

@novemberborn
Copy link
Member

@Jaden-Giordano I've been focused on landing other improvements and breaking changes in preparation for a 1.0 release. That said we have enough pieces in place for extensions to be added. If you'd be willing to land a hand that'd be great.

@greym0uth
Copy link
Contributor

greym0uth commented Mar 19, 2018

@novemberborn So really the only issue is that ava is filtering based on the extensions hard coded:
https://github.com/avajs/ava/blob/master/lib/ava-files.js#L60
We shouldn't need to filter since the glob already handles that with the comma separated lists, i.e. **/*.{js,jsx}. I say we should just remove this line it should work just fine. Let me know what you think and I'll create the PR if you approve.

Edit:
Just tested it out by removing the line and setting the files option to tests/**/*.{js,jsx}; it finds all the files and runs all the tests. Here is the test directory I used:

tests/
-a/
--test-in-folder.js
--react-test-in-folder.jsx
-test.js
-react-test.jsx

I'm going to make the pull request so we can fine tune it there.

@novemberborn
Copy link
Member

@Jaden-Giordano it's not that easy I'm afraid. We need to hook this up to our Babel pipeline, and separately our compiled enhancements. (Though currently those are implemented using the Babel pipeline, say when we bring in TypeScript we may have a TypeScript-specific implementation.)

So ava.babel.extensions = [] should contain test file extensions to which the Babel pipeline is applied, as well as the compiled enhancements. ava.extensions = [] should contain test file extensions to which only the compiled enhancements are applied. To avoid ambiguities you shouldn't be allowed to specify the same extension in both lists. Also, we need to make sure the watcher recognizes these new extensions.

We need this refinement so you can specify ava.extensions = ['.ts'] and automatically opt-out of Babel compilation. Though until we have full TypeScript support you'll also have to set ava.compileEnhancements = false.

@greym0uth
Copy link
Contributor

greym0uth commented Mar 21, 2018

So basically ava.babel.extensions should precompile with the CachingPrecompiler and ava.extensions should opt-out of that? Which would mean the writer of the tests utilizing ava.extensions would need to precompile the tests themselves. Is that the intended flow?

Edit:
Also are we allow both extension providers to be used? If so, you mentioned not allowing the same extension in each so we can throw an error in that case.

@greym0uth
Copy link
Contributor

@novemberborn

@novemberborn
Copy link
Member

So basically ava.babel.extensions should precompile with the CachingPrecompiler and ava.extensions should opt-out of that?

Yes, albeit with the nuance that the compileEnhancement setting is currently implemented through the precompiler. It's a bit roundabout, I know.

Which would mean the writer of the tests utilizing ava.extensions would need to precompile the tests themselves.

The test files may not even need precompiling, or alternatively you might use use something like ts-node/register to compile them on the fly.

@jamesthurley
Copy link

To help those looking for what you need to change now this issue is closed:

With AVA version 1.0.0-beta.5.1 I used the following settings to successfully run my .ts tests:

    "babel": false,
    "extensions": [
      "ts"
    ],
    "require": [
      "ts-node/register"
    ]

Thanks @novemberborn and @Jaden-Giordano for getting this working!

@novemberborn
Copy link
Member

If anybody's game for updating the TypeScript recipe that'd be great. See #1822 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests