public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [RFC PATCH] test.sh: add SHOULD_PASS, SHOULD_FAIL functions
@ 2016-08-19 14:43 Stanislav Kholmanskikh
  2016-08-22 12:31 ` Cyril Hrubis
  0 siblings, 1 reply; 3+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-19 14:43 UTC (permalink / raw)
  To: ltp

Sometimes we need to execute a command and call tst_resm TPASS/TFAIL
based on the command's exit status.

The existing ROD() function can make 99% of the job, we just
need to let it know how the command's exit code should be
interpreted. This patch does it and introduce a couple of new
functions to help with the described situation.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
This is to help with situations like:

echo 1.0 > memory.limit_in_bytes 2> /dev/null
if [ $? -ne 0 ]; then
   tst_resm TPASS "return value is $?"
else
   tst_resm TFAIL "return value is 0"
fi

which could be transformed to:

SHOULD_FAIL echo 1.0 \> memory.limit_in_bytes


 testcases/lib/test.sh |   50 +++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/testcases/lib/test.sh b/testcases/lib/test.sh
index bd66109..ca2c00b 100644
--- a/testcases/lib/test.sh
+++ b/testcases/lib/test.sh
@@ -220,12 +220,17 @@ ROD_SILENT()
 	fi
 }
 
-ROD()
+ROD_DISPATCHER()
 {
+	local act
 	local cmd
 	local arg
 	local file
 	local flag
+	local ret
+
+	act="$1"
+	shift
 
 	for arg; do
 		file="${arg#\>}"
@@ -251,9 +256,46 @@ ROD()
 		$@
 	fi
 
-	if [ $? -ne 0 ]; then
-		tst_brkm TBROK "$@ failed"
-	fi
+	ret=$?
+
+	case "$act" in
+		0) # break on failure
+		if [ $ret -ne 0 ]; then
+			tst_brkm TBROK "$@ failed"
+		fi;;
+
+		1) # the command should pass
+		if [ $ret -eq 0 ]; then
+			tst_resm TPASS "$@ passed as expected"
+		else
+			tst_resm TFAIL "$@ failed unexpectedly"
+		fi;;
+
+		2) # the command should fail
+		if [ $ret -ne 0 ]; then
+			tst_resm TPASS "$@ failed as expected"
+		else
+			tst_resm TFAIL "$@ passed unexpectedly"
+		fi;;
+
+		*) tst_brkm TBROK "unknown action '$act'";;
+	esac
+}
+
+ROD()
+{
+	ROD_DISPATCHER 0 $@
+}
+
+SHOULD_PASS()
+{
+	ROD_DISPATCHER 1 $@
+}
+
+SHOULD_FAIL()
+{
+	# redirect stderr since we expect the command to fail
+	ROD_DISPATCHER 2 $@ 2> /dev/null
 }
 
 tst_acquire_device()
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [LTP] [RFC PATCH] test.sh: add SHOULD_PASS, SHOULD_FAIL functions
  2016-08-19 14:43 [LTP] [RFC PATCH] test.sh: add SHOULD_PASS, SHOULD_FAIL functions Stanislav Kholmanskikh
@ 2016-08-22 12:31 ` Cyril Hrubis
  2016-08-22 12:51   ` Stanislav Kholmanskikh
  0 siblings, 1 reply; 3+ messages in thread
From: Cyril Hrubis @ 2016-08-22 12:31 UTC (permalink / raw)
  To: ltp

Hi!
> Sometimes we need to execute a command and call tst_resm TPASS/TFAIL
> based on the command's exit status.
> 
> The existing ROD() function can make 99% of the job, we just
> need to let it know how the command's exit code should be
> interpreted. This patch does it and introduce a couple of new
> functions to help with the described situation.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
> This is to help with situations like:
> 
> echo 1.0 > memory.limit_in_bytes 2> /dev/null
> if [ $? -ne 0 ]; then
>    tst_resm TPASS "return value is $?"
> else
>    tst_resm TFAIL "return value is 0"
> fi
> 
> which could be transformed to:
> 
> SHOULD_FAIL echo 1.0 \> memory.limit_in_bytes

This is quite nice. Maybe it would better be named EXPECT_FAIL and
EXPECT_PASS or something but SHOULD_ prefix is fine as well.

We should include documentation for these in this commit as well.

A few comments to the implementation below.

>  testcases/lib/test.sh |   50 +++++++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 46 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/lib/test.sh b/testcases/lib/test.sh
> index bd66109..ca2c00b 100644
> --- a/testcases/lib/test.sh
> +++ b/testcases/lib/test.sh
> @@ -220,12 +220,17 @@ ROD_SILENT()
>  	fi
>  }
>  
> -ROD()
> +ROD_DISPATCHER()
>  {
> +	local act
>  	local cmd
>  	local arg
>  	local file
>  	local flag
> +	local ret
> +
> +	act="$1"
> +	shift
>  
>  	for arg; do
>  		file="${arg#\>}"
> @@ -251,9 +256,46 @@ ROD()
>  		$@
>  	fi
>  
> -	if [ $? -ne 0 ]; then
> -		tst_brkm TBROK "$@ failed"
> -	fi
> +	ret=$?

Why don't we just return from this function here and let the caller
examine the $?

> +	case "$act" in
> +		0) # break on failure
> +		if [ $ret -ne 0 ]; then
> +			tst_brkm TBROK "$@ failed"
> +		fi;;
> +
> +		1) # the command should pass
> +		if [ $ret -eq 0 ]; then
> +			tst_resm TPASS "$@ passed as expected"
> +		else
> +			tst_resm TFAIL "$@ failed unexpectedly"
> +		fi;;
> +
> +		2) # the command should fail
> +		if [ $ret -ne 0 ]; then
> +			tst_resm TPASS "$@ failed as expected"
> +		else
> +			tst_resm TFAIL "$@ passed unexpectedly"
> +		fi;;
> +
> +		*) tst_brkm TBROK "unknown action '$act'";;
> +	esac
> +}
> +
> +ROD()
> +{
> +	ROD_DISPATCHER 0 $@
> +}
> +
> +SHOULD_PASS()
> +{
> +	ROD_DISPATCHER 1 $@
> +}
> +
> +SHOULD_FAIL()
> +{
> +	# redirect stderr since we expect the command to fail
> +	ROD_DISPATCHER 2 $@ 2> /dev/null
>  }

I think that we should quote the $@ in these functions.

Try for yourself:

8<------------------------------------

#!/bin/sh

d()
{
        echo "-------------"
        for i; do
                echo $i
        done
        echo "-------------"
}

dd()
{
        d "$@"
}


d a b c
dd a b c
d "a b c"
dd "a b c"

8<------------------------------------

If you unquote the $@ in dd() the "a b c" in the last call would be separated
on spaces.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [LTP] [RFC PATCH] test.sh: add SHOULD_PASS, SHOULD_FAIL functions
  2016-08-22 12:31 ` Cyril Hrubis
