[v5,3/4] support: Add FAIL test failure helper

Message ID alpine.DEB.2.21.2407121946040.38148@angie.orcam.me.uk
State Committed
Delegated to: DJ Delorie
Headers
Series stdio-common: Add test for vfscanf with matches longer than INT_MAX [BZ #27650] |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed

Commit Message

Maciej W. Rozycki July 12, 2024, 8:48 p.m. UTC
  From: Maciej W. Rozycki <macro@redhat.com>

Add a FAIL test failure helper analogous to FAIL_RET, that does not 
cause the current function to return, providing a standardized way to 
report a test failure with a message supplied while permitting the 
caller to continue executing, for further reporting, cleaning up, etc.

Update existing test cases that provide a conflicting definition of FAIL 
by removing the local FAIL definition and then as follows:

- tst-fortify-syslog: provide a meaningful message in addition to the 
  file name already added by <support/check.h>; 'support_record_failure' 
  is already called by 'support_print_failure_impl' invoked by the new 
  FAIL test failure helper.

- tst-ctype: no update to FAIL calls required, with the name of the file
  and the line number within of the failure site additionally included 
  by the new FAIL test failure helper, and error counting plus count 
  reporting upon test program termination also already provided by 
  'support_record_failure' and 'support_report_failure' respectively, 
  called by 'support_print_failure_impl' and 'adjust_exit_status' also 
  respectively.  However in a number of places 'printf' is called and 
  the error count adjusted by hand, so update these places to make use 
  of FAIL instead.  And last but not least adjust the final summary just 
  to report completion, with any error count following as reported by 
  the test driver.

- test-tgmath2: no update to FAIL calls required, with the name of the 
  file of the failure site additionally included by the new FAIL test 
  failure helper.  Also there is no need to track the return status by 
  hand as any call to FAIL will eventually cause the test case to return 
  an unsuccesful exit status regardless of the return status from the 
  test function, via a call to 'adjust_exit_status' made by the test 
  driver.
---
Hi,

 I have verified, by inducing errors by hand across all three test cases 
affected, that replacement error reporting is at least equivalent to 
original one and that the return status is correct.  Given the nature of 
the change all the test cases, even though independent from each other and 
the change to <support/check.h>, need to go in as a single commit so as to 
avoid transient regressions, which could then adversely affect bisection 
in the future.

  Maciej

Changes from v4:

- Remove local FAIL macro definitions from tst-fortify-syslog, tst-ctype, 
  and test-tgmath2, adjusting call sites to make use of the macro now 
  provided by <support/check.h>.

No change from v3.

New change in v3.
---
 debug/tst-fortify-syslog.c |    8 +-------
 localedata/tst-ctype.c     |   40 +++++++++-------------------------------
 math/test-tgmath2.c        |   13 +++----------
 support/check.h            |    5 +++++
 4 files changed, 18 insertions(+), 48 deletions(-)

glibc-support-check-fail.diff
  

Comments

DJ Delorie July 23, 2024, 1:10 a.m. UTC | #1
"Maciej W. Rozycki" <macro@orcam.me.uk> writes:
> Add a FAIL test failure helper analogous to FAIL_RET, that does not 
> cause the current function to return,

Sort of a FAIL_NOP.  Ok.

> - tst-fortify-syslog: provide a meaningful message in addition to the 
>   file name already added by <support/check.h>; 'support_record_failure' 
>   is already called by 'support_print_failure_impl' invoked by the new 
>   FAIL test failure helper.

Ok.

> - tst-ctype: no update to FAIL calls required, with the name of the file
>   and the line number within of the failure site additionally included 
>   by the new FAIL test failure helper, and error counting plus count 
>   reporting upon test program termination also already provided by 
>   'support_record_failure' and 'support_report_failure' respectively, 

Ok.

> - test-tgmath2: no update to FAIL calls required, with the name of the 
>   file of the failure site additionally included by the new FAIL test 
>   failure helper.  Also there is no need to track the return status by 
>   hand as any call to FAIL will eventually cause the test case to return 
>   an unsuccesful exit status regardless of the return status from the 
>   test function, via a call to 'adjust_exit_status' made by the test 
>   driver.
> ---
> Hi,
>
>  I have verified, by inducing errors by hand across all three test cases 
> affected, that replacement error reporting is at least equivalent to 
> original one and that the return status is correct.  Given the nature of 
> the change all the test cases, even though independent from each other and 
> the change to <support/check.h>, need to go in as a single commit so as to 
> avoid transient regressions, which could then adversely affect bisection 
> in the future.
>
>   Maciej
>
> Changes from v4:
>
> - Remove local FAIL macro definitions from tst-fortify-syslog, tst-ctype, 
>   and test-tgmath2, adjusting call sites to make use of the macro now 
>   provided by <support/check.h>.
>
> No change from v3.
>
> New change in v3.
> ---
>  debug/tst-fortify-syslog.c |    8 +-------
>  localedata/tst-ctype.c     |   40 +++++++++-------------------------------
>  math/test-tgmath2.c        |   13 +++----------
>  support/check.h            |    5 +++++
>  4 files changed, 18 insertions(+), 48 deletions(-)
>
> glibc-support-check-fail.diff



> Index: glibc/debug/tst-fortify-syslog.c
>  #include <string.h>
>  #include <unistd.h>
> -#include <stdio.h>

Ok.

> -#define FAIL()						\
> -  do {							\
> -      printf ("Failure on line %d\n", __LINE__);	\
> -      support_record_failure ();			\
> -  } while (0)

Ok.

>  #define CHK_FAIL_END					\
>        chk_fail_ok = 0;					\
> -      FAIL ();						\
> +      FAIL ("not supposed to reach here");		\
>      }

