[PATCH] selftests: futex: convert test to use ksft TAP13 framework

[PATCH] selftests: futex: convert test to use ksft TAP13 framework

[Shuah Khan]

Convert test to use ksft TAP13 framework.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 .../selftests/futex/functional/futex_requeue_pi.c    |  8 +++++---
 .../functional/futex_requeue_pi_mismatched_ops.c     |  3 ++-
 .../functional/futex_requeue_pi_signal_restart.c     |  5 +++--
 .../functional/futex_wait_private_mapped_file.c      |  6 ++++--
 .../selftests/futex/functional/futex_wait_timeout.c  |  5 +++--
 .../futex/functional/futex_wait_uninitialized_heap.c |  3 ++-
 .../futex/functional/futex_wait_wouldblock.c         |  3 ++-
 tools/testing/selftests/futex/include/logging.h      | 20 +++++++++-----------
 8 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/tools/testing/selftests/futex/functional/futex_requeue_pi.c b/tools/testing/selftests/futex/functional/futex_requeue_pi.c
index d24ab7421e73..54cd5c414e82 100644
--- a/tools/testing/selftests/futex/functional/futex_requeue_pi.c
+++ b/tools/testing/selftests/futex/functional/futex_requeue_pi.c
@@ -394,9 +394,11 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	printf("%s: Test requeue functionality\n", basename(argv[0]));
-	printf("\tArguments: broadcast=%d locked=%d owner=%d timeout=%ldns\n",
-	       broadcast, locked, owner, timeout_ns);
+	ksft_print_header();
+	ksft_print_msg("%s: Test requeue functionality\n", basename(argv[0]));
+	ksft_print_msg(
+		"\tArguments: broadcast=%d locked=%d owner=%d timeout=%ldns\n",
+		broadcast, locked, owner, timeout_ns);
 
 	/*
 	 * FIXME: unit_test is obsolete now that we parse options and the
diff --git a/tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c b/tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c
index e0a798ad0d21..08187a16507f 100644
--- a/tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c
+++ b/tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c
@@ -78,7 +78,8 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	printf("%s: Detect mismatched requeue_pi operations\n",
+	ksft_print_header();
+	ksft_print_msg("%s: Detect mismatched requeue_pi operations\n",
 	       basename(argv[0]));
 
 	if (pthread_create(&child, NULL, blocking_child, NULL)) {
diff --git a/tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c b/tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c
index 982f83577501..f0542a344d95 100644
--- a/tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c
+++ b/tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c
@@ -143,9 +143,10 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	printf("%s: Test signal handling during requeue_pi\n",
+	ksft_print_header();
+	ksft_print_msg("%s: Test signal handling during requeue_pi\n",
 	       basename(argv[0]));
-	printf("\tArguments: <none>\n");
+	ksft_print_msg("\tArguments: <none>\n");
 
 	sa.sa_handler = handle_signal;
 	sigemptyset(&sa.sa_mask);
diff --git a/tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c b/tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c
index bdc48dc047e5..6216de828093 100644
--- a/tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c
+++ b/tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c
@@ -97,8 +97,10 @@ int main(int argc, char **argv)
 		}
 	}
 
-	printf("%s: Test the futex value of private file mappings in FUTEX_WAIT\n",
-	       basename(argv[0]));
+	ksft_print_header();
+	ksft_print_msg(
+		"%s: Test the futex value of private file mappings in FUTEX_WAIT\n",
+		basename(argv[0]));
 
 	ret = pthread_create(&thr, NULL, thr_futex_wait, NULL);
 	if (ret < 0) {
diff --git a/tools/testing/selftests/futex/functional/futex_wait_timeout.c b/tools/testing/selftests/futex/functional/futex_wait_timeout.c
index 6aadd560366e..bab3dfe1787f 100644
--- a/tools/testing/selftests/futex/functional/futex_wait_timeout.c
+++ b/tools/testing/selftests/futex/functional/futex_wait_timeout.c
@@ -68,9 +68,10 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	printf("%s: Block on a futex and wait for timeout\n",
+	ksft_print_header();
+	ksft_print_msg("%s: Block on a futex and wait for timeout\n",
 	       basename(argv[0]));
-	printf("\tArguments: timeout=%ldns\n", timeout_ns);
+	ksft_print_msg("\tArguments: timeout=%ldns\n", timeout_ns);
 
 	/* initialize timeout */
 	to.tv_sec = 0;
diff --git a/tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c b/tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c
index d237a8b702f0..26975322545b 100644
--- a/tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c
+++ b/tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c
@@ -99,7 +99,8 @@ int main(int argc, char **argv)
 		exit(1);
 	}
 
