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

Dealing with failure: assertions/terminate #220

Closed
gnzlbg opened this issue Nov 20, 2013 · 17 comments
Closed

Dealing with failure: assertions/terminate #220

gnzlbg opened this issue Nov 20, 2013 · 17 comments

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 20, 2013

Right now there aren't any documented ways of checking that my program fails correctly when no exceptions are involved. In particular, calls to std::terminate and to assert() (in debug mode only, i.e. when NDEBUG is not defined).

In google-test one can do: EXPECT_DEATH/EXPECT_DEBUG_DEATH to test for failure in general and failure in debug mode only as well as to test that the correct failure message is printed (although for asserts I always write a regexp that catches everything).

Note: a workaround is to define your ASSERT macro to throw an exception and hope that no-one catches it. But this is just a workaround.

@mgeier
Copy link

mgeier commented Nov 22, 2013

++

@philsquared
Copy link
Collaborator

@gnzlbg, this seems to be the same issue as #219, slightly expanded on.

I agree that this would be useful functionality. I think there was another mention of this recently somewhere else too (was it you again, @gnzlbg). On that occasion google test was mentioned as being able to handle ASSERTs and I was wondering how they did that without going out of process (I had some ideas) - but now that you mention terminate too I thought I should check.
Sure enough they go out of process!
I've been planning to add an out-of-proc runner for a while as this has a few nice properties (Catching all sorts of crashes and termination types, but also for when you want to be really sure the tests are run in isolation).

It's not trivial to do this portably, but is not rocket science either. I'll up the priority of this a bit but can't promise it imminently.

Thanks for raising this (and @mgeier for post (or was it pre)-incrementing!)

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 26, 2013

@philsquared looking at google-test's implementation it seems that they spawn a child process for each test, log the test output, and catch the exit code. They test that the exit code is correct (e.g. 0 for EXPECT_EXIT, non-zero for EXPECT_DEATH) and that a given regular expression finds a match in the test output.

A portable implementation (across Linux, Mac, and Windows) is a lot of work indeed. Is using Process (it is not part of Boost) an option?

@philsquared
Copy link
Collaborator

I don't know if I'd say a lot - but it's not a five minute feature. I was already tinkering with it (got a basic popen implementation for POSIX platforms) and I've written the Windows versions before (for employers).

I can't use Boost due to my "no external dependencies" policy - which is right at the heart of Catch's identity.
However there's no reason that a Catch implementation can't be influenced by Boost (and GTest) as references.

@acidtonic
Copy link

I can also help test the unix implementation of this... preferably without bringing boost into the picture at all. My projects are pure C++11 these days and catch fits that discipline perfectly. If you do head the boost direction I'd be willing to put effort forth to help provide a pure C++ runner as an alternative for others like myself. Loving Catch by the way! Keep up the good work.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Dec 3, 2013

I just want to mention again that Process is proposed for inclusion into Boost but hasn't been accepted yet (and it seems abandoned). Second: as everything boost licensed, you can just pick up the headers you need and add them to your project. That is, you don't need to add an external dependency if you don't want to. It usually make sense to add an external dependency to get bugfixes but since Process seems abandoned I don't see any drawbacks in just getting the functionality we need (a portable implementation for managing processes) and dumping out the rest. Process went through 2-3 rounds of peer-review so if you want to implement your own C++ runner it makes sense to look at its implementation anyway.

@philsquared
Copy link
Collaborator

That's a good point, @gnzlbg. I do sometimes fully extract code (slightly tweaked) from boost - or least use it is a reference - and I would probably do that.

Thanks too, @acidtonic, for the offer of testing - that would be very useful. Although I develop on a POSIX based system (Mac OS X) I'm often getting caught out by subtle differences.

@zdavisk
Copy link

zdavisk commented Dec 5, 2013

On a side note, is there any plans for a direct "FAILURE" macro that doesn't require an expression?

It's probably me but I can't seem to get a check for a nullptr to work inside a catch REQUIRE() statement. So then I end up checking the null pointer in an "IF" statement that has a line inside that says REQUIRE(1 == 0) or something similar.

Would be great if we had a direct failure macro. :)

@philsquared
Copy link
Collaborator

@zdavisk to immediately answer your question the FAIL() macro does exactly what you're asking for.

However that's addressing the symptom. You really should be able to directly check for nullptr within a REQUIRE(). What happens when you try? What compiler and version are you using (and on what platform)?

@philsquared
Copy link
Collaborator

oh... and perhaps you could raise this as a new issue?

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jul 27, 2015

Catch came up on reddit today and I mentioned the lack of this feature as basically the only thing that I missed from GTest (not that I went back to GTest just because of it tho). I know it is complicated to achieve this in a portable way, I just wanted to ask if there has been some progress on this.

@martinmoene
Copy link
Collaborator

@horenmar
Copy link
Member

horenmar commented Jan 29, 2017

This is copy pasted from #219

Since we have recently merged a signal rework, I decided to test this issue.

Given this source code against single-include catch.hpp generated from current master,

#define CATCH_CONFIG_MAIN
#include "catch.hpp"

#include <cassert>
#include <exception>

TEST_CASE("TEST-terminate", "[.term]"){
    SECTION("First") {
        std::terminate();
    }
}

TEST_CASE("TEST-assert", "[.ass]") {
    SECTION("First"){
        assert(false && "Hello");
    }
}

ran like this ./a.out -s -r compact [.ass], I get this output

a.out: test.cpp:15: void ____C_A_T_C_H____T_E_S_T____3(): Assertion `false && "Hello"' failed.
test.cpp:14: failed: fatal error condition with message: 'SIGABRT - Abort (abnormal termination) signal'
Failed 1 test case, failed 1 assertion.

Aborted (core dumped)

which shows failure at line 14, which is the line of "First" section macro.

If I run the code like this ./a.out -s -r compact [.term], I get this:

terminate called without an active exception
test.cpp:8: failed: fatal error condition with message: 'SIGABRT - Abort (abnormal termination) signal'
Failed 1 test case, failed 1 assertion.

Aborted (core dumped)

which also shows the failure at the correct section.

Seeing this, I am inclined to close this issue.

@horenmar horenmar added the Resolved - pending review Issue waiting for feedback from the original author label Jan 29, 2017
@philsquared
Copy link
Collaborator

Unlike #219, this issue is about testing for a fatal error - then continuing!
AFAICS that can only be achieved by going out of process (as the earlier parts of the thread discussed). So I think we need to leave this open (or close in favour of a single issue on the matter).

@horenmar
Copy link
Member

Oh right. It seemed weird that the same person would open two issues about the same thing, but not weirder than other things I've seen in the issue backlog.

@horenmar horenmar added Feature Request and removed Resolved - pending review Issue waiting for feedback from the original author labels Jan 30, 2017
@kgreenek
Copy link

Major +1 for this feature. This is the one thing I miss from GTest. It's really important to be able to test that asserts happen when they should imo. Right now I have to use BOOST_ASSERT (or similar macro) as a workaround.

@horenmar
Copy link
Member

Since this will not be added to Catch Classic, but might be added to Catch 2 at some point, I opened #853 instead.

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

No branches or pull requests

8 participants