Googletest: EXPECT_THROW should allow testing of exception contents

[barreyra]

There should be a nice way for EXPECT_THROW to let me verify what's in my exception. Even testing the what() contents would already be helpful.

[Bu11etmagnet]

Not that easy, given the ability to throw any class or non-class type, e.g.

throw 42;
throw "up";
std::vector<std::string>{};

The only possibility I can think of is accepting some sort of user-specified callable object (lambda, std::function, ...) to check the exception.

[barreyra]

We can use the same approach that googlemock offers and use the matchers defined there, just as it's possible using EXPECT_THAT.

[PaulSolt]

I'm not sure how you'd specify all the conditions with the generic EXPECT_THROW(), a better approach might be to use the try/catch block where you can test for expected exceptions and any additional values or the what() message.

Try the following code with various exceptions that you can uncomment/comment in the throwException function:

class TestException : public std::runtime_error {
public:
  TestException(const std::string& message) : runtime_error(message) {

  }
  int errorCode;
};

void throwException() {

    // Test Invalid Exception properties
  TestException e("INVALID_SETTING");
  e.errorCode = 22;
  throw e;

    // Test Valid Exception properties
//  TestException e("VALID_SETTING");
//  e.errorCode = 20;
//  throw e;

    // Test std::runtime_error
//  throw std::runtime_error("runtime_error");

    // Test a non-standard value being thrown (C++11x)
//  throw 10;
}

Catch the exceptions using try/catch and as a last resort the catch(...) matcher for any other kind of unhandled exception.

TEST(ExceptionTest, ExpectThrowsSpecificException) {

  try {
    throwException();
    FAIL() << "throwException() should throw an error\n";
  } catch (TestException& exception) {
    EXPECT_THAT(std::string(exception.what()), Eq("VALID_SETTING"));
    EXPECT_THAT(exception.errorCode, Eq(20));

  } catch (std::runtime_error& exception) {

      // Catch something more general
    FAIL() << "Was expecting ExceptionTest: " << exception.what() << std::endl;
  } catch (...) {

      // std::current_exception is part of C++11x
    FAIL() << "ERROR: Unexpected exception thrown: " << std::current_exception << std::endl;
  }

}

[barreyra]

What about something like:

EXPECT.THROW(myfunc(),
    AllOf(
        A<TestException>(),
        Property(&std::exception::what(), "VALID_SETTING"),
        Field(&TestException::errorCode, 20)
    )
);

[sbroberg]

What about specifying a variable to be a copy of the caught exception, e.g.

ASSERT_THROW(myfunc(), ExpectedException, myCopy);
EXPECT_TRUE(myCopy.what(), "The message I expect");
...

The presence of myCopy would, of course, declare the "ExpectedException myCopy;" outside the scope of the try. Of course, ExpectedException would have to have default & copy constructors for this to work.

[gewesp]

How about a new macro EXPECT_THROW_WHAT(myFunc(), ExpectedException, ExpectedWhatString)?

Of course that would work only with exceptions derived from std::exception. But programmers not doing that should be fired without notice anyway.

[smcguffee]

I completely agree. This is bad of google if they didn't provide a way to do this. Maybe they did though? If so, what is it?

[xanather]

Just started playing with gtest. How is this not a thing?

[gennadiycivil]

It might be helpful if you explain in details what you are trying to do. As
it stands now it is hard to understand what your use case is.
Thanks

On Tue, Apr 16, 2019, 08:25 Xanather [e-mail] wrote:

Just started playing with gtest. How is this not a thing?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/google/googletest/issues/952#issuecomment-483637520,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AMJSMgwF4flP5frZWYITvdl8P9ZUu-sIks5vhcFPgaJpZM4LEZTF
.

[barreyra]

@gennadiycivil, what about my proposed syntax in my Mar 15, 2017 comment above? I think that makes the intent clear: we want to ensure that a given expression throws an exception that satisfies a given criteria.

[xanather]

@gennadiycivil not sure what you meant by that. There was 8 comments above mine stating whats needed. Any solution to add boolean logic by checking the exception contents.

Right now I need to start littering my tests with try catch{}'s when I could do something like this:

ASSERT_THROW(myfunc(), ExpectedException, myCopy);
EXPECT_TRUE(myCopy.what(), "The message I expect");

[xanather]

@gennadiycivil I see https://github.com/google/googletest/pull/1661 was closed which is unfortunate even though many people want it... and for good reason.

Same with https://github.com/google/googletest/pull/641

