Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What is the proposed way to test a try block? #671

Closed
dpservis opened this issue Jun 8, 2016 · 10 comments
Closed

What is the proposed way to test a try block? #671

dpservis opened this issue Jun 8, 2016 · 10 comments

Comments

@dpservis
Copy link

dpservis commented Jun 8, 2016

REQUIRE_NOTHROW is a great way to work with exceptions but what if I want to test a larger block where there may be several calls that throw, e.g.

try
{
// Make a lot of things here
}
catch(...)
{
FAIL("an exception was thrown");
}

@Darelbi
Copy link

Darelbi commented Jun 8, 2016

bool exception = false;
try{
    //...
}catch(...){
    exception = true;
}
REQUIRE(exception == false);

@milleniumbug
Copy link

I use

REQUIRE_NOTHROW([&]()
{
    // your code
}());

@lightmare
Copy link
Contributor

REQUIRE_NOTHROW is a great way to work with exceptions but what if I want to test a larger block where there may be several calls that throw

Why would you want to hide which of those calls threw? I mean, wouldn't it be better to REQUIRE_NOTHROW each of them, so that when something fails, you know where the trouble is, instead of just "somewhere in this block"?

@milleniumbug
Copy link

milleniumbug commented Jun 15, 2016

@lightmare What if you want to test declarations? They aren't expressions.

IMO you can get enough context from the name of the exception. If you can't, then your test is too big :D

@lightmare
Copy link
Contributor

@milleniumbug How about SECTION?

@dpservis
Copy link
Author

Hi all

thanks for the answers, my main motivation for this are throwing constructors of classes without default constructors and then using these to make other objects/calls etc.

@dpservis
Copy link
Author

@milleniumbug good idea if using C++11

@lightmare
Copy link
Contributor

lightmare commented Jun 22, 2016

@milleniumbug You need to wrap the anonymous function in parentheses, otherwise it will suffer the same problem with commas as a plain block:

REQUIRE_NOTHROW(
{
    // if there is a comma outside parentheses, it won't even pass preprocessing
    std::pair<int, int> nono;
});

REQUIRE_NOTHROW(([&]
{
    std::pair<int, int> ok;
})());

@dpservis You can do that with sections:

SECTION("foo and bar with some input")
{
    ThrowingFoo foo("input");
    ThrowingBar bar(foo, "more input");
    REQUIRE(bar.isValid());
    REQUIRE_NOTHROW(bar.doStuff());
    // etc.
}

If a constructor throws, it terminates the section and you get exactly as much information as you'd get from REQUIRE_NOTHROW with a block, i.e. only line number of the start of the section/block and exception.what();

@dpservis
Copy link
Author

@milleniumbug great!

@philsquared
Copy link
Collaborator

Although the lambda suggestion works (assuming C++11 on) I don't think it adds much value in this case.
Any uncaught exceptions that escape the test case are reported as failures anyway. The advantage of using REQUIRE_NOTHROW explicitly is that (a) you better document your intent that this really should be a non-throwing operation and (b) you get file/ line information that pinpoints the responsible call. Once you use a multiline lambda with many potentially throwing sites you start to lose those advantages (increasingly so the larger the block).

So I would recommend either use REQUIRE_NOTHROW on each line that you want to be specific about - or just relax and let Catch, well,... catch, your exceptions outside the test case.

Either way I'm going to close this issue now as I think it's been resolved in one of several ways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants