[PATCH] test-suite: handle format with filename.c not existing

[PATCH] test-suite: handle format with filename.c not existing

[Randy Dunlap]

From: Randy Dunlap <rdunlap@infradead.org>

In error(), quiet is undefined when the command:
  $ ./test-suite format filename1.c foobar1

is used and filename1.c does not exist. This causes a shell script error:
  ./test-suite: line 147: [: : integer expression expected

because $quiet is undefined in
  [ "$quiet" -ne 1 ] && echo "error: $1"
so the error message is lost.

Just initialize quiet before any command line parsing.

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
 validation/test-suite |    2 ++
 1 file changed, 2 insertions(+)

--- sprs.orig/validation/test-suite
+++ sprs.next/validation/test-suite
@@ -368,6 +368,8 @@ arg_file()
 	return 0
 }
 
+quiet=0
+
 case "$1" in
 	'')
 		do_test_suite

Re: [PATCH] test-suite: handle format with filename.c not existing

[Christopher Li]

On Mon, Jan 22, 2018 at 11:05 AM, Randy Dunlap <rdunlap@infradead.org> wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
>
> In error(), quiet is undefined when the command:
>   $ ./test-suite format filename1.c foobar1
>
> is used and filename1.c does not exist. This causes a shell script error:
>   ./test-suite: line 147: [: : integer expression expected
>
> because $quiet is undefined in
>   [ "$quiet" -ne 1 ] && echo "error: $1"
> so the error message is lost.
>
> Just initialize quiet before any command line parsing.
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
>  validation/test-suite |    2 ++
>  1 file changed, 2 insertions(+)
>
> --- sprs.orig/validation/test-suite
> +++ sprs.next/validation/test-suite
> @@ -368,6 +368,8 @@ arg_file()
>         return 0
>  }
>
> +quiet=0
> +

Good catch. I think this fix is not ideal.
There is another quite=0 inside "do_test()".
<quote>
quiet=0
[ $must_fail -eq 1 ] && [ $V -eq 0 ] && quiet=1
</quote>

The "quite=0" only cover the case inside "do_test()".
Even with this patch,  If  the "quite" is used outside of
"do_test()", The "quite" variable is not set properly according to "V".

I would suggest move the above two quote line out side
of "do_test()", right after the default value of "V", in the
beginning of the file:

<quote>
# defaults to not verbose
[ -z "$V" ] && V=0
</quote>

Chris

Re: [PATCH] test-suite: handle format with filename.c not existing

[Randy Dunlap]

On 01/23/2018 03:22 AM, Christopher Li wrote:
> On Mon, Jan 22, 2018 at 11:05 AM, Randy Dunlap <rdunlap@infradead.org> wrote:
>> From: Randy Dunlap <rdunlap@infradead.org>
>>
>> In error(), quiet is undefined when the command:
>>   $ ./test-suite format filename1.c foobar1
>>
>> is used and filename1.c does not exist. This causes a shell script error:
>>   ./test-suite: line 147: [: : integer expression expected
>>
>> because $quiet is undefined in
>>   [ "$quiet" -ne 1 ] && echo "error: $1"
>> so the error message is lost.
>>
>> Just initialize quiet before any command line parsing.
>>
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> ---
>>  validation/test-suite |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> --- sprs.orig/validation/test-suite
>> +++ sprs.next/validation/test-suite
>> @@ -368,6 +368,8 @@ arg_file()
>>         return 0
>>  }
>>
>> +quiet=0
>> +
> 
> Good catch. I think this fix is not ideal.
> There is another quite=0 inside "do_test()".
> <quote>
> quiet=0
> [ $must_fail -eq 1 ] && [ $V -eq 0 ] && quiet=1
> </quote>
> 
> The "quite=0" only cover the case inside "do_test()".
> Even with this patch,  If  the "quite" is used outside of
> "do_test()", The "quite" variable is not set properly according to "V".
> 
> I would suggest move the above two quote line out side
> of "do_test()", right after the default value of "V", in the
> beginning of the file:
> 
> <quote>
> # defaults to not verbose
> [ -z "$V" ] && V=0
> </quote>

At the beginning of the file, $must_fail is not defined, so it will not -eq 1,
so quiet will remain = 0.

I guess that I don't understand the interplay between $V and $quiet and why
have both of them.

Are you suggesting that $quiet should be initialized by default like this:
(just drop the $must_fail part)

> quiet=0
> [ $V -eq 0 ] && quiet=1


thanks,
-- 
~Randy

Re: [PATCH] test-suite: handle format with filename.c not existing

[Christopher Li]

On Tue, Jan 23, 2018 at 10:20 AM, Randy Dunlap <rdunlap@infradead.org> wrote:
>
> I guess that I don't understand the interplay between $V and $quiet and why
> have both of them.
>

