Subject: Re: [PATCH v2] selftests: kcmp: convert to TAP13 output
To: Shuah Khan <shuahkh@osg.samsung.com>, shuah@kernel.org
Cc: linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
        Tim Bird <Tim.Bird@sony.com>
References: <20170630234758.348-1-shuahkh@osg.samsung.com>
From: Paul Elder <paul.elder@pitt.edu>
Message-ID: <9dc7074d-9c7e-0b64-a41a-2f4ceebd0761@pitt.edu>
Date: Sat, 1 Jul 2017 11:27:38 +0900
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <20170630234758.348-1-shuahkh@osg.samsung.com>
Content-Type: text/plain; charset=windows-1252
Content-Language: en-US
Content-Transfer-Encoding: 8bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Jul 2017 02:27:52.4587 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN2PR04MB2176
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 3555
Lines: 119

On 07/01/2017 08:47 AM, Shuah Khan wrote:
> Convert to TAP13 output using ksft_ api. Child runs tests, increments test
> counters, and prints test results.
> 
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
> 
> Changes since v1:
> - Add ksft_print_header()
> 
>  tools/testing/selftests/kcmp/kcmp_test.c | 48 +++++++++++++-------------------
>  1 file changed, 19 insertions(+), 29 deletions(-)
> 
> diff --git a/tools/testing/selftests/kcmp/kcmp_test.c b/tools/testing/selftests/kcmp/kcmp_test.c
> index a5a4da856dfe..563018d81c45 100644
> --- a/tools/testing/selftests/kcmp/kcmp_test.c
> +++ b/tools/testing/selftests/kcmp/kcmp_test.c
> @@ -34,16 +34,14 @@ int main(int argc, char **argv)
>  	fd1 = open(kpath, O_RDWR | O_CREAT | O_TRUNC, 0644);
>  	pid1 = getpid();
>  
> -	if (fd1 < 0) {
> -		perror("Can't create file");
> -		ksft_exit_fail();
> -	}
> +	ksft_print_header();
> +
> +	if (fd1 < 0)
> +		ksft_exit_fail_msg("Can't create file: %s\n", strerror(errno));
>  
>  	pid2 = fork();
> -	if (pid2 < 0) {
> -		perror("fork failed");
> -		ksft_exit_fail();
> -	}
> +	if (pid2 < 0)
> +		ksft_exit_fail_msg("fork() failed: %s\n", strerror(errno));
>  
>  	if (!pid2) {
>  		int pid2 = getpid();
> @@ -51,14 +49,14 @@ int main(int argc, char **argv)
>  
>  		fd2 = open(kpath, O_RDWR, 0644);
>  		if (fd2 < 0) {
> -			perror("Can't open file");
> -			ksft_exit_fail();
> +			ksft_print_msg("Can't open file: %s\n",
> +				strerror(errno));
> +			exit(KSFT_FAIL);
>  		}
>  
>  		/* An example of output and arguments */
> -		printf("pid1: %6d pid2: %6d FD: %2ld FILES: %2ld VM: %2ld "
> -		       "FS: %2ld SIGHAND: %2ld IO: %2ld SYSVSEM: %2ld "
> -		       "INV: %2ld\n",
> +		ksft_print_msg(
> +			"pid1: %6d pid2: %6d FD: %2ld\n  FILES: %2ld VM: %2ld FS: %2ld SIGHAND: %2ld\n  IO: %2ld SYSVSEM: %2ld INV: %2ld\n",
Is it okay that there's no # after the newlines? Will that confuse test output
parsers?

>  		       pid1, pid2,
>  		       sys_kcmp(pid1, pid2, KCMP_FILE,		fd1, fd2),
>  		       sys_kcmp(pid1, pid2, KCMP_FILES,		0, 0),
> @@ -74,28 +72,22 @@ int main(int argc, char **argv)
>  		/* This one should return same fd */
>  		ret = sys_kcmp(pid1, pid2, KCMP_FILE, fd1, fd1);
>  		if (ret) {
> -			printf("FAIL: 0 expected but %d returned (%s)\n",
> +			ksft_test_result_fail(
> +				"0 expected but %d returned (%s)\n",
>  				ret, strerror(errno));
> -			ksft_inc_fail_cnt();
>  			ret = -1;
> -		} else {
> -			printf("PASS: 0 returned as expected\n");
> -			ksft_inc_pass_cnt();
> -		}
> +		} else
> +			ksft_test_result_pass("0 returned as expected\n");
I remember Tim Bird mentioning before that the test descriptions should be
non-dynamic to not confuse diffs. What did we decide on about that?

Also specifically with this test output (seems like Tim had a similar
comment before), the output doesn't really describe the test.


Thanks,

Paul

>  
>  		/* Compare with self */
>  		ret = sys_kcmp(pid1, pid1, KCMP_VM, 0, 0);
>  		if (ret) {
> -			printf("FAIL: 0 expected but %d returned (%s)\n",
> +			ksft_test_result_fail(
> +				"0 expected but %d returned (%s)\n",
>  				ret, strerror(errno));
> -			ksft_inc_fail_cnt();
>  			ret = -1;
> -		} else {
> -			printf("PASS: 0 returned as expected\n");
> -			ksft_inc_pass_cnt();
> -		}
> -
> -		ksft_print_cnts();
> +		} else
> +			ksft_test_result_pass("0 returned as expected\n");
>  
>  		if (ret)
>  			ksft_exit_fail();
> @@ -104,6 +96,4 @@ int main(int argc, char **argv)
>  	}
>  
>  	waitpid(pid2, &status, P_ALL);
> -
> -	return ksft_exit_pass();
>  }
>