t.throws() expected and msg parameters don't seem to behave as they are described.

[cagross]

Hello. I am a little confused by the documentation for t.throws(), as it seems to be discrepant in two ways with the behavior I experience:

1. The documentation says that the expected parameter, if present, must be a RegExp or Function. But when I pass in a string, and execute a test, the test seems to complete without issue (see below for code). Furthermore, the string passed as the expected parameter is printed to the console, seemingly as the test description. Nor does the expected string match the string representation of the exception (as described in the documentation). Here is a screenshot of a passing test--something similar occurs for a failing test. Why is that? What am I misunderstanding?

2. The documentation mentions that the third parameter--msg--is an optional description of the assertion. So when/where exactly is this string used? I’ve executed two tests—one passing and one failing (see below for code). In neither case is the msg parameter printed to the console (see pass screenshot here; see fail screenshot here).

---

My passing test. To change it to a failing test, comment out the throw line.

  t.throws(
    () => {
      throw 'Error message.'// Comment this line to force a failing test.
    },
    'Message 1',
    'Message 2'
  )

Thanks in advance.

[ljharb]

Looking at the code, if expected is a string, and the error's .message property is equal to it, it fails with The "error/message" argument is ambiguous.. However, it shouldn't be possible for expected to be a string, because when it is a string, expected becomes undefined and msg becomes whatever the second argument is. In other words, in your example, Message 1 becomes the message, Message 2 becomes extra which as a string is unused - which means your assertion will pass as long as it throws anything, and fail as long as it doesn't.

It seems like it might be helpful here to fail if extra is a string, since that can't possibly do anything valuable, but I'm concerned that'd be a breaking change.

Additionally, the docs need updating; expected can be a regex or a function or a "validation object", just like node's assert.throws.

[cagross]

OK thanks for all that--it is helpful.

However, it shouldn't be possible for expected to be a string, because when it is a string, expected becomes undefined and msg becomes whatever the second argument is. In other words, in your example, Message 1 becomes the message,

OK I understand now, and can confirm this behavior--thanks for that explanation. But is that typical behavior in JS? That behavior wasn't entirely obvious to me from reading the documentation. I guess I could have looked at the source code. Just my two cents, but adding a sentence to the throw() documentation to describe what happens when expected is a string might help noobs like me :-) Maybe I could compose that text and send it to you. Or even do make the edit and submit a pull request.

Message 2 becomes extra which as a string is unused

I don't understand--what exactly are you referencing with extra?

Additionally, the docs need updating; expected can be a regex or a function or a "validation object", just like node's assert.throws.

OK noted, thanks.

[ljharb]

It's typical behavior in a lot of older APIs that allow you to omit "not the last" parameter.

I'd be happy to review a PR that improves the docs.

extra contains options - like skip or todo (that can override the options from the containing test), or a message property that will be used instead of the message argument, things like that. Most tests never use it.

[cagross]

It's typical behavior in a lot of older APIs that allow you to omit "not the last" parameter.

OK got it--thanks for the explanation.

I'd be happy to review a PR that improves the docs.

OK noted. I would very much like edit the README instructions to indicate this behavior with throws(). But I really wouldn't want to do so without first completely understanding it. Namely, I'm still a little confused about extra. Earlier you said,

extra contains options - like skip or todo (that can override the options from the containing test), or a message property that will be used instead of the message argument, things like that. Most tests never use it.

So is that to say that extra is a fourth possible parameter when calling throws()--a parameter that needs to be an object? I'm happy to move this discussion to a new thread (so we can close this issue), or even discontinue this discussion if you don't have the time for it right now. I'm sure it's relatively low priority.

[ljharb]

Yes, that’s right. It’s one that is almost never used except by custom assertions, altho you might do something like this:

t.throws(fn, TypeError, msg, { skip: true });

[cagross]

OK thanks for that. Understood that extra is a fourth optional parameter that can be passed to t.throws(). I have to be honest, I still have a few questions about it. But how about for now I ignore that, and do this with the README. I'll edit the t.throws() section of the README to indicate only:

1. The expected parameter can be a regex, function. or a "validation object" (as you mentioned above).
2. The behavior of t.throws() if the expected parameter is not one of the above three values.

I'll omit any mention of extra for now. Then I can submit a pull request to you. At that time, if you want me to edit what I've written in the README, or add info about extra, we can discuss in the pull request, and I can add commits later if need be before you merge. Would that be OK?

[ljharb]

That sounds awesome, thank you!

[cagross]

OK sounds good. I may need a few days though. In the meantime, do you want to close this issue? You've answered my original issue. When I create the pull request, maybe I can reference this issue (and vice versa?).

[ljharb]

I’m fine leaving it open, and closing it via your PR.

[cagross]

OK cool. Sorry, needing a few more days on this--something came up at work.

[cagross]

Hi Jordan,

I'm back, and want to add that validation object example to the t.throws() section of the README. Before I do that, I'm trying to confirm that an actual test with a validation object behaves as expected. I've crafted a simple test with a validation object which passes (see below). But the thing is, it always passes, regardless of what's in the validation object. How can I get this test to fail? I was under the impression that, in this example, if the value of validationObject.code was 404, the test would pass; all other values would result in a failing test. I could very well be misunderstanding something though :-/

  const err = new TypeError('Wrong value')
  err.code = 404
  t.throws(
    () => {
      throw err
    },
    {
      code: 404 // Test passes regardless of value (e.g. 404, 405, null, etc).
    },
    'Test message.'
  )

FYI I borrowed this example from the Node documentation page on assert.throws().

[ljharb]

https://github.com/substack/tape/blob/master/lib/test.js#L617-L630 is the code that's supposed to be handling it. In other words, it should be returning false if the code values aren't deeply strictly equal.

Perhaps your PR could include test cases? If they fail, we've got a bug to fix :-)

[cagross]

OK thanks for the link to that code--it should be helpful here. Let me run my example test a few times while stepping through this code with a debugger, to see if I can narrow down the issue (if any). If I can do that, what do you want as the next step? How about I create a new issue here in GitHub, describe the problem I'm experiencing, then open a PR referencing the issue so we can discuss? Or would you rather me to skip the issue creation, and simply open a PR?

And for the time being, do you want me to perhaps remove the validation object information from the t.throws() README section, just in case it's a bit buggy? It might prevent some headaches on the part of users that try to put this into practice, only to find that it doesn't have the expected results.

[ljharb]

Up to you; no need for an issue first if you’re not doing anything controversial :-)

The unpublished readme shouldn’t confuse anyone; let’s get it fixed instead.

[cagross]

OK gotcha--no issue, just a PR. Let me try to do some debugging now.

[cagross]

https://github.com/substack/tape/blob/master/lib/test.js#L617-L630 is the code that's supposed to be handling it. In other words, it should be returning false if the code values aren't deeply strictly equal.

OK I had a better look and believe I see the issue--good old user error :-) In my testing, I was using Tape v4.13, and it appears that the code you highlighted was added after that version. Once I tested with the latest version of Tape, I can indeed confirm the expected behavior. Sorry for the confustion there!

Let me add the example to the README now and submit a PR. Stand by...