Re: expressive test macros (was: Report test_atomic_ops() failuresconsistently, via macros)

On Sat, Oct 05, 2019 at 12:07:29PM -0700, Andres Freund wrote:
> +#define EXPECT_EQ_U32(result_expr, expected_expr)  \
> +   do { \
> +       uint32      result = (result_expr); \
> +       uint32      expected = (expected_expr); \
> +       if (result != expected) \
> +           elog(ERROR, \
> +                "%s yielded %u, expected %s in file \"%s\" line %u", \
> +                #result_expr, result, #expected_expr, __FILE__, __LINE__); \
> +   } while (0)
> ...
> 
> 
> I wonder if we should put these (and a few more, for other types), into
> a more general place. I would like to have them for writing both tests
> like regress.c:test_atomic_ops(), and for writing assertions that
> actually display useful error messages.  For the former it makes sense
> to ERROR out, for the latter they ought to abort, as currently.
> 
> Seems like putting ASSERT_{EQ,LT,...}_{U32,S32,...} (or Assert_Eq_...,
> but that'd imo look weirder than the inconsistency) into c.h would make
> sense, and EXPECT_ somewhere in common/pg_test.h or such?

Sounds reasonable.  For broader use, I would include the expected value, not
just expected_expr:

 elog(ERROR, \
      "%s yielded %u, expected %s (%u) in file \"%s\" line %u", \
      #result_expr, result, #expected_expr, expected, __FILE__, __LINE__); \

I didn't do that for the atomics tests, where expected_expr is always trivial.
The codebase has plenty of Assert(x == y) where either of x or y could have
the surprising value.