kotest: shouldNotThrow and shouldNotThrowAny matchers

Having these matchers would be great -shouldNotThrow -shouldNotThrowAny

Currently there is not way of expressing that easily with kotlintest matchers

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Reactions: 3
  • Comments: 30 (24 by maintainers)

Commits related to this issue

Most upvoted comments

@TAGC Would you mind creating an issue for that? I agree with you that syntax would be prettier and human-readable.

Not sure what you mean. Why create a new issue for the same proposal as this one?

What’s the difference except more noise ?

In short test-cases, it probably does create more noise than it’s worth. For example, in my case instead of:

it("can be connected to other ports of a supertype") {
    val derivedPort = createDummy<Derived>()
    val basePort = createDummy<Base>()
    derivedPort join basePort
}

I might write:

it("can be connected to other ports of a supertype") {
    val derivedPort = createDummy<Derived>()
    val basePort = createDummy<Base>()
    shouldNotThrow { derivedPort join basePort }
}

And it wouldn’t make much difference.

However, in longer test-cases the advantage is that it helps distinguish the assertion from the rest of the setup/cleanup code.

it("verifies something") {
    val x = Foo()
    x.bar()
    x.baz()
    // More setup for x
    
    val y = Foo2()
    y.bar(x)
    y.baz(x)
    // More setup for y
    
    y.baz(z)
    
    cleanup(x)
    cleanup(y)
    // More cleanup
}

vs.

it("verifies something") {
    val x = Foo()
    x.bar()
    x.baz()
    // More setup for x

    val y = Foo2()
    y.bar(x)
    y.baz(x)
    // More setup for y

    shouldNotThrow { y.baz(z) }

    cleanup(x)
    cleanup(y)
    // More cleanup
}

Without this assertion, I might be inclined to write y.baz(z) // should not throw, but that’s not as good for the reason why comments in general are not preferable to readable code: because comments can become stale, they can lie, but code never lies.

Furthermore, if you just write the assertion as a plain statement and it fails, all you get is a regular stacktrace associated with the unit test, indistinguishable in structure from the stacktraces you’d get if any other part of the test (setup/cleanup) failed. A failed shouldNotThrow assertion can instead capture exceptions and rethrow them with a message like “Expected no exception to be thrown, but <wrapped exception> was thrown”.

In the same way that the shouldNotThrow statement helps visually distinguish the assertion code from the rest of test code (making the test more readable), the exception it throws can help distinguish an assertion failure from any other failure within the code (making the problem easier to understand and the bug easier to fix).

It’s an especially nice assertion to have when you have some function that should throw exceptions under certain circumstances and shouldn’t throw in others, and you want test both types of scenarios. That leads to nice symmetrical code:

public void Message_Decoder_Should_Decode_Additional_Messages_After_Decoding_Complete_And_After_Reset()
{
    // GIVEN a mock decoder that is able to decode messages and has completed decoding.
    var mockDecoder = new MockMessageDecoder
    {
        DecodingMessagesAllowed = true
    };
    mockDecoder.SetDecodingComplete();

    // WHEN we reset the mock decoder.
    mockDecoder.Reset();

    // THEN we should be able to decode more messages.
    Should.NotThrow(() => mockDecoder.DecodeMessage(default(GenericMessage)));
}

[Fact]
public void Message_Decoder_Should_Decode_No_Additional_Messages_After_Decoding_Complete_And_Before_Reset()
{
    // GIVEN a mock decoder that can decode messages.
    var mockDecoder = new MockMessageDecoder
    {
        DecodingMessagesAllowed = true
    };

    // WHEN decoding is complete.
    mockDecoder.SetDecodingComplete();

    // THEN an exception should be thrown when trying to decode another message.
    Should.Throw<InvalidOperationException>(() => mockDecoder.DecodeMessage(default(GenericMessage)));
}

@sksamuel Sometimes users want to write things in a more explicitly.

It’s the same reason we added the matchers shouldNotBeTrue and shouldNotBeFalse https://github.com/kotlintest/kotlintest/issues/452

Of course (3 ==3).shouldBeTrue is exactly the same as (3==3).shouldNotBeFalse, but sometimes one might want to say in a way or another.

Adding a shouldNotThrow empty block doesn’t look like a problem to me, and it won’t make users more confused

2 reasons that I can think of now:

  • I think tests are more readable if you explicitly write shouldNotThrowAny
  • assertion error can more precisely tell you why your test failed: “you expected no exception, but we found: SomethingException