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–18 | 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 | ||
10 | ||
38 |
This seems unrelated, and it creates confusion in the patch. Can you please audit and only keep what's relevant to the patch?