Re: pgsql: Test pg_atomic_fetch_add_ with variable addend and 16-bit edge c

On Sun, Sep 15, 2019 at 01:00:21PM -0300, Alvaro Herrera wrote:
> On 2019-Sep-14, Noah Misch wrote:
> > Test pg_atomic_fetch_add_ with variable addend and 16-bit edge cases.
>
> I don't understand this.
>
> if (pg_atomic_fetch_add_u32(&var, INT_MAX) != INT_MAX)
> elog(ERROR, "pg_atomic_add_fetch_u32() #3 wrong");
>
> pg_atomic_fetch_add_u32(&var, 2); /* wrap to 0 */
>
> if (pg_atomic_fetch_add_u32(&var, PG_INT16_MAX) != 0)
> elog(ERROR, "pg_atomic_fetch_add_u32() #3 wrong");
>
> The existing one seems to be naming the wrong function in the error
> message, and if you fix that then you have two "#3 wrong" cases.
> Isn't this confusing? Am I just too sensitive? Is this pointless to
> fix?

It's a typo-class defect. Would you like me to fix it, or would you like to
fix it? I'd consider wrapping the tests in a macro (may be overkill, since
these tests don't change much):

--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -670,6 +670,16 @@ test_atomic_flag(void)
pg_atomic_clear_flag(&flag);
}

+#define EXPECT(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)
+
static void
test_atomic_uint32(void)
{
@@ -678,17 +688,10 @@ test_atomic_uint32(void)
int i;

pg_atomic_init_u32(&var, 0);
-
- if (pg_atomic_read_u32(&var) != 0)
- elog(ERROR, "atomic_read_u32() #1 wrong");
-
+ EXPECT(pg_atomic_read_u32(&var), 0);
pg_atomic_write_u32(&var, 3);
-
- if (pg_atomic_read_u32(&var) != 3)
- elog(ERROR, "atomic_read_u32() #2 wrong");
-
- if (pg_atomic_fetch_add_u32(&var, pg_atomic_read_u32(&var) - 2) != 3)
- elog(ERROR, "atomic_fetch_add_u32() #1 wrong");
+ EXPECT(pg_atomic_read_u32(&var), 3);
+ EXPECT(pg_atomic_fetch_add_u32(&var, pg_atomic_read_u32(&var) - 2), 3);

if (pg_atomic_fetch_sub_u32(&var, 1) != 4)
elog(ERROR, "atomic_fetch_sub_u32() #1 wrong");