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

Nested test functions hide errors #948

Closed
tp opened this issue Jul 1, 2016 · 9 comments · Fixed by #961
Closed

Nested test functions hide errors #948

tp opened this issue Jul 1, 2016 · 9 comments · Fixed by #961

Comments

@tp
Copy link

tp commented Jul 1, 2016

Description

I was just reviewing a PR where a colleague wrote tests in a nested way.

While the sub-tests never execute / show up in the test output they also did not create an error.

I would expect such code as the example below to at least throw an error.

Test Source

import test from 'ava';

test('outer', (t) => {

  test('innter', (t) => {
    t.fail();
    throw new Error('ERR');
  });

});

Console Output

=> ./node_modules/.bin/ava -v avaTest.js

  ✔ outer

  1 test passed [16:35:53]

Config

No config in package.json

Command-Line Arguments

/ava -v avaTest.js

Relevant Links

  • If your project is public, link to the repo so we can investigate directly.
  • BONUS POINTS: Create a minimal reproduction and upload it to GitHub. This will get you the fastest support.

Environment

Tell us which operating system you are using, as well as which versions of Node.js, npm, and AVA. Run the following to get it quickly:

Node.js v5.9.0
darwin 15.5.0
ava 0.15.2
npm 3.8.6
@novemberborn
Copy link
Member

Yea, seems like the test runner should fail if a test is declared after it's started running tests (or a beforeEach hook etc).

@GramParallelo
Copy link

I'm looking to contribute my first PR. Can I have a go at this? (might need a little helping hand :-/ ) Thanks

@sindresorhus
Copy link
Member

@GramParallelo Yes, go ahead. We're happy to handhold you through it :)

Jump into https://gitter.im/avajs/ava if you have any questions.

@pinwen
Copy link

pinwen commented Jul 4, 2016

Can I start looking into this too?

@GramParallelo
Copy link

GramParallelo commented Jul 6, 2016

@pinwen Yes, please. I'm a little lost and also can't put in the time at the moment. Thanks.

@asafigan
Copy link
Contributor

Hey, I've never contributed to an open-source project before. It doesn't look like there has been that much progress on this issue. I was hoping I could help out with this issue?

@asafigan
Copy link
Contributor

I was thinking that a property, like hasBegunRunning, could be set to true in the runner's run method and than checked when the test method was was called. Would that be a good approach?

@asafigan
Copy link
Contributor

@pinwen Are you still working on this issue?

@jfmengels
Copy link
Contributor

Actually very surprised we didn't think of this for the ESLint plugin earlier. Created an issue over there avajs/eslint-plugin-ava#131

jamestalmage pushed a commit that referenced this issue Jul 14, 2016
…`test` (#961)

Fixes #948.

AVA does not support nested / async calls to `test`, but we were not providing a great error message when users attempted that.

* added failing test for expected error

* tests passed: waiting for review

* added test

* changed error message

* fixed error in test

* changed 'hasBegunRunning' to 'hasStarted'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants