RE: [PATCH v3 5/5] kselftest: kcmp: Port kcmp test to TAP v13

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Alice Ferrazz on Thursday, June 15, 2017 7:33 AM
> 
> Make the kcmp test output in the TAP13 format by using the
> TAP13 output functions defined in kselftest.h
> 
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Paul Elder <paul.elder@xxxxxxxx>
> Signed-off-by: Alice Ferrazzi <alice.ferrazzi@xxxxxxxxx>
> ---
> v3: Add kcmp test port to TAP13 - Alice
> 
>  tools/testing/selftests/kcmp/kcmp_test.c | 33 ++++++++++++++-------------
> -----
>  1 file changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/testing/selftests/kcmp/kcmp_test.c
> b/tools/testing/selftests/kcmp/kcmp_test.c
> index a5a4da856d..b007e62e03 100644
> --- a/tools/testing/selftests/kcmp/kcmp_test.c
> +++ b/tools/testing/selftests/kcmp/kcmp_test.c
> @@ -35,30 +35,28 @@ int main(int argc, char **argv)
>  	pid1 = getpid();
> 
>  	if (fd1 < 0) {
> -		perror("Can't create file");
> -		ksft_exit_fail();
> +		ksft_exit_fail_msg("Can't create file");
>  	}
> 
>  	pid2 = fork();
>  	if (pid2 < 0) {
> -		perror("fork failed");
> -		ksft_exit_fail();
> +		ksft_exit_fail_msg("fork failed");
>  	}
> 
>  	if (!pid2) {
>  		int pid2 = getpid();
>  		int ret;
> +		char buf[512];
> 
>  		fd2 = open(kpath, O_RDWR, 0644);
>  		if (fd2 < 0) {
> -			perror("Can't open file");
> -			ksft_exit_fail();
> +			ksft_exit_fail_msg("Can't open file");
>  		}
> 
>  		/* An example of output and arguments */
> -		printf("pid1: %6d pid2: %6d FD: %2ld FILES: %2ld VM: %2ld "
> +		snprintf(&buf[0], sizeof(buf) ,"pid1: %6d pid2: %6d FD: %2ld
> FILES: %2ld VM: %2ld "
>  		       "FS: %2ld SIGHAND: %2ld IO: %2ld SYSVSEM: %2ld "
> -		       "INV: %2ld\n",
> +		       "INV: %2ld",
>  		       pid1, pid2,
>  		       sys_kcmp(pid1, pid2, KCMP_FILE,		fd1, fd2),
>  		       sys_kcmp(pid1, pid2, KCMP_FILES,		0, 0),
> @@ -73,30 +71,27 @@ int main(int argc, char **argv)
> 
>  		/* This one should return same fd */
>  		ret = sys_kcmp(pid1, pid2, KCMP_FILE, fd1, fd1);
> +		ksft_test_result_pass(buf);
This uses a string with all kinds of dynamic data (particularly pid
values) as the test description.  Please use only non-dynamic
strings for test descriptions.  If it is desired to output information about
the data used for the test, that should be in a YAML block.

>  		if (ret) {
> -			printf("FAIL: 0 expected but %d returned (%s)\n",
> +			snprintf(&buf[0], sizeof(buf), "0 expected but %d
> returned (%s)",
>  				ret, strerror(errno));
> -			ksft_inc_fail_cnt();
> +			ksft_test_result_fail(buf);
This is using test results in place of the test description.  Please output
test results information and explanation AFTER the test description line.

>  			ret = -1;
>  		} else {
> -			printf("PASS: 0 returned as expected\n");
> -			ksft_inc_pass_cnt();
> +			ksft_test_result_pass("0 returned as expected");
This is not a good test description.  It describes results rather than the
test case.

>  		}
> 
>  		/* Compare with self */
>  		ret = sys_kcmp(pid1, pid1, KCMP_VM, 0, 0);
>  		if (ret) {
> -			printf("FAIL: 0 expected but %d returned (%s)\n",
> +			snprintf(&buf[0], sizeof(buf), "0 expected but %d
> returned (%s)",
>  				ret, strerror(errno));
> -			ksft_inc_fail_cnt();
> +			ksft_test_result_fail(buf);
Same issue as above here.  Please do not use dynamic strings with results
as the test description.

>  			ret = -1;
>  		} else {
> -			printf("PASS: 0 returned as expected\n");
> -			ksft_inc_pass_cnt();
> +			ksft_test_result_pass("0 returned as expected");
Same issue here.
>  		}
> 
> -		ksft_print_cnts();
> -
>  		if (ret)
>  			ksft_exit_fail();
>  		else
> @@ -105,5 +100,5 @@ int main(int argc, char **argv)
> 
>  	waitpid(pid2, &status, P_ALL);
> 
> -	return ksft_exit_pass();
> +	return 0;
The return should be removed, and this should be just a call to:
ksft_exit_pass();

Or, this should be left alone.  Shuah said she would review these
calls to see if the return was still warranted.

The ksft_exit_pass() routine is there to provide a level of abstraction
(e.g. in case the exit codes change in the future), so it should not
be removed.
 -- Tim


>  }
> --
> 2.13.0
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux