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

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

Quuxplusone added a comment.

> just want to ask, are you going to do smth related to this patch, or prefer me to take care of all the dependencies

Yes, sorry, it's on my to-do list. If we're lucky I'll get to it today. :)



================
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:
> > 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? 
`TEST_LIBCPP_ASSERT_FAILURE` would be OK by me.
(The competitor is `ASSERT_FAILS_WITH_MESSAGE`. Either of these two is fine with me.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100595