[libcxx-commits] [PATCH] D100595: [libcxx][test] Attempt to make debug mode tests more bulletproof

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/test/support/debug_macros.h:25-31
+#define EXPECTED_FAIL(expr, m)                                                                                         \
+  do {                                                                                                                 \
+    ::expected_libcpp_assert_message = m;                                                                              \
+    std::__libcpp_set_debug_function(&::test_debug_function);                                                          \
+    (void)(expr);                                                                                                      \
+    assert(false);                                                                                                     \
+  } while (false)
----------------
krisb wrote:
> Quuxplusone wrote:
> > I think the only remaining controversial point is what to name this macro. I don't really like `EXPECTED_FAIL` (in that it doesn't start with either `ASSERT_` or `TEST_`, and it still reminds me of `XFAIL`, and //technically// it's a reserved identifier because <errno.h>).
> > I suggest `ASSERT_FAILS_WITH_MESSAGE(expr, m)` — what do you think of that name, @krisb?
> `ASSERT_FAILS_WITH_MESSAGE(expr, m)` is okay to me, but I'd still prefer to start it with `EXPECT`/`EXPECTED`. Maybe we can find something better? Here are some options that came to my mind:
> 
> EXPECT_ASSERT_FAIL
> EXPECT_LIBCPP_ASSERT_FAIL
> EXPECT_ASSERT_FAIL_WITH_MESSAGE
> 
> EXPECT_DEATH_WITH_MESSAGE
> EXPECT_EXIT_WITH_MESSAGE
> 
> EXPECTED_ERROR
> 
> LIBCPP_ASSERT_FAIL
> LIBCPP_ASSERT_FAIL_WITH_MESSAGE
> 
> If nothing looks good to you, I'll stay with your variant.
I specifically want something that starts either with `ASSERT_` or `TEST_`, for consistency with the other testing macros.
I do see how it's awkward that we want to express that we're "asserting" that "_LIBCPP_ASSERT" "assert-fails" — lots of potential repetition in there, that we're both trying to avoid somehow.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100595/new/

https://reviews.llvm.org/D100595