Re: [PATCH] math: Add LIBM_TEST_VERBOSE environment variable support.

[Carlos O'Donell <carlos@redhat.com>]

On 5/6/24 20:34, Joe Simmons-Talbott wrote:
> From: Joe Talbott <joetalbott@gmail.com>
> 
> Allow the libm-test-driver based tests to have their verbosity set based
> on the LIMB_TESt_VERBOSE environment variable.  This allows the entire

s/LIMB_TESt_VERBOSE/LIBM_TEST_VERBOSE/g

Please call it GLIBC_TEST_LIBM_VERBOSE.

We should really use a known conventional prefix e.g. GLIBC_TEST.

We don't currently, but lets move in that direction.

At worse I see WAIT_FOR_DEBUGGER, PID_OUTSIDE_CONTAINER, TIMEOUTFACTOR,
TEST_DIRECT and TEST_COREDUMPS, which could all get cleaned up.

> testsuite to be run with a non-default verbosity.
> ---
>  math/libm-test-support.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/math/libm-test-support.c b/math/libm-test-support.c
> index 1d60ac783b..a45686054f 100644
> --- a/math/libm-test-support.c
> +++ b/math/libm-test-support.c
> @@ -1139,6 +1139,7 @@ libm_test_init (int argc, char **argv)
>    int remaining;
>    char *ulps_file_path;
>    size_t dir_len = 0;
> +  char *verbose_env;
>  
>    verbose = 1;
>    output_ulps = 0;
> @@ -1148,6 +1149,10 @@ libm_test_init (int argc, char **argv)
>    /* XXX set to 0 for releases.  */
>    ignore_max_ulp = 0;
>  
> +  verbose_env = getenv("LIBM_TEST_VERBOSE");

Adjust as above.

> +  if (verbose_env != NULL)
> +    verbose = (unsigned int) strtoul (verbose_env, NULL, 0);

What happens if this conversion fails?

Please see how we do this with TIMEOUTFACOR:

326   /* If set, read the test TIMEOUTFACTOR value from the environment.
327      This value is used to scale the default test timeout values. */
328   char *envstr_timeoutfactor = getenv ("TIMEOUTFACTOR");
329   if (envstr_timeoutfactor != NULL)
330     {
331       char *envstr_conv = envstr_timeoutfactor;
332       unsigned long int env_fact;
333 
334       env_fact = strtoul (envstr_timeoutfactor, &envstr_conv, 0);
335       if (*envstr_conv == '\0' && envstr_conv != envstr_timeoutfactor)
336         timeoutfactor = MAX (env_fact, 1);
337     }


> +
>    /* Parse and process arguments.  */
>    argp_parse (&argp, argc, argv, 0, &remaining, NULL);
>  

-- 
Cheers,
Carlos.