@ 2016-08-22 12:51   ` Stanislav Kholmanskikh
  0 siblings, 0 replies; 3+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-22 12:51 UTC (permalink / raw)
  To: ltp



On 08/22/2016 03:31 PM, Cyril Hrubis wrote:
> Hi!
>> Sometimes we need to execute a command and call tst_resm TPASS/TFAIL
>> based on the command's exit status.
>>
>> The existing ROD() function can make 99% of the job, we just
>> need to let it know how the command's exit code should be
>> interpreted. This patch does it and introduce a couple of new
>> functions to help with the described situation.
>>
>> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
>> ---
>> This is to help with situations like:
>>
>> echo 1.0 > memory.limit_in_bytes 2> /dev/null
>> if [ $? -ne 0 ]; then
>>    tst_resm TPASS "return value is $?"
>> else
>>    tst_resm TFAIL "return value is 0"
>> fi
>>
>> which could be transformed to:
>>
>> SHOULD_FAIL echo 1.0 \> memory.limit_in_bytes
> 
> This is quite nice. Maybe it would better be named EXPECT_FAIL and
> EXPECT_PASS or something but SHOULD_ prefix is fine as well.

Ok, will rename to EXPECT_*.

> 
> We should include documentation for these in this commit as well.

Ok.

> 
> A few comments to the implementation below.
> 
>>  testcases/lib/test.sh |   50 +++++++++++++++++++++++++++++++++++++++++++++---
>>  1 files changed, 46 insertions(+), 4 deletions(-)
>>
>> diff --git a/testcases/lib/test.sh b/testcases/lib/test.sh
>> index bd66109..ca2c00b 100644
>> --- a/testcases/lib/test.sh
>> +++ b/testcases/lib/test.sh
>> @@ -220,12 +220,17 @@ ROD_SILENT()
>>  	fi
>>  }
>>  
>> -ROD()
>> +ROD_DISPATCHER()
>>  {
>> +	local act
>>  	local cmd
>>  	local arg
>>  	local file
>>  	local flag
>> +	local ret
>> +
>> +	act="$1"
>> +	shift
>>  
>>  	for arg; do
>>  		file="${arg#\>}"
>> @@ -251,9 +256,46 @@ ROD()
>>  		$@
>>  	fi
>>  
>> -	if [ $? -ne 0 ]; then
>> -		tst_brkm TBROK "$@ failed"
>> -	fi
>> +	ret=$?
> 
> Why don't we just return from this function here and let the caller
> examine the $?

No specific reason. Will put the handling of $? into the callers of
ROD_DISPATCHER, and rename ROD_DISPATCHER to something like ROD_BASE,
since now it doesn't "dispatch".

> 
>> +	case "$act" in
>> +		0) # break on failure
>> +		if [ $ret -ne 0 ]; then
>> +			tst_brkm TBROK "$@ failed"
>> +		fi;;
>> +
>> +		1) # the command should pass
>> +		if [ $ret -eq 0 ]; then
>> +			tst_resm TPASS "$@ passed as expected"
>> +		else
>> +			tst_resm TFAIL "$@ failed unexpectedly"
>> +		fi;;
>> +
>> +		2) # the command should fail
>> +		if [ $ret -ne 0 ]; then
>> +			tst_resm TPASS "$@ failed as expected"
>> +		else
>> +			tst_resm TFAIL "$@ passed unexpectedly"
>> +		fi;;
>> +
>> +		*) tst_brkm TBROK "unknown action '$act'";;
>> +	esac
>> +}
>> +
>> +ROD()
>> +{
>> +	ROD_DISPATCHER 0 $@
>> +}
>> +
>> +SHOULD_PASS()
>> +{
>> +	ROD_DISPATCHER 1 $@
>> +}
>> +
>> +SHOULD_FAIL()
>> +{
>> +	# redirect stderr since we expect the command to fail
>> +	ROD_DISPATCHER 2 $@ 2> /dev/null
>>  }
> 
> I think that we should quote the $@ in these functions.
> 
> Try for yourself:
> 
> 8<------------------------------------
> 
> #!/bin/sh
> 
> d()
> {
>         echo "-------------"
>         for i; do
>                 echo $i
>         done
>         echo "-------------"
> }
> 
> dd()
> {
>         d "$@"
> }
> 
> 
> d a b c
> dd a b c
> d "a b c"
> dd "a b c"
> 
> 8<------------------------------------
> 
> If you unquote the $@ in dd() the "a b c" in the last call would be separated
> on spaces.
> 

Got it. Thank you.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-08-22 12:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-19 14:43 [LTP] [RFC PATCH] test.sh: add SHOULD_PASS, SHOULD_FAIL functions Stanislav Kholmanskikh
2016-08-22 12:31 ` Cyril Hrubis
2016-08-22 12:51   ` Stanislav Kholmanskikh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox