Breaking: t.end requires test.async, and t.plan will not auto end tests. Fixes #244.

[jamestalmage]

See #244

[jamestalmage]

Overall, I like it.

There are a few places where it creates some weirdness. All revolve around our async assertions (t.throws(rejectedPromise), t.throws(rejectedObservable)).

test('this does not work', t => {
  var promise = Promise.reject(new Error('foo'));

  // this assertion actually happens asynchronously
  t.throws(promise);
  // we do not return a promise, so AVA assumes this is a synchronous function
});

There are a number of easy ways around this this. The issue will be preventing user confusion.

(Ordered from best to worst workaround IMHO)

test('use async/await', async t => {
  var promise = Promise.reject(new Error('foo'));

  await t.throws(promise); 
});

test('return the assertion promise', t => {
  var promise = Promise.reject(new Error('foo'));

  // t.throws returns a promise, so return it from our test function.
  return t.throws(promise);
});

test.async('legacy async with a plan', t => {
  t.plan(1);
  var promise = Promise.reject(new Error('foo'));

  // test is auto-ended once this resolves and we reach our plan count.
  t.throws(promise);
});

I like this last one the least. As previously stated, I think allowing t.plan() to auto-end tests is a bad choice that causes confusing false positives. Since we are biting the bullet with a major breaking change here, I would love to see us correct that as well.

[sindresorhus]

(Ordered from best to worst workaround IMHO)

#1 is how I already use it and how it was intended. We should just document it better.

[sindresorhus]

I think we have a resolution in #244, unless there's anything else that's still undecided? Could you rebase this PR?

[jamestalmage]

This is ready for review.

// @sindresorhus @vdemedes

I have updated the docs to reflect the new behavior, with a few language tweaks along the way.

[jamestalmage]

The previous Observable example was so trivial that I don't feel it adequately explained the advantage AVA offered.
I think this:

1. Is a simple enough usage of observable that you don't necessarily need to be familiar with them to understand what is happening.
2. Shows a novel use of t.plan() in coordination with the Observable filter. It's a little closer to a test someone might actually right.

@BarryThePenguin
What I know about Observables I learned in the last 20 minutes, so feel free to tell me where I missed the mark.

[BarryThePenguin]

You could change map with .do(() => t.pass());. Otherwise, looks good.

RxJS has some docs on marble testing. But I'm not sure how they would fit with AvA

[jamestalmage]

Good call! Thanks!

[sindresorhus]

Why not just this here instead?

if (!this.metadata.async) {
    throw new Error('t.end is not supported in this context. To use t.end as a callback, you must explicitly declare the test asynchronous via `test.async(testName, fn)` ');
}

[jamestalmage]

You mean just have t.end throw when executed?

Because we want to fail fast, and if passed as a callback:

   fs.readFile(path, t.end);

We are waiting for the function to actually be called instead of throwing ASAP.

[sindresorhus]

Does it matter? It shouldn't be used with test() anyways. So I don't see how it makes a difference.

[jamestalmage]

If they don't return a promise - we assume sync. We might mark the test done before the callback returns.

[sindresorhus]

Ok, never mind. Got it now. Sorry about the noise.

[sindresorhus]

Looks good to me, except for a few inline comments :)

@vdemedes ?

[vadimdemedes]

✅from me.

[jamestalmage]

Today is a major holiday in the US, so I won't be wrapping this up untill tomorrow.

[sindresorhus]

@jamestalmage No worries at all. Enjoy Thanksgiving :D

[jamestalmage]

Ahhh, Windows!

@sindresorhus - I'll start looking into it here, but can you just retrigger the build? I was watching that one, and it was running REALLY slow. I think something was wrong on that VM.

[sindresorhus]

Done

[jamestalmage]

OK, CI passed.

Are we good here? I think all concerns are addressed.

[sindresorhus]

👍 Excellent. I'm super excited about this!