This version of death tests spawns processes instead of forking the current process. This makes it much easier to port the tests to more platforms, since a lot of platforms don't support forking since it's a very intrusive and expensive operation.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
| libcxx/test/libcxx/algorithms/debug_less.inconsistent.pass.cpp | ||
|---|---|---|
| 16–17 | This seems unrelated, and it creates confusion in the patch. Can you please audit and only keep what's relevant to the patch? | |
I'm ambivalent on this patch because it leaves us in a state where we have two frameworks for death tests and both are reasonably heavyweight. Instead, if we wanted to improve significantly on top of what we have today, I think we would need to implement support for these tests in the test harness (Lit).
We explored this a bit and it could look something like this:
#define TEST_EXPECTED_ERROR(expr, message)
do {
if (gather_tests) {
print(f"{test_counter} [[[{message}]]]");
} else if (test_counter-- == 0) {
expr();
}
} while (false)
// From lit:
// 1. Run the test with no arguments.
// 2. Parse the output and build a mapping like this:
// {test-number => expected-error-message}
// 3. Then you run the executable with each test number and you check the error message.
TEST_RUN_VERIFY_MAIN() {
{
typedef std::array<int, 0> C;
C c = {};
C const& cc = c;
TEST_EXPECTED_ERROR(c[0], "cannot call array<T, 0>::operator[] on a zero-sized array");
TEST_EXPECTED_ERROR(c[1], "cannot call array<T, 0>::operator[] on a zero-sized array");
TEST_EXPECTED_ERROR(cc[0], "cannot call array<T, 0>::operator[] on a zero-sized array");
TEST_EXPECTED_ERROR(cc[1], "cannot call array<T, 0>::operator[] on a zero-sized array");
}
{
typedef std::array<const int, 0> C;
C c = {{}};
C const& cc = c;
TEST_EXPECTED_ERROR(c[0], "cannot call array<T, 0>::operator[] on a zero-sized array");
TEST_EXPECTED_ERROR(c[1], "cannot call array<T, 0>::operator[] on a zero-sized array");
TEST_EXPECTED_ERROR(cc[0], "cannot call array<T, 0>::operator[] on a zero-sized array");
TEST_EXPECTED_ERROR(cc[1], "cannot call array<T, 0>::operator[] on a zero-sized array");
}
return 0;
}Unfortunately, the "From Lit" part ends up being quite complicated (we prototyped) and so it's not clear it's worth pursuing at this time. Barring that, I think we should instead improve the current death test framework to support intentional calls to std::terminate, which should be doable roughly with
diff --git a/libcxx/test/support/check_assertion.h b/libcxx/test/support/check_assertion.h index 741dc4a78828..263413b50b62 100644 --- a/libcxx/test/support/check_assertion.h +++ b/libcxx/test/support/check_assertion.h @@ -111,7 +111,7 @@ inline AssertionInfoMatcher& GlobalMatcher() { struct DeathTest { enum ResultKind { - RK_DidNotDie, RK_MatchFound, RK_MatchFailure, RK_SetupFailure, RK_Unknown + RK_DidNotDie, RK_MatchFound, RK_MatchFailure, RK_SetupFailure, RK_TerminatedAsExpected, RK_Unknown }; static const char* ResultKindToString(ResultKind RK) { @@ -121,6 +121,7 @@ struct DeathTest { CASE(RK_DidNotDie); CASE(RK_SetupFailure); CASE(RK_MatchFound); + CASE(RK_TerminatedAsExpected); CASE(RK_Unknown); } return "not a result kind"; @@ -256,6 +257,11 @@ void std::__libcpp_verbose_abort(char const* format, ...) { template <class Func> inline bool ExpectDeath(const char* stmt, Func&& func, AssertionInfoMatcher Matcher) { DeathTest DT(Matcher); + set_terminate([] { + if (__expect_to_terminate__) + std::exit(RK_TerminatedAsExpected); + std::abort(); + }); DeathTest::ResultKind RK = DT.Run(func); auto OnFailure = [&](const char* msg) { std::fprintf(stderr, "EXPECT_DEATH( %s ) failed! (%s)\n\n", stmt, msg); @@ -273,6 +279,8 @@ inline bool ExpectDeath(const char* stmt, Func&& func, AssertionInfoMatcher Matc switch (RK) { case DeathTest::RK_MatchFound: return true; + case DeathTest::RK_TerminatedAsExpected: + return true; case DeathTest::RK_SetupFailure: return OnFailure("child failed to setup test environment"); case DeathTest::RK_Unknown:
| libcxx/test/libcxx/containers/sequences/array/array.zero/assert.subscript.pass.cpp | ||
|---|---|---|
| 20 | Should this be TEST_TERMINATIONS? It's also problematic that the test is going to appear to work if you misuse the "test framework" like this. Could probably be detected by referencing something that TEST_TERMINATIONS defines from EXPECT_LIBCPP_ASSERT_FAILURE. | |
| libcxx/test/support/terminating_test_helper.h | ||
| 9 | ||
| 37 | ||
This seems unrelated, and it creates confusion in the patch. Can you please audit and only keep what's relevant to the patch?