Considering that PR was 4 years ago and this issue has been up for 3 years I will just create my implement own macro and not waste any more time here commenting on such a trivial feature which the maintainers do not seem to care about.

As an example of the amount of lines that is caused by not supporting this:

        bool foundException = false;
        try
        {
            clientv6->connect(ip6Addresses[0], port);
        }
        catch (const tcp_exception & e)
        {
            ASSERT_EQ(e.m_reason, tcp_exception::reason::connect_refused);
            foundException = true;
        }
        ASSERT_TRUE(foundException);

If maintainers would like a PR implementing this:

ASSERT_THROW_OUT(myfunc(), ExpectedException, outException);
EXPECT_EQ(outException.what(), "The message I expect");

Which has a chance at getting merged let me know.

Regards

[gennadiycivil]

Apologies for a misleading comment, I missed the beginning of this thread (interesting UX bug because I was looking on a mobile device ).

To answer your question
1) Yes we would welcome a PR
2) We are actively discouraging any new macros so said PR should not include new macros

NOTE:
There is a general purpose exception testing work planned to implement this CppCon Talk

[Quuxplusone]

"We are actively discouraging any new macros" doesn't make sense in the context of GTest, as GTest is literally all macros. There is no non-macro interface to GTest; every new user-visible feature must include a new macro by definition.

Anyway, here's the macro I use in my codebase:

#define EXPECT_THROW_WITH_MESSAGE(stmt, etype, whatstring) EXPECT_THROW( \
        try { \
            stmt; \
        } catch (const etype& ex) { \
            EXPECT_EQ(std::string(ex.what()), whatstring); \
            throw; \
        } \
    , etype)

[aborisovich]

Well, this is very disappointing that Google rejects pull request with argument "We actively discourage new macros" (#1661). Especially when we talk about such "core" and elemental feature as exception message validation. How about you stop playing chess and "how abouts" and make this framework more user friendly first? And then do your "how abouts" and improvements?

[grantwwu]

@Quuxplusone I don't think that's actually true, given that there's now EXPECT_THAT( and the general https://github.com/google/googletest/blob/482ac6ee63429af2aa9c44f4e6427873fb68fb1f/googlemock/docs/cook_book.md#using-matchers-in-googletest-assertions interface.

@gennadiycivil Do you have any update on the work to implement that CppCon talk?

[asoffer]

@grantwwu given that Google does not use exceptions internally, implementing exception-safety testing is not particularly high priority. But I also don't think that would be useful here.

As a workaround, I think the macro @Quuxplusone provided is reasonable to use without wrapping it in a macro (just using it inline).

[sbroberg]

@asoffer, @Quuxplusone's solution is also what I use (as a macro); using it inline is tedious and ugly - you could make the same case that you don't need EXPECT_EQ and its ilk because you could also use them inline as if(expr) {fail test}.

[asoffer]

Internally we've been discussing this and our concerns are that, as Google does not use exceptions, we are the wrong people to do the design and implementation work on this feature. We are determining a process by which we can engage the community (perhaps via video conferencing) to discuss what approach we want to take on this.

[cmoore3]

Whether or not Google uses exceptions is completely irrelevant because gtest is open source and has thousands of production users. The feature is valuable, has multiple possible implementations in the comments above, and despite being encouraged to put up a pull request it was closed without any constructive feedback. Many organizations have to roll their own custom macro for this and it's a real bummer!

[grantwwu]

@gennadiycivil has asked that PRs not include a new macro. I'm not familiar enough with GTest to know why they don't want new macros, but I can imagine several plausible reasons for not wanting one.

Whether or not Google uses exceptions is completely irrelevant because gtest is open source and has thousands of production users.

I'm not sure if you finished reading the sentence, which said "as Google does not use exceptions, we [Google] are the wrong people to do the design and implementation work on this feature".

[AmatanHead]

In our code, we use the following custom matchers:

EXPECT_THAT(
    []() { throw std::runtime_error("message"); },
    Throws<std::runtime_error>());

EXPECT_THAT(
    []() { throw std::runtime_error("message"); },
    ThrowsMessage<std::runtime_error>(HasSubstr("message")));

EXPECT_THAT(
    []() { throw std::runtime_error("message"); },
    ThrowsMessageHasSubstr<std::runtime_error>("message"));

EXPECT_THAT(
    []() { throw std::runtime_error("message"); },
    Throws<std::runtime_error>(Property(&std::runtime_error::what, HasSubstr("message"))));

Would you be interested in a pull request with these matchers?

[CJ-Johnson]

The PR has been submitted internally and will be upstreamed shortly. Thank you all for the feature request!