Conversions for tape

[jamestalmage]

avajs/ava#644

[DrewML]

I started working on this here

Some things it doesn't handle so far, that I know of:

1. The options parameter to test
2. The onFinish event
3. timeoutAfter
4. t.comment
5. createStream
6. Renaming the argument to the test callback to t, if it's not already using that identifier (If I recall, some of the static analysis ava does requires that identifier)

I should note that this changes every test call to test.serial. Tests in tape run serially, and I don't think it's possible to use static analysis to determine whether any test cases are "safe" to run in parallel.

TODO:

• Handle transforming of import declaration for tape, in addition to existing handling of require
• Print warning for things that can't be automagically fixed (assertions that don't map 1:1 to ava, as an example)
• Rename first param in test callback function, if it is not t (and replace usage of identifier in block)
• Tests

[DrewML]

@jamestalmage @sindresorhus How do y'all want to handle the scenario where a feature is used that doesn't have an equivalent in ava? I looked for prior art in lib, but those are only ava >> ava transforms.

We could print them to stdout, but that's typically not very pretty with jscodeshift since the logs are intermingled with the normal jscodeshift output.

The other option with jscodeshift is to use the stats function, but this is only useful with the --dry flag.

[sindresorhus]

Do you have an example of a feature? We could maybe add comments in the code about stuff that needs manual migration.

[sindresorhus]

I started working on this here

Awesome! :)

I should note that this changes every test call to test.serial. Tests in tape run serially, and I don't think it's possible to use static analysis to determine whether any test cases are "safe" to run in parallel.

👍

[DrewML]

A general list of examples would be the list of things not handled in this comment.

An explicit example for the sake of discussion would be the optional options parameter to the test function in tape.

We could definitely map the skip option to test.skip in ava, but (to the best of my knowledge), I don't think there are equivalents for the other 2.

[sindresorhus]

Ok. We should do a best effort and back out gracefully if we can't confidently do the change.

In the above example we could look at the options if inline. If opts.skip, use test.skip. If any of the other two option, just ignore, as the last one doesn't matter, and the timeout neither, since we don't timeout ever. Although, I've personally never seen anyone use the options parameter. Pretty edge-casy I think.

[skovhus]

Hi @sindresorhus and @DrewML

I don't know the status of this, but I've made some progress on creating a codemod.

I think is solves most transformations including imports/requires, assertions, Tape test function options, t.throws incompatibilities, t.onFinish, etc.

What is missing is:

• Figure out how to handle async tape tests... Currently I'm just removing all t.end, but that is not correct if we have an async test. Then the async function will never be reached and AVA will just pass the test although some assert in an async function might fail.
• Rename first param in test callback function, if it is not t (and replace usage of identifier in block) [from above]
• integrate with ava-codemods + write tests

See
https://gist.github.com/skovhus/5a9e904eb39b3e4f74c1e11ac1f97b12

The tricky part is how to handle async tape tests... I guess something that has an t.end can safely be transferred to t.cb(...). Ideas are more than welcome.

Let me know if I should create a pull requests for this, and if you have some comments to the missing pieces mentioned above. : )

[sindresorhus]

@skovhus Awesome! A pull request would be great :)

The tricky part is how to handle async tape tests... I guess something that has an t.end can safely be transferred to t.cb(...). Ideas are more than welcome.

Yup. I don't see how we could safely statically determine whether a test is sync or async with tape as there is no indication. Safer to just transform all tests to test.cb and tell the user to manually go through and convert the synchronous tests.

// NOTE: t.skip can be implemented with test.skip, but is maybe not the same
// semantically (e.g. t.skip can be called inside conditional branches).
'skip',

AVA does support skipping assertions: https://github.com/avajs/ava#skipping-assertions

Tape also does not have a real true/false assertion, they're all just truthy, so we should also recommend users to go through and use t.true()/t.false() wherever possible instead of t.truthy()/.t.falsy().

Same with .serial. We should recommend users go through the tests and make as much as possible concurrent.

[skovhus]

@sindresorhus Cool! Thanks for the feedback.

Yes, so now I'm converting all tests containing a t.end to a test.cb. This shouldn't be necessary if t.end is called in the same outer scope of the test function, but that is a optimization for later.

I'll to wrap it up and send a pull request. 👍

[sindresorhus]

This shouldn't be necessary if t.end is called in the same outer scope of the test function, but that is a optimization for later.

Good point. If you don't plan on fixing that in your pull request, would you mind opening an issue so we don't forget?

[skovhus]

Good point. If you don't plan on fixing that in your pull request, would you mind opening an issue so we don't forget?

I will : )