Re: pgsql: Test pg_atomic_fetch_add_ with variable addend and 16-bitedge c - Mailing list pgsql-committers

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");