prevent nested tests #948

[asafigan]

This is my first pull request so any feedback is welcome.

fixes #948

Added a test in the runner test file which expects an error to be thrown when test is called in a test. I don't know if this is the right file to put this test, but it seem like a good place since my solution only changed the runner.js file.

My solution was to add a new property to runner called 'hasBegunRunning' which is set to true in the run method. The chaining function checks this property and throws an error when it is true.

I don't know if this is a good solution but it is simple and it works. Also, I don't know if I need more tests for this pull request.

[jamestalmage]

This could also happen with:

test(async t => {
  // stuff
});

setImmediate(() => {
  test(t => {
    // not allowed!
  });
});

The message should include something about declaring tests async.

All tests and hooks must be declared synchronously in your test file, and cannot be nested within other tests.

[jamestalmage]

I don't know if I need more tests for this pull request.

Add a second one that tests what I talked about above, and I think you are covered.

// test
var r = new Runner();

r.test(() => Promise.resolve());
r.run()

t.throws(() => {
  r.test(() => {});
}, {message: ...})

This is my first pull request

One of the best first PR's I have seen. Very good job. 🎉

[asafigan]

also I didn't know what type of error to throw

[asafigan]

Ok, I will add that test and also tweak the error message and the second test

[asafigan]

Ok I'm gone with those changes. Let me know if there is anything else I need to change.

[sotojuan]

👍 great job!

[asafigan]

thanks!

[jamestalmage]

LGTM

[sindresorhus]

I would use isRunning instead.

[asafigan]

I didn't name it isRunning because it isn't reset to false after the tests are done running.

[asafigan]

but I don't like hasBegunRunning either

[sindresorhus]

hasStarted?

[asafigan]

I like that better

[sindresorhus]

Let's go with that then.

[asafigan]

Ok, I changed the property name.

[asafigan]

Also should we mention in the docs somewhere that you shouldn't have embedded tests or hooks. And also that we have no alternative to describe.

[jamestalmage]

Thanks @asafigan!

Also should we mention in the docs somewhere that you shouldn't have embedded tests or hooks. And also that we have no alternative to describe.

That probably belongs in a "transitioning from mocha/tape" recipe. Not sure it belongs in the core docs.

[asafigan]

woot! first contribution! 🎉
Thanks for all of the help on this

[sotojuan]

Awesome work @asafigan!

[sindresorhus]

Super first PR. Thanks @asafigan! Keep 'em coming ;)