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


Reply via email to