Tests: remove t.plan in favour of t.end

[michaelfig]

This repo should not use t.plan(NNN). It is a burden to maintain and doesn't gain anything over t.end().

For robustness (allowing subsequent test cases to run after one fails with an exception), the preferred pattern for test cases is:

test('description', t => {
  try {
    // [Test assertions go here.]
  } catch (e) {
    // [Fail the test case if there is an unexpected exception.]
    t.assert(false, e);
  } finally {
    // [Mark this test case as finished.]
    t.end();
  }
});

[erights]

Won't test itself deal with the thrown error? If so, I'd prefer to omit the try/catch

[michaelfig]

No, it doesn't deal with thrown errors. It just aborts the whole test suite with a stack trace.

[michaelfig]

I'm asking for advice from the tape-promise authors in jprichardson/tape-promise#19

[warner]

When trying to get a test to pass for the first time, I think I might actually prefer if the whole test suite aborts. If it doesn't do that, I find it difficult to scroll back through all the subsequent tests to find the one that failed (piping everything into a pager doesn't usually work for me, because some stuff goes to stdout and some to stderr, and then I also lose all the coloring that tape and/or our frontends are providing). Also, tape doesn't print the filename of the tests it is running, so it's often a slow process to deduce which test is failing without a stack trace to work from.

(of course none of this is relevant to removing t.plan, that's obviously the right thing to do regardless)

[michaelfig]

The pattern I've been using that reports exceptions properly is to make the new test as:

// Just run this test until you're ready to remove `.only` and run the whole suite.
test.only('description', t => {
  try {
    // [Test assertions go here.]
  } catch (e) {
    // [Fail the test case and write the stack trace.]
    t.isNot(e, e, 'unexpected exception');
  } finally {
    // [Mark this test case as finished.]
    t.end();
  }
});