-	printf("%s: Test the uninitialized futex value in FUTEX_WAIT\n",
+	ksft_print_header();
+	ksft_print_msg("%s: Test the uninitialized futex value in FUTEX_WAIT\n",
 	       basename(argv[0]));
 
 
diff --git a/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c b/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c
index 9a2c56fa7305..da15a63269b4 100644
--- a/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c
+++ b/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c
@@ -64,7 +64,8 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	printf("%s: Test the unexpected futex value in FUTEX_WAIT\n",
+	ksft_print_header();
+	ksft_print_msg("%s: Test the unexpected futex value in FUTEX_WAIT\n",
 	       basename(argv[0]));
 
 	info("Calling futex_wait on f1: %u @ %p with val=%u\n", f1, &f1, f1+1);
diff --git a/tools/testing/selftests/futex/include/logging.h b/tools/testing/selftests/futex/include/logging.h
index 4e7944984fbb..01989644e50a 100644
--- a/tools/testing/selftests/futex/include/logging.h
+++ b/tools/testing/selftests/futex/include/logging.h
@@ -109,22 +109,20 @@ void log_verbosity(int level)
  */
 void print_result(const char *test_name, int ret)
 {
-	const char *result = "Unknown return code";
-
 	switch (ret) {
 	case RET_PASS:
-		ksft_inc_pass_cnt();
-		result = PASS;
-		break;
+		ksft_test_result_pass("%s\n", test_name);
+		ksft_print_cnts();
+		return;
 	case RET_ERROR:
-		result = ERROR;
-		break;
+		ksft_test_result_error("%s\n", test_name);
+		ksft_print_cnts();
+		return;
 	case RET_FAIL:
-		ksft_inc_fail_cnt();
-		result = FAIL;
-		break;
+		ksft_test_result_fail("%s\n", test_name);
+		ksft_print_cnts();
+		return;
 	}
-	printf("selftests: %s [%s]\n", test_name, result);
 }
 
 /* log level macros */
-- 
2.11.0

Re: [PATCH] selftests: futex: convert test to use ksft TAP13 framework

[Stafford Horne]

On Fri, Aug 04, 2017 at 06:46:50PM -0600, Shuah Khan wrote:
> Convert test to use ksft TAP13 framework.


> --- a/tools/testing/selftests/futex/include/logging.h
> +++ b/tools/testing/selftests/futex/include/logging.h
> @@ -109,22 +109,20 @@ void log_verbosity(int level)
>   */
>  void print_result(const char *test_name, int ret)
>  {
> -	const char *result = "Unknown return code";
> -
>  	switch (ret) {
>  	case RET_PASS:
> -		ksft_inc_pass_cnt();
> -		result = PASS;
> -		break;
> +		ksft_test_result_pass("%s\n", test_name);
> +		ksft_print_cnts();
> +		return;
>  	case RET_ERROR:
> -		result = ERROR;
> -		break;
> +		ksft_test_result_error("%s\n", test_name);
> +		ksft_print_cnts();
> +		return;

This looks ok to me, but this function `ksft_test_result_error` was added
in another patch you just sent, it was a bit hard to review.

If you need to make and updates, could you please send a series next?

-Stafford

>  	case RET_FAIL:
> -		ksft_inc_fail_cnt();
> -		result = FAIL;
> -		break;
> +		ksft_test_result_fail("%s\n", test_name);
> +		ksft_print_cnts();
> +		return;
>  	}
> -	printf("selftests: %s [%s]\n", test_name, result);

Re: [PATCH] selftests: futex: convert test to use ksft TAP13 framework

[Shuah Khan]

On 08/05/2017 09:15 AM, Stafford Horne wrote:
> On Fri, Aug 04, 2017 at 06:46:50PM -0600, Shuah Khan wrote:
>> Convert test to use ksft TAP13 framework.
> 
> 
>> --- a/tools/testing/selftests/futex/include/logging.h
>> +++ b/tools/testing/selftests/futex/include/logging.h
>> @@ -109,22 +109,20 @@ void log_verbosity(int level)
>>   */
>>  void print_result(const char *test_name, int ret)
>>  {
>> -	const char *result = "Unknown return code";
>> -
>>  	switch (ret) {
>>  	case RET_PASS:
>> -		ksft_inc_pass_cnt();
>> -		result = PASS;
>> -		break;
>> +		ksft_test_result_pass("%s\n", test_name);
>> +		ksft_print_cnts();
>> +		return;
>>  	case RET_ERROR:
>> -		result = ERROR;
>> -		break;
>> +		ksft_test_result_error("%s\n", test_name);
>> +		ksft_print_cnts();
>> +		return;
> 
> This looks ok to me, but this function `ksft_test_result_error` was added
> in another patch you just sent, it was a bit hard to review.

thanks for the review.

> 
> If you need to make and updates, could you please send a series next?

I should have sent it as a series in the first place. Sorry about that.

> 
> -Stafford
> 
>>  	case RET_FAIL:
>> -		ksft_inc_fail_cnt();
>> -		result = FAIL;
>> -		break;
>> +		ksft_test_result_fail("%s\n", test_name);
>> +		ksft_print_cnts();
>> +		return;
>>  	}
>> -	printf("selftests: %s [%s]\n", test_name, result);
> 
> 
> 

thanks,
-- Shuah

Re: [PATCH] selftests: futex: convert test to use ksft TAP13 framework

[Darren Hart]

On Fri, Aug 04, 2017 at 06:46:50PM -0600, Shuah Khan wrote:
> Convert test to use ksft TAP13 framework.
> 

Please include some information in the commit about why we're doing this
and what it is.

>From what I can tell this is converting raw printk to a wrapped set of
logging functions to produce a more consistent output across the
kselftest suite.

I have no objection to that for the futex tests:

Acked-by: Darren Hart (VMware) <dvhart@infradead.org>


> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
>  .../selftests/futex/functional/futex_requeue_pi.c    |  8 +++++---
>  .../functional/futex_requeue_pi_mismatched_ops.c     |  3 ++-
>  .../functional/futex_requeue_pi_signal_restart.c     |  5 +++--
>  .../functional/futex_wait_private_mapped_file.c      |  6 ++++--
>  .../selftests/futex/functional/futex_wait_timeout.c  |  5 +++--
>  .../futex/functional/futex_wait_uninitialized_heap.c |  3 ++-
>  .../futex/functional/futex_wait_wouldblock.c         |  3 ++-
>  tools/testing/selftests/futex/include/logging.h      | 20 +++++++++-----------
>  8 files changed, 30 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/testing/selftests/futex/functional/futex_requeue_pi.c b/tools/testing/selftests/futex/functional/futex_requeue_pi.c
> index d24ab7421e73..54cd5c414e82 100644
> --- a/tools/testing/selftests/futex/functional/futex_requeue_pi.c
> +++ b/tools/testing/selftests/futex/functional/futex_requeue_pi.c
> @@ -394,9 +394,11 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> -	printf("%s: Test requeue functionality\n", basename(argv[0]));
> -	printf("\tArguments: broadcast=%d locked=%d owner=%d timeout=%ldns\n",
> -	       broadcast, locked, owner, timeout_ns);
> +	ksft_print_header();
> +	ksft_print_msg("%s: Test requeue functionality\n", basename(argv[0]));
> +	ksft_print_msg(
> +		"\tArguments: broadcast=%d locked=%d owner=%d timeout=%ldns\n",
> +		broadcast, locked, owner, timeout_ns);
>  
>  	/*
>  	 * FIXME: unit_test is obsolete now that we parse options and the
> diff --git a/tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c b/tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c
> index e0a798ad0d21..08187a16507f 100644
> --- a/tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c
> +++ b/tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c
> @@ -78,7 +78,8 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> -	printf("%s: Detect mismatched requeue_pi operations\n",
> +	ksft_print_header();
> +	ksft_print_msg("%s: Detect mismatched requeue_pi operations\n",
>  	       basename(argv[0]));
>  
>  	if (pthread_create(&child, NULL, blocking_child, NULL)) {
> diff --git a/tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c b/tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c
> index 982f83577501..f0542a344d95 100644
> --- a/tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c
> +++ b/tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c
> @@ -143,9 +143,10 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> -	printf("%s: Test signal handling during requeue_pi\n",
> +	ksft_print_header();
> +	ksft_print_msg("%s: Test signal handling during requeue_pi\n",
>  	       basename(argv[0]));
> -	printf("\tArguments: <none>\n");
> +	ksft_print_msg("\tArguments: <none>\n");
>  
>  	sa.sa_handler = handle_signal;
>  	sigemptyset(&sa.sa_mask);
> diff --git a/tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c b/tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c
> index bdc48dc047e5..6216de828093 100644
> --- a/tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c
> +++ b/tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c
> @@ -97,8 +97,10 @@ int main(int argc, char **argv)
>  		}
>  	}
>  
> -	printf("%s: Test the futex value of private file mappings in FUTEX_WAIT\n",
> -	       basename(argv[0]));
> +	ksft_print_header();
> +	ksft_print_msg(
> +		"%s: Test the futex value of private file mappings in FUTEX_WAIT\n",
> +		basename(argv[0]));
>  
>  	ret = pthread_create(&thr, NULL, thr_futex_wait, NULL);
>  	if (ret < 0) {
> diff --git a/tools/testing/selftests/futex/functional/futex_wait_timeout.c b/tools/testing/selftests/futex/functional/futex_wait_timeout.c
> index 6aadd560366e..bab3dfe1787f 100644
> --- a/tools/testing/selftests/futex/functional/futex_wait_timeout.c
> +++ b/tools/testing/selftests/futex/functional/futex_wait_timeout.c
> @@ -68,9 +68,10 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> -	printf("%s: Block on a futex and wait for timeout\n",
> +	ksft_print_header();
> +	ksft_print_msg("%s: Block on a futex and wait for timeout\n",
>  	       basename(argv[0]));
> -	printf("\tArguments: timeout=%ldns\n", timeout_ns);
> +	ksft_print_msg("\tArguments: timeout=%ldns\n", timeout_ns);
>  
>  	/* initialize timeout */
>  	to.tv_sec = 0;
> diff --git a/tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c b/tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c
> index d237a8b702f0..26975322545b 100644
> --- a/tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c
> +++ b/tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c
> @@ -99,7 +99,8 @@ int main(int argc, char **argv)
>  		exit(1);
>  	}
>  
> -	printf("%s: Test the uninitialized futex value in FUTEX_WAIT\n",
> +	ksft_print_header();
> +	ksft_print_msg("%s: Test the uninitialized futex value in FUTEX_WAIT\n",
>  	       basename(argv[0]));
>  
>  
> diff --git a/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c b/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c
> index 9a2c56fa7305..da15a63269b4 100644
> --- a/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c
> +++ b/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c
> @@ -64,7 +64,8 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> -	printf("%s: Test the unexpected futex value in FUTEX_WAIT\n",
> +	ksft_print_header();
> +	ksft_print_msg("%s: Test the unexpected futex value in FUTEX_WAIT\n",
>  	       basename(argv[0]));
>  
>  	info("Calling futex_wait on f1: %u @ %p with val=%u\n", f1, &f1, f1+1);
> diff --git a/tools/testing/selftests/futex/include/logging.h b/tools/testing/selftests/futex/include/logging.h
> index 4e7944984fbb..01989644e50a 100644
> --- a/tools/testing/selftests/futex/include/logging.h
> +++ b/tools/testing/selftests/futex/include/logging.h
> @@ -109,22 +109,20 @@ void log_verbosity(int level)
>   */
>  void print_result(const char *test_name, int ret)
>  {
> -	const char *result = "Unknown return code";
> -
>  	switch (ret) {
>  	case RET_PASS:
> -		ksft_inc_pass_cnt();
> -		result = PASS;
> -		break;
> +		ksft_test_result_pass("%s\n", test_name);
> +		ksft_print_cnts();
> +		return;
>  	case RET_ERROR:
> -		result = ERROR;
> -		break;
> +		ksft_test_result_error("%s\n", test_name);
> +		ksft_print_cnts();
> +		return;
>  	case RET_FAIL:
> -		ksft_inc_fail_cnt();
> -		result = FAIL;
> -		break;
> +		ksft_test_result_fail("%s\n", test_name);
> +		ksft_print_cnts();
> +		return;
>  	}
> -	printf("selftests: %s [%s]\n", test_name, result);
>  }
>  
>  /* log level macros */
> -- 
> 2.11.0
> 
> 

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH] selftests: futex: convert test to use ksft TAP13 framework

[Shuah Khan]

On 08/16/2017 05:25 PM, Darren Hart wrote:
> On Fri, Aug 04, 2017 at 06:46:50PM -0600, Shuah Khan wrote:
>> Convert test to use ksft TAP13 framework.
>>
> 
> Please include some information in the commit about why we're doing this
> and what it is.
> 

Updated the change log with more information as follows:

"Convert test to use ksft TAP13 framework to print user friendly
test output which is consistent across kselftest suite."

> From what I can tell this is converting raw printk to a wrapped set of
> logging functions to produce a more consistent output across the
> kselftest suite.
> 
> I have no objection to that for the futex tests:
> 
> Acked-by: Darren Hart (VMware) <dvhart@infradead.org>

Thanks. Applied to linux-kselftest next for 4.14-rc1 with the updated
change log and your Ack.

-- Shuah