t.doesNotThrow() expected parameters seems to be ignored

[m-r-r]

Hello,

I read the code of the method t.doesNotThrow() and I don't understand the purpose of the expected parameter.

The documentation says :

expected, if present, limits what should not be thrown, and must be a RegExp or Function. The RegExp matches the string representation of the exception, as generated by err.toString(). For example, if you set expected to /user/, the test will fail only if the string representation of the exception contains the word user. Any other exception will result in a passed test. The Function is the exception thrown (e.g. Error).

So, if my understanding is correct, the following tests should not fail :

const test = require("tape");

test('should not fail', (t) => {
    t.doesNotThrow(
        () => { throw new Error("Foo bar") },
        TypeError,
        "the callback does not throw a TypeError"
    );
    t.end();
});

test('should not fail', (t) => {
    t.doesNotThrow(
        () => { throw new Error("Foo bar") },
        /baz/,
        "the callback does not throw 'baz'"
    );
    t.end();
});

But the current behavior of t.doesNotThrow() is to always fail if the callback throws an exception.

Should the method t.doesNotThrow() be updated to match the documentation, or is my understanding of the documentation incorrect ?

[ljharb]

node's assert seems to work the same way:

assert.doesNotThrow(() => { throw new Error("Foo bar") }, TypeError); // throws
assert.doesNotThrow(() => { throw new Error("Foo bar") }, /baz/); // thros

node's docs seem to reinforce this.

Thus, i think tape's docs may need to be updated to clarify how it works?

[marvin-martian]

Hi! Sorry this doesn't make sense to me. I am just trying to use doesNotThrow. The example tests supplied by @m-r-r should be the logical behavior. Just NOT like node does it. It is not a documentation problem it seems like a bug.

Sorry for all the edits.. I should read what I type.

Nodejs is asserting a condition, not testing a condition. There is a difference between assertion and test. A test should be, IMHO, a strict binary state. true/false. Not some other state.

[marvin-martian]

...and as the node docs state :

Using assert.doesNotThrow() is actually not useful because there is no benefit in catching an error and then rethrowing it. Instead, consider adding a comment next to the specific code path that should not throw and keep error messages as expressive as possible.

So making it behave like Nodejs assert.doesNotThrow is not particularly useful for testing purposes. IMHO tape should remove t.doesNotThrow or make it a proper useful test.

[ljharb]

A test and an assertion are identical.

If node changes doesNotThrow, we will too. Make the case to them.

[marvin-martian]

A test and an assertion are identical.

@ljharb That is an assertion. Grab a dictionary and test it. :-)

TAP is "Test anywhere protocol" not AAP -> "Assert anywhere protocol".

TAP is for testing not asserting.

The difference between test and assert is subtle and it seems like the same, I understand your assumption, but I assure you it is not.

Politicians assert. Scientists test.

Test is absolute, assert not necessarily so. This is why it is ok for Nodejs to throw an error in assert.doesNotThrow(). They are "testing" assertion, nothing else. It is not a problem with Nodejs, nor an issue for them, it is a problem with tape. Otherwise Nodejs would call their "assert" library "test".

Tape is a test suite not an assert suite. I make the case to you because it is appropriate, there is no need for node to rename their library nor change the functionality of assert.doesNotThrow().

But in a test suite that should not be the behavior, the tape documentation as it stands is correct, the function is buggy.

[marvin-martian]

For a little clarity:

An assertion is a claim of fact. A test verifies the claim.

See the difference?

[ljharb]

Sure, and t.doesNotThrow is an assertion, that’s part of its overarching test. Either way, the semantics game you’re playing doesn't impact this issue.

This library’s test assertions largely match node’s assert - that has always been one of tape’s most important rules, and we’re not going to deviate here.

[marvin-martian]

Semantics is not a game, it helps us to determine the differences in meaning. A test suite that asserts instead of tests is not useful.

For myself it's ok. If you don't want to change I can fork it for myself, fix the issue and continue on.

[ljharb]

@marvin-martian there's nothing to fix. you're welcome to make a broken fork all you like.

Every test suite asserts, to accomplish testing. There does not exist a testing tool that lacks assertions as the means to do it (expect, should, assert are all forms of assertions).

[marvin-martian]

Not really.. the developers assert that it accomplishes testing. Whether it tests or or not is a matter of test.

[marvin-martian]

Please don't get me wrong.. you are right, assertions are used in tests.

...however it should not necessarily be the final conclusion of the test. An assertion doesn't examine itself. It is (then it asserts) or it carries on.. this is why node's assert.doesNotThrow throws the error if the assertion is not expected. It just continues.

This is why assertions throw AssertionErrors, and you need to put them in a try... ..catch to capture them, if they were tests they would return absolutes.

An assertion does not have an absolute. It does not return true/false.

However a test suit is trying to test, not assert, so it needs to determine an absolute, or it is not testing.

But easy. If the rule for the project is that you make Tape for assertion.. ..so be it. I am not trying to force anybody to do anything.. ..nor can I.

It is something I can work easily around for myself.

[ljharb]

Assertions don't have to throw - that's just how "assert" APIs work. "expect" and "should" APIs don't necessarily have to end the test when they fail, and tape's assertions never end the test when they fail.

[marvin-martian]

Yes you are right the assertions don't have to throw, in fact they do not react at all. And because of that they are not determining an absolute. It is only when you evaluate it's behavior that you can determine an absolute result.

The evaluation of an assertion is the test.

Tape should never end an entire test until all tests have been answered for, this has nothing to do with assertion, but only for completion of tests.

I know the semantics may be difficult to comprehend, it was difficult for me at first.

Just to apply the idea elsewhere in a different context to help grasp the concept: Politicians assert their claims.. it is up to the public to evaluate the politicians assertions to make an absolute decision. The public is testing the politicians assertions. If the public would just mimic the politicians claims, they are not evaluating the politician.

Tape should test node's assertions, not mimic them.

[marvin-martian]

A proper test for a function doesNotThrow a particular error would reflect some thing like this.

t.assert((
let pass = true,
     expected = /^Expected error$/u;
 try { 
 myTestFn();
} catch(e) {
 pass = !expected.test(e.toString());
}
return pass;
)(),
'I am not throwing "Expected error"'
);