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

t.throws() causes Node syntax error if first argument is function call with await arguments #987

Closed
nesheroj opened this issue Aug 3, 2016 · 12 comments
Labels
bug current functionality does not work as desired help wanted

Comments

@nesheroj
Copy link

nesheroj commented Aug 3, 2016

Description

I have a test that breaks after updating ava to version ~0.15.2 that works when using [email protected]. This has been tested on node 6.3.1 and 4.2.4 amongst others. and seems related to a missing "use strict" after transpiling ES6 modules (wild guess there). I've looked at the commits between the two versions but haven't been able to identify what might cause the problem, but I haven't tested individual commits due to lack of time.

Test Source, error message, and configuration

You can see the relevant files and error message here: https://gist.github.com/nesukun/f8cb2eef133548224d21bfd8a3ae5aa4

As a side note, using AVA's default babel config does not solve or alter the result. Hope you don't mind having everything in a gist, as I prepared that for the gitter channel before opening the issue.

Environment

Tested mainly using the versions of node specified above under macOS 10.11.5 and inside the docker default node image

@vadimdemedes
Copy link
Contributor

Let's see how test files look between different AVA versions:

  1. switch to 0.14.0
  2. rm -rf node_modules/.cache/ava
  3. npm test -- test/server/core/crypto.js (to test that specific file, that's causing trouble)
  4. save the contents of node_modules/.cache/ava/*.js file in a gist
  5. switch to latest AVA (0.16.0)
  6. rm -rf node_modules/.cache/ava
  7. npm test -- test/server/core/crypto.js
  8. save the contents of node_modules/.cache/ava/*.js file in a gist
  9. post the gists here

@nesheroj
Copy link
Author

nesheroj commented Aug 9, 2016

@vadimdemedes
Copy link
Contributor

Could you paste them without diff (as separate files/gists)?

@sotojuan
Copy link
Contributor

@nesukun Sorry for the delay. Are you having the same issues in 0.16?

@nesheroj
Copy link
Author

Hi @sotojuan,
Yes, my lastest samples are using whichever where the lastest 0.14 and 0.16 versions when I made the comment. If you think it might have been resolved since then, or it could help you debug better, I could try again whith the current version.

@sotojuan
Copy link
Contributor

Sorry, didn't see the 0.16.x! I'll investigate.

@novemberborn
Copy link
Member

This is the failing line:

t.throws(checkAgainst('4p455w0rd', await hash('4n0th3rp455w0rd')));

It's transpiled into:

t.throws(_avaThrowsHelper(function () {
  return (0, _crypto.checkAgainst)('4p455w0rd', (yield (0, _crypto.hash)('4n0th3rp455w0rd')));
}, {
  line: 21,
  column: 11,
  source: 'checkAgainst(\'4p455w0rd\', await hash(\'4n0th3rp455w0rd\'))',
  filename: '/private/var/folders/_6/p8qxp_3n62zg9081tvb0lcc80000gn/T/tmp.dkCnTDjWip/mawpedia/test/server/core/crypto.js'
}));

The function wrapper was introduced in #742. It breaks the async/await transpilation, since the yield (0, _crypto.hash)('4n0th3rp455w0rd') is no longer used in a generator function.

@nesukun the workaround would be to compute the hash outside the t.throws():

const targetHash = await hash('4n0th3rp455w0rd');
t.throws(checkAgainst('4p455w0rd', targetHash));

@jamestalmage this is a gnarly bug. I suppose the workaround would be to hoist any await statements out of the wrapper but that might be rather difficult.

@novemberborn novemberborn added bug current functionality does not work as desired and removed question labels Sep 23, 2016
@novemberborn
Copy link
Member

@nesukun another potential workaround:

const p = checkAgainst('4p455w0rd', await hash('4n0th3rp455w0rd'));
t.throws(p);

@novemberborn novemberborn changed the title Failing test after upgrading to 0.15.x t.throws() causes Node syntax error if first argument is function call with await arguments Sep 23, 2016
@nesheroj
Copy link
Author

Thanks for the investigation and response. I've stumbled with this issue in a few different proyects. Will surely follow your workaround recommendation until a more suitable solution is found. Have a great day!

@novemberborn novemberborn self-assigned this Oct 18, 2016
novemberborn added a commit to avajs/babel-plugin-throws-helper that referenced this issue Oct 18, 2016
The helper wraps the original argument with a new function. This is a
problem if the argument contains yield or await expressions, since the
function they now run is is not a generator or marked as asynchronous.

Instead hoist these expressions so they're resolved outside of the
helper.

Fixes avajs/ava#987.
@novemberborn novemberborn removed their assignment Jan 17, 2017
@novemberborn
Copy link
Member

I've tried to improve our throws helper but got stuck on another edge case. If anybody's interested in taking this on, the PR is at avajs/babel-plugin-throws-helper#6.

novemberborn added a commit to avajs/babel-plugin-throws-helper that referenced this issue Jan 17, 2017
The helper wraps the original argument with a new function. This is a
problem if the argument contains yield or await expressions, since the
function they now run is is not a generator or marked as asynchronous.

Instead hoist these expressions so they're resolved outside of the
helper.

Fixes avajs/ava#987.
@novemberborn
Copy link
Member

Fixed in d924045.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug current functionality does not work as desired help wanted
Projects
None yet
Development

No branches or pull requests

4 participants