Ok.

> Index: glibc/localedata/tst-ctype.c
> ===================================================================
> --- glibc.orig/localedata/tst-ctype.c
> +++ glibc/localedata/tst-ctype.c
> @@ -21,6 +21,8 @@
>  #include <stdio.h>
>  #include <string.h>
>  
> +#include <support/check.h>
> +

Ok.

> -#define FAIL(str, args...) \
> -  {									      \
> -    printf ("      " str "\n", ##args);					      \
> -    ++errors;								      \
> -  }

Ok.

> -  int errors = 0;

Ok.

>  	    {
>  	      if (((__ctype_b[(unsigned int) *inp] & classes[n].mask) != 0)
>  		  != (*resp != '0'))
> -		{
> -		  printf ("    is%s('%c' = '\\x%02x') %s true\n", inpline,
> -			  *inp, *inp, *resp == '1' ? "not" : "is");
> -		  ++errors;
> -		}
> +		FAIL ("    is%s('%c' = '\\x%02x') %s true\n", inpline,
> +		      *inp, *inp, *resp == '1' ? "not" : "is");


Ok 3x.


> -  if (errors != 0)
> -    {
> -      printf ("  %d error%s for `%s' locale\n\n\n", errors,
> -	      errors == 1 ? "" : "s", setlocale (LC_ALL, NULL));
> -      return 1;
> -    }
> -
> -  printf ("  No errors for `%s' locale\n\n\n", setlocale (LC_ALL, NULL));
> +  printf ("Completed testing for `%s' locale\n\n\n", setlocale (LC_ALL, NULL));
>    return 0;
>  }

Failure count is reported by the skeleton, ok.

> Index: glibc/math/test-tgmath2.c
> +#include <support/check.h>

Ok.

> -#define FAIL(str) \
> -  do								\
> -    {								\
> -      printf ("%s failure on line %d\n", (str), __LINE__);	\
> -      result = 1;						\
> -    }								\
> -  while (0)

Ok.

>  int
>  test_cos (const int Vint4, const long long int Vllong4)
>  {
> -  int result = 0;
> -

Ok.

> -  return result;
> +  return 0;

Ok.

> Index: glibc/support/check.h
>  /* Record a test failure, print the failure message to standard output
> +   and pass the result of 1 through.  */
> +#define FAIL(...) \
> +  support_print_failure_impl (__FILE__, __LINE__, __VA_ARGS__)
> +
> +/* Record a test failure, print the failure message to standard output

Ok.

LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>
  

Patch

Index: glibc/debug/tst-fortify-syslog.c
===================================================================
--- glibc.orig/debug/tst-fortify-syslog.c
+++ glibc/debug/tst-fortify-syslog.c
@@ -22,7 +22,6 @@ 
 #include <syslog.h>
 #include <string.h>
 #include <unistd.h>
-#include <stdio.h>
 
 #include <support/check.h>
 #include <support/support.h>
@@ -46,18 +45,13 @@  handler (int sig)
     _exit (127);
 }
 
-#define FAIL()						\
-  do {							\
-      printf ("Failure on line %d\n", __LINE__);	\
-      support_record_failure ();			\
-  } while (0)
 #define CHK_FAIL_START					\
   chk_fail_ok = 1;					\
   if (! setjmp (chk_fail_buf))				\
     {
 #define CHK_FAIL_END					\
       chk_fail_ok = 0;					\
-      FAIL ();						\
+      FAIL ("not supposed to reach here");		\
     }
 
 static void
Index: glibc/localedata/tst-ctype.c
===================================================================
--- glibc.orig/localedata/tst-ctype.c
+++ glibc/localedata/tst-ctype.c
@@ -21,6 +21,8 @@ 
 #include <stdio.h>
 #include <string.h>
 
+#include <support/check.h>
+
 
 static const char lower[] = "abcdefghijklmnopqrstuvwxyz";
 static const char upper[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
@@ -53,19 +55,11 @@  static struct classes
 #define nclasses (sizeof (classes) / sizeof (classes[0]))
 
 
-#define FAIL(str, args...) \
-  {									      \
-    printf ("      " str "\n", ##args);					      \
-    ++errors;								      \
-  }
-
-
 static int
 do_test (void)
 {
   const char *cp;
   const char *cp2;
-  int errors = 0;
   char *inpline = NULL;
   size_t inplinelen = 0;
   char *resline = NULL;
@@ -394,11 +388,8 @@  punct = %04x  alnum = %04x\n",
 	    {
 	      if (((__ctype_b[(unsigned int) *inp] & classes[n].mask) != 0)
 		  != (*resp != '0'))
-		{
-		  printf ("    is%s('%c' = '\\x%02x') %s true\n", inpline,
-			  *inp, *inp, *resp == '1' ? "not" : "is");
-		  ++errors;
-		}
+		FAIL ("    is%s('%c' = '\\x%02x') %s true\n", inpline,
+		      *inp, *inp, *resp == '1' ? "not" : "is");
 	      ++inp;
 	      ++resp;
 	    }
@@ -408,11 +399,8 @@  punct = %04x  alnum = %04x\n",
 	  while (*inp != '\0')
 	    {
 	      if (tolower (*inp) != *resp)
-		{
-		  printf ("    tolower('%c' = '\\x%02x') != '%c'\n",
-			  *inp, *inp, *resp);
-		  ++errors;
-		}
+		FAIL ("    tolower('%c' = '\\x%02x') != '%c'\n",
+		      *inp, *inp, *resp);
 	      ++inp;
 	      ++resp;
 	    }
@@ -422,11 +410,8 @@  punct = %04x  alnum = %04x\n",
 	  while (*inp != '\0')
 	    {
 	      if (toupper (*inp) != *resp)
-		{
-		  printf ("    toupper('%c' = '\\x%02x') != '%c'\n",
-			  *inp, *inp, *resp);
-		  ++errors;
-		}
+		FAIL ("    toupper('%c' = '\\x%02x') != '%c'\n",
+		      *inp, *inp, *resp);
 	      ++inp;
 	      ++resp;
 	    }
@@ -436,14 +421,7 @@  punct = %04x  alnum = %04x\n",
     }
 
 
-  if (errors != 0)
-    {
-      printf ("  %d error%s for `%s' locale\n\n\n", errors,
-	      errors == 1 ? "" : "s", setlocale (LC_ALL, NULL));
-      return 1;
-    }
-
-  printf ("  No errors for `%s' locale\n\n\n", setlocale (LC_ALL, NULL));
+  printf ("Completed testing for `%s' locale\n\n\n", setlocale (LC_ALL, NULL));
   return 0;
 }
 
Index: glibc/math/test-tgmath2.c
===================================================================
--- glibc.orig/math/test-tgmath2.c
+++ glibc/math/test-tgmath2.c
@@ -24,6 +24,8 @@ 
 #include <string.h>
 #include <tgmath.h>
 
+#include <support/check.h>
+
 //#define DEBUG
 
 typedef complex float cfloat;
@@ -87,13 +89,6 @@  enum
 int count;
 int counts[Tlast][C_last];
 
-#define FAIL(str) \
-  do								\
-    {								\
-      printf ("%s failure on line %d\n", (str), __LINE__);	\
-      result = 1;						\
-    }								\
-  while (0)
 #define TEST_TYPE_ONLY(expr, rettype) \
   do								\
     {								\
@@ -133,8 +128,6 @@  int counts[Tlast][C_last];
 int
 test_cos (const int Vint4, const long long int Vllong4)
 {
-  int result = 0;
-
   TEST (cos (vfloat1), float, cos);
   TEST (cos (vdouble1), double, cos);
   TEST (cos (vldouble1), ldouble, cos);
@@ -152,7 +145,7 @@  test_cos (const int Vint4, const long lo
   TEST (cos (Vcdouble1), cdouble, cos);
   TEST (cos (Vcldouble1), cldouble, cos);
 
-  return result;
+  return 0;
 }
 
 int
Index: glibc/support/check.h
===================================================================
--- glibc.orig/support/check.h
+++ glibc/support/check.h
@@ -25,6 +25,11 @@ 
 __BEGIN_DECLS
 
 /* Record a test failure, print the failure message to standard output
+   and pass the result of 1 through.  */
+#define FAIL(...) \
+  support_print_failure_impl (__FILE__, __LINE__, __VA_ARGS__)
+
+/* Record a test failure, print the failure message to standard output
    and return 1.  */
 #define FAIL_RET(...) \
   return support_print_failure_impl (__FILE__, __LINE__, __VA_ARGS__)