Unnecessarily inserts await

[amk221]

If this code mod is run on an app a second time (to update more tests that have since been added), then it does this:

click('button'); // Intentionally no await
await waitUntil(() => { ... });

await click('button'); // Intentionally no await
await waitUntil(() => { ... });

This then breaks the test suite because await click waits for everything to settle, so waitUntil can't do it's thing.

[snewcomer]

Is the condition inside waitUntil ephemeral? Meaning, even after a macro task tick + settledness, I would imagine the condition would still be truthy?

[amk221]

I’ll set up a test repo. It could well be me doing something wrong.

[amk221]

Here is a test repo.

If the codemod is run against it, it will insert await, where there was intentionally no await to begin with.

Related discord conversation (from July 2019)

[snewcomer]

Oh you are testing intermediate states. My bad.

Because tests were not async/await before, the more general assumption of this codemod is that the tests are now enabled with this feature. So I'm not sure there is a way to infer that you are testing intermediate states with the codemod sadly.

Reference tests:

https://github.com/ember-codemods/ember-test-helpers-codemod/blob/master/transforms/integration/__testfixtures__/click.input.js

https://github.com/ember-codemods/ember-test-helpers-codemod/blob/master/transforms/integration/__testfixtures__/click.output.js

[amk221]

Are you saying it’s not possible to fix?

It’s a very annoying bug... consider a large app with a big team of varying ember experience...

The ‘old’ (or less desirable) techniques for testing are still perfectly, so it’s still useful to run this codemod against an app that it has already been run on.

But this means we then have to go through the diff (which could be huge, and remove all the awaits that have been put back.

[snewcomer]

I'm sure we could avoid adding await if the callback is already async/await enabled (thus we can make the a reasonable assumption you are testing an intermediate state). But otherwise no. Do you think that might help?

[amk221]

I think so! Tbh I'm surprised nobody else has mention this before :D