It seems that $quiet is just invert of $V, which introduce in
the following two commits by Luc.

commit: ee43fc784e52e3ee9e1b45da1be5193ea3b64334
Author: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Date:   Mon Feb 13 00:29:04 2017 +0100

    testsuite: quieter error reporting for 'known-to-fail'

commit: d2a7009a78effb2a8ac867de9240c4a89d181f5d
Author: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Date:   Mon Feb 13 00:29:03 2017 +0100

    testsuite: allow quieter error reporting


> Are you suggesting that $quiet should be initialized by default like this:
> (just drop the $must_fail part)
>
>> quiet=0
>> [ $V -eq 0 ] && quiet=1


yes, I was thinking some thing like this,
but I haven't give it a lot of test.

>diff --git a/validation/test-suite b/validation/test-suite
index cf151a3..5e10942 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -31,6 +31,7 @@ known_ko_tests=0

 # defaults to not verbose
 [ -z "$V" ] && V=0
+[ $V -eq 0 ] && quiet=1 || quiet=0

 ##

Chris

Re: [PATCH] test-suite: handle format with filename.c not existing

[Randy Dunlap]

On 01/23/2018 03:38 PM, Christopher Li wrote:

> yes, I was thinking some thing like this,
> but I haven't give it a lot of test.
> 
>> diff --git a/validation/test-suite b/validation/test-suite
> index cf151a3..5e10942 100755
> --- a/validation/test-suite
> +++ b/validation/test-suite
> @@ -31,6 +31,7 @@ known_ko_tests=0
> 
>  # defaults to not verbose
>  [ -z "$V" ] && V=0
> +[ $V -eq 0 ] && quiet=1 || quiet=0
> 
>  ##

Fine with me.

Acked-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

-- 
~Randy

Re: [PATCH] test-suite: handle format with filename.c not existing

[Luc Van Oostenryck]

On Tue, Jan 23, 2018 at 10:20:55AM -0800, Randy Dunlap wrote:
> 
> At the beginning of the file, $must_fail is not defined, so it will not -eq 1,
> so quiet will remain = 0.
> 
> I guess that I don't understand the interplay between $V and $quiet and why
> have both of them.

$V is something global, set once for all when starting the test suite.
$quiet is something that can change for each file being tested.

> Are you suggesting that $quiet should be initialized by default like this:
> (just drop the $must_fail part)
> 
> > quiet=0
> > [ $V -eq 0 ] && quiet=1

The whole can probably be simplified a bit but the idea with
$must_fail (set for test cases we known/expected to fail) is
that we're not interested in any sort of details about the
failure of a test that is supposed to fail.

Regards,
-- Luc

Re: [PATCH] test-suite: handle format with filename.c not existing

[Luc Van Oostenryck]

