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.