Prevent incorrect use of t.throws()

[sindresorhus]

I've seen a lot of incorrect use of the throws assertion in other test runner and even done the mistake myself sometimes. Now I'm beginning to see it with AVA too, so would be nice to be preemptive about it.

People don't realize they need to wrap their call in a function, so many end up doing t.throws(foo()) instead of t.throws(() => foo());. It's an easy mistake to make.

Happened yesterday: avajs/ava#739

We should disallow doing t.throws(foo()) and t.notThrows(foo()).

I think we can easily add auto-fixing to this by just wrapping the call in an arrow function like so t.throws(() => foo());.

[jamestalmage]

Unfortunately this won't work.

foo() might return a promise, in which case, t.throws(foo()) is perfectly acceptable.

[jamestalmage]

We could improve things with a custom babel plugin though:

  t.throws(foo());

becomes:

t.throws(throwsHelper(
  function () {
    return foo(a, b);
  }, {
    line: 3,
    column: 3,
    source: 'foo(a, b)'
  }
);

function throwsHelper(fn, data) {
  try {
     return fn();
  } catch (e) {
     throw new IncorrectThrowsError(e, data);
  }
}

Basically, we take the expression and wrap it in a function. Pass that function to the helper. The helper calls the wrapper function, returning the result. If it synchronously throws an Error - we print a good error message (including the source). Suggesting they wrap it correctly.

[jfmengels]

That wouldn't work for nested functions, would it?

function foo(value) {
  return function() {
    // ...
  };
}

t.throws(foo(2)); // valid and proper use
t.throws(foo); // valid but not the expected use

You could force the user to give a non-call expression to throws

const fn = foo(2);
t.throws(fn);

But that sounds error-prone too and feels like just moving the problem somewhere else.

[twada]

For Promises, how about composing small helper function such as assert-exception with t.truthy

• t.truthy(rejects(foo()))

Or introduce t.rejects ?

• t.rejects(foo())

Then disallow doing t.throws(foo()) and t.notThrows(foo()).

[jamestalmage]

That wouldn't work for nested functions, would it?

Yeah - it always would.

Take your examples.

t.throws(foo(2)); // valid and proper use

becomes:

t.throws(helper(function () {
  return foo(2);
}));

---

t.throws(foo); // valid but not the expected use

becomes:

t.throws(helper(function () {
  return foo;
}));

---

We are always just returning the evaluation of the expression. (i.e. we make no change to the value the assertion sees).

The only thing we protect against is a synchronous throw (and by protect, all I mean is we fail with a graceful error message that explains what they did wront).

[jfmengels]

@jamestalmage Ah ok, you're just wrapping the call in an extra layer of try-catch 👍

[jamestalmage]

@jamestalmage Ah ok, you're just wrapping the call in an extra layer of try-catch 👍

Yes. I decided to say that in the most verbose way possible!

[jfmengels]

And you did well, now I got it ;)

[sindresorhus]

@jamestalmage Using a Babel helper plugin is a really cool idea! Wonder if we can prevent/improve upon any other usage with Babel helpers.

For posterity:

• detect improper use of t.throws ava#742
• https://github.com/jamestalmage/babel-plugin-ava-throws-helper

[jfmengels]

Should we close this issue? I have the impression that we can't do much statically.

[sindresorhus]

Yup