Skip to content
This repository was archived by the owner on Jan 11, 2019. It is now read-only.

Use ava macros in tests #25

Merged
merged 1 commit into from
Aug 14, 2016
Merged

Conversation

DrewML
Copy link
Contributor

@DrewML DrewML commented Jul 3, 2016

Also added titles to tests

@DrewML
Copy link
Contributor Author

DrewML commented Jul 3, 2016

It looks like jscodeshift quietly dropped support for nodeversions < 4 here. This is the cause of the failing tests.

Was going to open up an issue about the lack of a major version bump for that change, but they're still at major 0, so it's technically valid per semver :(.

Because ava still supports those versions of node, it feels a bit weird to force users of this util to upgrade specifically for the codemods. There are some options:

  1. Could submit a PR to jscodeshift to propose reverting the piece of that changeset that removed babel compilation on build/publish
  2. Could run babel-require with the es2015 preset when we detect a user of this lib is running a version of node not supported by jscodeshift anymore.

Thoughts?

@sindresorhus
Copy link
Member

  1. Could run babel-require with the es2015 preset when we detect a user of this lib is running a version of node not supported by jscodeshift anymore.

We can just do that by default. I believe that's how it used to work.

@DrewML
Copy link
Contributor Author

DrewML commented Jul 4, 2016

Huh, tried to add the babel configuration, but I am just...stumped. Have never had this issue before, with ava or other projects, so I might need a hand.

Here are the steps I took (including updating to babel 6) for node 0.12.x.

  1. npm uninstall --save-dev babel && npm install --save-dev babel-core babel-register babel-preset-es2015
  2. Added "presets": ["es2015"] to the babel config in package.json
  3. Added "node_modules/jscodeshift/**" to the only array in the babel config
  4. Changed the require config under ava to babel-register

I tried a few additional things along the way, including:

  1. Specifying node_modules/** (wouldn't be permanent, but for the sake of testing) in the only array
  2. Switching to a .babelrc to verify this wasn't an issue with it not picking up the config in package.json
  3. Set ignore: false in the babel config, to verify the default ignoring of node_modules wasn't taking precedence over my entry in the only array.
  4. Removed only entirely, and set ignore: false.

Nothing I've tried has seemed to work so far. Any suggestions?

@DrewML DrewML changed the title Use ava assertions in tests, and upgrade ava to latest version Use ava macros in tests, and upgrade ava to latest version Jul 4, 2016
@sindresorhus
Copy link
Member

You might need babel-runtime too.

@DrewML
Copy link
Contributor Author

DrewML commented Jul 5, 2016

You might need babel-runtime too.

Hmm, I don't think that's it, unfortunately. node 0.12.x is failing on a const variable declaration right now, which isn't something babel-runtime handles (just normal transpilation to a var there).

@sindresorhus
Copy link
Member

Can you push your Babel changes in an additional commit? Hard to debug without the code.

@DrewML
Copy link
Contributor Author

DrewML commented Jul 5, 2016

Ha, I can imagine that would help 😛 Pushed

@DrewML
Copy link
Contributor Author

DrewML commented Jul 5, 2016

Current exception is:

SyntaxError: Use of const in strict mode.
    at exports.runInThisContext (vm.js:73:16)
    at Module._compile (module.js:443:25)
    at Module._extensions..js (module.js:478:10)
    at require.extensions.(anonymous function) (/Users/alevine/ava-codemods/node_modules/babel-register/lib/node.js:166:7)
    at extensions.(anonymous function) (/Users/alevine/ava-codemods/node_modules/ava/node_modules/require-precompiled/index.js:16:3)
    at Object.require.extensions.(anonymous function) [as .js] (/Users/alevine/ava-codemods/node_modules/ava/lib/test-worker.js:91:3)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/alevine/ava-codemods/node_modules/jscodeshift/index.js:11:18)
    at Module._compile (module.js:460:26)
    at Module._extensions..js (module.js:478:10)
    at require.extensions.(anonymous function) (/Users/alevine/ava-codemods/node_modules/babel-register/lib/node.js:166:7)
    at extensions.(anonymous function) (/Users/alevine/ava-codemods/node_modules/ava/node_modules/require-precompiled/index.js:16:3)
    at Object.require.extensions.(anonymous function) [as .js] (/Users/alevine/ava-codemods/node_modules/ava/lib/test-worker.js:91:3)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/alevine/ava-codemods/test/_helpers.js:11:20)
    at Module._compile (module.js:460:26)
    at loader (/Users/alevine/ava-codemods/node_modules/babel-register/lib/node.js:158:5)
    at require.extensions.(anonymous function) (/Users/alevine/ava-codemods/node_modules/babel-register/lib/node.js:168:7)
    at extensions.(anonymous function) (/Users/alevine/ava-codemods/node_modules/ava/node_modules/require-precompiled/index.js:16:3)
    at Object.require.extensions.(anonymous function) [as .js] (/Users/alevine/ava-codemods/node_modules/ava/lib/test-worker.js:91:3)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/alevine/ava-codemods/test/ok-to-truthy.js:3:1)
    at Module._compile (module.js:460:26)
    at extensions.(anonymous function) (/Users/alevine/ava-codemods/node_modules/ava/node_modules/require-precompiled/index.js:13:11)
    at Object.require.extensions.(anonymous function) [as .js] (/Users/alevine/ava-codemods/node_modules/ava/lib/test-worker.js:91:3)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/alevine/ava-codemods/node_modules/ava/lib/test-worker.js:95:1)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:501:10)
    at startup (node.js:129:16)
    at node.js:814:3

Which at least shows that ava is running babel-register, and that require in a non-test file is still going through babel

@DrewML
Copy link
Contributor Author

DrewML commented Jul 5, 2016

Interesting - now node versions > 0.12.x && != 4.0.x are failing, but node > 4.0.x is failing due to what looks like an issue with the dependency tree + one of minimatch's dependencies (both locally and in CI).

Update: I updated my npm version locally to 3.10.3 from 3.9.5, and that seems to fix the missing dependency issue that is happening in CI (CI is using 3.9.5).

@jamestalmage
Copy link
Contributor

jamestalmage commented Jul 6, 2016

I wonder if facebook/jscodeshift@327f985 is causing issues with your attempt to upgrade to Babel 6 here.

Really, this PR is doing three things: upgrade AVA, upgrade Babel, switch to macros. It might be best to do those one at a time.

@DrewML
Copy link
Contributor Author

DrewML commented Jul 6, 2016

Really, this PR is doing three things: upgrade AVA, upgrade Babel, switch to macros. It might be best to do those one at a time.

Yeah, the more I worked on it the more I started to realize that. Will be splitting off

@sindresorhus
Copy link
Member

I dropped Node.js 0.10 and 0.12 support here so this should be a non-issue now.

@DrewML Would you be able to finish this?

@DrewML
Copy link
Contributor Author

DrewML commented Aug 13, 2016

Whoops - totally forgot about this - really wish GitHub had some sort of Todo list feature.

I'll get this done this weekend. I ended up figuring out what the issue was here. There is a "bug" where it's impossible to get babel-register to allow transpiling anything in node_modules, unless you pass in the optional arguments to babel-register (it won't respect that info in package.json or .babelrc).

@DrewML DrewML force-pushed the use-ava-assertions branch from 6798f41 to 9726d2d Compare August 13, 2016 02:08
@DrewML
Copy link
Contributor Author

DrewML commented Aug 13, 2016

Alright, getting closer. Other issues are resolved, now just hitting some dependency hell I'm working on debugging. What I know so far:

  • Current setup works when installing deps with npm v2.15.8
  • Current setup works when installing deps with npm v3.8.7, but only if you run npm install twice
  • Current setup fails on exceptions thrown by recast when using npm versions >= v3.8.6

@DrewML DrewML force-pushed the use-ava-assertions branch from 9726d2d to a6f5744 Compare August 13, 2016 02:51
@DrewML DrewML changed the title Use ava macros in tests, and upgrade ava to latest version Use ava macros in tests Aug 13, 2016
@DrewML
Copy link
Contributor Author

DrewML commented Aug 13, 2016

@sindresorhus Tests are passing now. I punted on the Babel 6 upgrade in this PR, because we seem to be hitting a bug with how the tree flattening works in npm v3.x.

jscodeshift has a dependency for babel-core@^5, and we had a dependency on babel-core@^6.10.4. Rather than giving jscodeshift it's own copy of babel-core that satisfies the semver range it has specified, most versions of [email protected] (except for v3.8.7 after 2 installs) fail out and just warn about a missing dependency.

I have an idea for a fix I'll try out in a separate PR.

@sindresorhus
Copy link
Member

Whoops - totally forgot about this - really wish GitHub had some sort of Todo list feature.

It does, kinda: https://github.com/pulls

@sindresorhus
Copy link
Member

@DrewML DrewML force-pushed the use-ava-assertions branch from a6f5744 to 7f7773b Compare August 14, 2016 00:10
@DrewML
Copy link
Contributor Author

DrewML commented Aug 14, 2016

@sindresorhus sindresorhus merged commit d727529 into avajs:master Aug 14, 2016
@sindresorhus
Copy link
Member

Awesome! Thank you :)

@DrewML DrewML deleted the use-ava-assertions branch August 14, 2016 01:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants