google / google/googletest

[FR]: A clean way to opt-out of `return` in `ASSERT_FOO` when using a throwy listener

Open
#4,770 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
C++
Stars
39.6k
Forks
10.9k
Avg merge
6d 13h
Merged PRs (30d)
1

Description

### Does the feature exist in the most recent commit?

No, although there is a very dirty workaround.

### Why do we need this feature?

The [documentation](https://google.github.io/googletest/advanced.html#asserting-on-subroutines-with-an-exception) suggests the following code to make assertions throwy rather than just returning from the current function:

```cpp
class ThrowListener : public testing::EmptyTestEventListener {
void OnTestPartResult(const testing::TestPartResult& result) override {
if (result.type() == testing::TestPartResult::kFatalFailure) {
throw testing::AssertionException(result);
}
}
};
int main(int argc, char** argv) {
...
testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener);
return RUN_ALL_TESTS();
}
```

When using that the `return` in this code is never actually reached https://github.com/google/googletest/blob/e9092b12dc3cf617d47578f13a1f64285cfa5b2f/googletest/include/gtest/internal/gtest-internal.h#L1292-L1293

However it sill causes many of the issues described elsewhere in the docs, such as [here](https://google.github.io/googletest/advanced.html#assertion-placement)

> The one constraint is that assertions that generate a fatal failure (FAIL* and ASSERT_*) can only be used in void-returning functions. [...] Constructors and destructors are not considered void-returning functions, according to the C++ language specification, and so you may not use fatal assertions in them; you’ll get a compilation error if you try.

We are looking at porting from a custom test framework that uses exceptions to gtest, and as expected the returns cause problems. We can work around it by putting code like this in our wrapping header, but that is quite dirty, so we would prefer a clean and supported solution:

```cpp
#include
#undef GTEST_FATAL_FAILURE_
#define GTEST_FATAL_FAILURE_(message) \
/*return*/ GTEST_MESSAGE_(message, ::testing::TestPartResult::kFatalFailure)
```

The key thing is that we still want to be able to distinguish `ASSERT` from `EXPECT` in the listener by using `kFatalFailure`, but without the downsides of that pesky `return`.

### Describe the proposal.

I think the ideal solution would be a build-time flag to opt-out of the return. Either some bazel and/or cmake config option or a `#define` that the code looks for. Eg

```cpp
#if GTEST_RETURN_ON_FATAL_ASSERT
#define GTEST_FATAL_FAILURE_(message) \
return GTEST_MESSAGE_(message, ::testing::TestPartResult::kFatalFailure)
#else
#define GTEST_FATAL_FAILURE_(message) \
GTEST_MESSAGE_(message, ::testing::TestPartResult::kFatalFailure)
#endif
```

Or define something that expands to either `return` or nothing to reduce code duplication. I _think_ it would generally be safe to link code built in both modes, so it _could_ be a per-TU option, although of course that is inviting technical ODR violations if any inline functions that use that macro are compiled in both modes. They are _probably_ benign, but the safest option would be to make it a config option when building gtest that all consumers of the lib need to use consistently. If you make it a build time flag, it would also be _ideal_ if that also caused it to automatically do the equivalent of registering the throwing listener so that it is impossible to forget to do so.

In fact, given that the documentation [here](https://google.github.io/googletest/faq.html#CtorVsSetUp) says _"The GoogleTest team is considering making the assertion macros throw on platforms where exceptions are enabled (e.g. Windows, Mac OS, and Linux client-side), which will eliminate the need for the user to propagate failures from a subroutine to its caller. Therefore, you shouldn’t use GoogleTest assertions in a destructor if your code could run on such a platform."_, it would be great if that was actually the default behavior when building gtest with exceptions enabled. Obviously with an opt-out for codebases that are not correctly following that advice, or mixing TUs build with and without exception support.

### Is the feature specific to an operating system, compiler, or build system version?

No. Although obviously it depends on exceptions being enabled to actually make the fatal asserts exit the test.

Contributor guide

Open the contributing guide

Research direction

Start with the ASSERT_FOO macro in googletest/include/gtest/internal/gtest-internal.h at the linked lines, then read the advanced assertion-placement and CtorVsSetUp documentation. Review how build configuration and exception-enabled listeners are handled; done means a documented, supported way to avoid the fatal-assert return while preserving kFatalFailure behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
testing-qa
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.