On Mon, Jan 22, 2018 at 11:05:09AM -0800, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
> 
> In error(), quiet is undefined when the command:
>   $ ./test-suite format filename1.c foobar1
> 
> is used and filename1.c does not exist. This causes a shell script error:
>   ./test-suite: line 147: [: : integer expression expected
> 
> because $quiet is undefined in
>   [ "$quiet" -ne 1 ] && echo "error: $1"
> so the error message is lost.
> 
> Just initialize quiet before any command line parsing.
> 
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>

cfr: https://marc.info/?l=linux-sparse&m=151273892009415

-- Luc

Re: [PATCH] test-suite: handle format with filename.c not existing

[Randy Dunlap]

On 01/24/2018 12:28 AM, Luc Van Oostenryck wrote:
> On Mon, Jan 22, 2018 at 11:05:09AM -0800, Randy Dunlap wrote:
>> From: Randy Dunlap <rdunlap@infradead.org>
>>
>> In error(), quiet is undefined when the command:
>>   $ ./test-suite format filename1.c foobar1
>>
>> is used and filename1.c does not exist. This causes a shell script error:
>>   ./test-suite: line 147: [: : integer expression expected
>>
>> because $quiet is undefined in
>>   [ "$quiet" -ne 1 ] && echo "error: $1"
>> so the error message is lost.
>>
>> Just initialize quiet before any command line parsing.
>>
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> 
> cfr: https://marc.info/?l=linux-sparse&m=151273892009415

I don't think that will fix the original problem that I posted the
patch for.


-- 
~Randy

Re: [PATCH] test-suite: handle format with filename.c not existing

[Christopher Li]

On Wed, Jan 24, 2018 at 9:48 AM, Randy Dunlap <rdunlap@infradead.org> wrote:
> I don't think that will fix the original problem that I posted the
> patch for.

Agree, as long as the $quiet is set inside do_test(), any call to
error() out side
of do_test() will cause error. That is the original problem.

Chris

Re: [PATCH] test-suite: handle format with filename.c not existing

[Luc Van Oostenryck]

On Wed, Jan 24, 2018 at 09:48:07AM -0800, Randy Dunlap wrote:
> On 01/24/2018 12:28 AM, Luc Van Oostenryck wrote:
> > On Mon, Jan 22, 2018 at 11:05:09AM -0800, Randy Dunlap wrote:
> >> From: Randy Dunlap <rdunlap@infradead.org>
> >>
> >> In error(), quiet is undefined when the command:
> >>   $ ./test-suite format filename1.c foobar1
> >>
> >> is used and filename1.c does not exist. This causes a shell script error:
> >>   ./test-suite: line 147: [: : integer expression expected
> >>
> >> because $quiet is undefined in
> >>   [ "$quiet" -ne 1 ] && echo "error: $1"
> >> so the error message is lost.
> >>
> >> Just initialize quiet before any command line parsing.
> >>
> >> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> > 
> > cfr: https://marc.info/?l=linux-sparse&m=151273892009415
> 
> I don't think that will fix the original problem that I posted the
> patch for.

Yes, you're right.
The original problem doesn't exist in my tree but for other reasons.

-- Luc

Re: [PATCH] test-suite: handle format with filename.c not existing

[Christopher Li]

On Tue, Jan 23, 2018 at 5:38 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>> diff --git a/validation/test-suite b/validation/test-suite
>> index cf151a3..5e10942 100755
>> --- a/validation/test-suite
>> +++ b/validation/test-suite
>> @@ -31,6 +31,7 @@ known_ko_tests=0
>>
>>  # defaults to not verbose
>>  [ -z "$V" ] && V=0
>> +[ $V -eq 0 ] && quiet=1 || quiet=0
>>
>>  ##
>
> Fine with me.
>
> Acked-by: Randy Dunlap <rdunlap@infradead.org>
> Tested-by: Randy Dunlap <rdunlap@infradead.org>
>
I consider that as a minor tweak of your original patch.
May I apply that change and keep you as the author of the
patch?

Thanks

Chris

Re: [PATCH] test-suite: handle format with filename.c not existing

[Luc Van Oostenryck]

On Wed, Jan 24, 2018 at 08:19:23PM +0100, Luc Van Oostenryck wrote:
> On Wed, Jan 24, 2018 at 09:48:07AM -0800, Randy Dunlap wrote:
> > On 01/24/2018 12:28 AM, Luc Van Oostenryck wrote:
> > > On Mon, Jan 22, 2018 at 11:05:09AM -0800, Randy Dunlap wrote:
> > >> From: Randy Dunlap <rdunlap@infradead.org>
> > >>
> > >> In error(), quiet is undefined when the command:
> > >>   $ ./test-suite format filename1.c foobar1
> > >>
> > >> is used and filename1.c does not exist. This causes a shell script error:
> > >>   ./test-suite: line 147: [: : integer expression expected
> > >>
> > >> because $quiet is undefined in
> > >>   [ "$quiet" -ne 1 ] && echo "error: $1"
> > >> so the error message is lost.
> > >>
> > >> Just initialize quiet before any command line parsing.
> > >>
> > >> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> > > 
> > > cfr: https://marc.info/?l=linux-sparse&m=151273892009415
> > 
> > I don't think that will fix the original problem that I posted the
> > patch for.
> 
> Yes, you're right.
> The original problem doesn't exist in my tree but for another reason.

Nevertheless, the reason explained in my patch is still valid
and $quiet also need to be reset early in do_test().

-- Luc

Re: [PATCH] test-suite: handle format with filename.c not existing

[Randy Dunlap]

On 01/24/2018 11:19 AM, Christopher Li wrote:
> On Tue, Jan 23, 2018 at 5:38 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>>> diff --git a/validation/test-suite b/validation/test-suite
>>> index cf151a3..5e10942 100755
>>> --- a/validation/test-suite
>>> +++ b/validation/test-suite
>>> @@ -31,6 +31,7 @@ known_ko_tests=0
>>>
>>>  # defaults to not verbose
>>>  [ -z "$V" ] && V=0
>>> +[ $V -eq 0 ] && quiet=1 || quiet=0
>>>
>>>  ##
>>
>> Fine with me.
>>
>> Acked-by: Randy Dunlap <rdunlap@infradead.org>
>> Tested-by: Randy Dunlap <rdunlap@infradead.org>
>>
> I consider that as a minor tweak of your original patch.
> May I apply that change and keep you as the author of the
> patch?

Sure, no problem.

Thanks.

-- 
~Randy