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

[Kristina Bessonova via Phabricator via libcxx-commits]

krisb added a comment.

@Quuxplusone , just want to ask, are you going to do smth related to this patch, or prefer me to take care of all the dependencies (like missed articles and db_move tests issue)? Just want to avoid doing the same things twice.



================
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)
----------------
Quuxplusone wrote:
> 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.
What about something like

TEST_EXIT_WITH_MESSAGE
TEST_ASSERT_FAILURE
TEST_LIBCPP_ASSERT_FAILURE

then? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100595