On Fri, Sep 30, 2022 at 8:26 PM David Gow davidgow@google.com wrote:
On Sat, Oct 1, 2022 at 8:26 AM 'Daniel Latypov' via KUnit Development kunit-dev@googlegroups.com wrote:
Context: Currently this macro's name, KUNIT_ASSERTION conflicts with the name of an enum whose values are {KUNIT_EXPECTATION, KUNIT_ASSERTION}.
It's hard to think of a better name for the enum, so rename this macro. It's also a bit strange that the macro might do nothing depending on the boolean argument `pass`. Why not have callers check themselves?
This patch: Moves the pass/fail checking into the callers of KUNIT_ASSERTION, so now we only call it when the check has failed. Then we rename the macro the _KUNIT_FAILED() to reflect the new semantics.
Signed-off-by: Daniel Latypov dlatypov@google.com
Looks good to me. I can't say the name _KUNIT_FAILED() feels perfect to me, but I can't think of anything better, either. We've not used a leading underscore for internal macros much thus far, as well, though I've no personal objections to starting.
Yeah, I also didn't add a leading underscore on the new KUNIT_INIT_ASSERT() macro elsewhere in this series. So I'm not necessarily proposing that we should start doing so here.
It feels like that KUNIT_FAILED is far too similar to the enum 55 enum kunit_status { 56 KUNIT_SUCCESS, 57 KUNIT_FAILURE, 58 KUNIT_SKIPPED, 59 };
I.e. we'd be remove one naming conflict between a macro and enum, but basically introducing a new one in its place :P Tbh, I was originally going to have this patch just be s/KUNIT_ASSERTION()/_KUNIT_ASSERTION() to reduce the conflict. But I figured we could reduce the number of arguments to the macro (drop `pass`) and have a reason to give it a different name.
I'm also not entirely convinced about _KUNIT_FAILED(), but I haven't had any significantly better ideas since I sent the RFC in May.
Daniel