diff mbox series

kunit: tool: make --json do nothing if --raw_ouput is set

Message ID 20221121195526.425882-1-dlatypov@google.com (mailing list archive)
State Accepted
Commit 309e22effb741a8c65131a2694a49839fd685a27
Delegated to: Brendan Higgins
Headers show
Series kunit: tool: make --json do nothing if --raw_ouput is set | expand

Commit Message

Daniel Latypov Nov. 21, 2022, 7:55 p.m. UTC 
When --raw_output is set (to any value), we don't actually parse the
test results. So asking to print the test results as json doesn't make
sense.

We internally create a fake test with one passing subtest, so --json
would actually print out something misleading.

This patch:
* Rewords the flag descriptions so hopefully this is more obvious.
* Also updates --raw_output's description to note the default behavior
  is to print out only "KUnit" results (actually any KTAP results)
* also renames and refactors some related logic for clarity (e.g.
  test_result => test, it's a kunit_parser.Test object).

Notably, this patch does not make it an error to specify --json and
--raw_output together. This is an edge case, but I know of at least one
wrapper around kunit.py that always sets --json. You'd never be able to
use --raw_output with that wrapper.

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
 tools/testing/kunit/kunit.py | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)


base-commit: 870f63b7cd78d0055902d839a60408f7428b4e84

Comments

David Gow Nov. 22, 2022, 5:46 a.m. UTC | #1 
On Tue, Nov 22, 2022 at 3:55 AM 'Daniel Latypov' via KUnit Development
<kunit-dev@googlegroups.com> wrote:
>
> When --raw_output is set (to any value), we don't actually parse the
> test results. So asking to print the test results as json doesn't make
> sense.
>
> We internally create a fake test with one passing subtest, so --json
> would actually print out something misleading.
>
> This patch:
> * Rewords the flag descriptions so hopefully this is more obvious.
> * Also updates --raw_output's description to note the default behavior
>   is to print out only "KUnit" results (actually any KTAP results)
> * also renames and refactors some related logic for clarity (e.g.
>   test_result => test, it's a kunit_parser.Test object).
>
> Notably, this patch does not make it an error to specify --json and
> --raw_output together. This is an edge case, but I know of at least one
> wrapper around kunit.py that always sets --json. You'd never be able to
> use --raw_output with that wrapper.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---

This seems sensible enough to me (and works fine here).

I really like the new flag descriptions, too.

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David

>  tools/testing/kunit/kunit.py | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index 4d4663fb578b..e7b6549712d6 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -192,12 +192,11 @@ def _map_to_overall_status(test_status: kunit_parser.TestStatus) -> KunitStatus:
>  def parse_tests(request: KunitParseRequest, metadata: kunit_json.Metadata, input_data: Iterable[str]) -> Tuple[KunitResult, kunit_parser.Test]:
>         parse_start = time.time()
>
> -       test_result = kunit_parser.Test()
> -
>         if request.raw_output:
>                 # Treat unparsed results as one passing test.
> -               test_result.status = kunit_parser.TestStatus.SUCCESS
> -               test_result.counts.passed = 1
> +               fake_test = kunit_parser.Test()
> +               fake_test.status = kunit_parser.TestStatus.SUCCESS
> +               fake_test.counts.passed = 1
>
>                 output: Iterable[str] = input_data
>                 if request.raw_output == 'all':
> @@ -206,14 +205,17 @@ def parse_tests(request: KunitParseRequest, metadata: kunit_json.Metadata, input
>                         output = kunit_parser.extract_tap_lines(output, lstrip=False)
>                 for line in output:
>                         print(line.rstrip())
> +               parse_time = time.time() - parse_start
> +               return KunitResult(KunitStatus.SUCCESS, parse_time), fake_test
>
> -       else:
> -               test_result = kunit_parser.parse_run_tests(input_data)
> -       parse_end = time.time()
> +
> +       # Actually parse the test results.
> +       test = kunit_parser.parse_run_tests(input_data)
> +       parse_time = time.time() - parse_start
>
>         if request.json:
>                 json_str = kunit_json.get_json_result(
> -                                       test=test_result,
> +                                       test=test,
>                                         metadata=metadata)
>                 if request.json == 'stdout':
>                         print(json_str)
> @@ -223,10 +225,10 @@ def parse_tests(request: KunitParseRequest, metadata: kunit_json.Metadata, input
>                         stdout.print_with_timestamp("Test results stored in %s" %
>                                 os.path.abspath(request.json))
>
> -       if test_result.status != kunit_parser.TestStatus.SUCCESS:
> -               return KunitResult(KunitStatus.TEST_FAILURE, parse_end - parse_start), test_result
> +       if test.status != kunit_parser.TestStatus.SUCCESS:
> +               return KunitResult(KunitStatus.TEST_FAILURE, parse_time), test
>
> -       return KunitResult(KunitStatus.SUCCESS, parse_end - parse_start), test_result
> +       return KunitResult(KunitStatus.SUCCESS, parse_time), test
>
>  def run_tests(linux: kunit_kernel.LinuxSourceTree,
>               request: KunitRequest) -> KunitResult:
> @@ -359,14 +361,14 @@ def add_exec_opts(parser) -> None:
>                             choices=['suite', 'test'])
>
>  def add_parse_opts(parser) -> None:
> -       parser.add_argument('--raw_output', help='If set don\'t format output from kernel. '
> -                           'If set to --raw_output=kunit, filters to just KUnit output.',
> +       parser.add_argument('--raw_output', help='If set don\'t parse output from kernel. '
> +                           'By default, filters to just KUnit output. Use '
> +                           '--raw_output=all to show everything',
>                              type=str, nargs='?', const='all', default=None, choices=['all', 'kunit'])
>         parser.add_argument('--json',
>                             nargs='?',
> -                           help='Stores test results in a JSON, and either '
> -                           'prints to stdout or saves to file if a '
> -                           'filename is specified',
> +                           help='Prints parsed test results as JSON to stdout or a file if '
> +                           'a filename is specified. Does nothing if --raw_output is set.',
>                             type=str, const='stdout', default=None, metavar='FILE')
>
>
>
> base-commit: 870f63b7cd78d0055902d839a60408f7428b4e84
> --
> 2.38.1.584.g0f3c55d4c2-goog
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20221121195526.425882-1-dlatypov%40google.com.
diff mbox series

Patch

diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 4d4663fb578b..e7b6549712d6 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -192,12 +192,11 @@  def _map_to_overall_status(test_status: kunit_parser.TestStatus) -> KunitStatus:
 def parse_tests(request: KunitParseRequest, metadata: kunit_json.Metadata, input_data: Iterable[str]) -> Tuple[KunitResult, kunit_parser.Test]:
 	parse_start = time.time()
 
-	test_result = kunit_parser.Test()
-
 	if request.raw_output:
 		# Treat unparsed results as one passing test.
-		test_result.status = kunit_parser.TestStatus.SUCCESS
-		test_result.counts.passed = 1
+		fake_test = kunit_parser.Test()
+		fake_test.status = kunit_parser.TestStatus.SUCCESS
+		fake_test.counts.passed = 1
 
 		output: Iterable[str] = input_data
 		if request.raw_output == 'all':
@@ -206,14 +205,17 @@  def parse_tests(request: KunitParseRequest, metadata: kunit_json.Metadata, input
 			output = kunit_parser.extract_tap_lines(output, lstrip=False)
 		for line in output:
 			print(line.rstrip())
+		parse_time = time.time() - parse_start
+		return KunitResult(KunitStatus.SUCCESS, parse_time), fake_test
 
-	else:
-		test_result = kunit_parser.parse_run_tests(input_data)
-	parse_end = time.time()
+
+	# Actually parse the test results.
+	test = kunit_parser.parse_run_tests(input_data)
+	parse_time = time.time() - parse_start
 
 	if request.json:
 		json_str = kunit_json.get_json_result(
-					test=test_result,
+					test=test,
 					metadata=metadata)
 		if request.json == 'stdout':
 			print(json_str)
@@ -223,10 +225,10 @@  def parse_tests(request: KunitParseRequest, metadata: kunit_json.Metadata, input
 			stdout.print_with_timestamp("Test results stored in %s" %
 				os.path.abspath(request.json))
 
-	if test_result.status != kunit_parser.TestStatus.SUCCESS:
-		return KunitResult(KunitStatus.TEST_FAILURE, parse_end - parse_start), test_result
+	if test.status != kunit_parser.TestStatus.SUCCESS:
+		return KunitResult(KunitStatus.TEST_FAILURE, parse_time), test
 
-	return KunitResult(KunitStatus.SUCCESS, parse_end - parse_start), test_result
+	return KunitResult(KunitStatus.SUCCESS, parse_time), test
 
 def run_tests(linux: kunit_kernel.LinuxSourceTree,
 	      request: KunitRequest) -> KunitResult:
@@ -359,14 +361,14 @@  def add_exec_opts(parser) -> None:
 			    choices=['suite', 'test'])
 
 def add_parse_opts(parser) -> None:
-	parser.add_argument('--raw_output', help='If set don\'t format output from kernel. '
-			    'If set to --raw_output=kunit, filters to just KUnit output.',
+	parser.add_argument('--raw_output', help='If set don\'t parse output from kernel. '
+			    'By default, filters to just KUnit output. Use '
+			    '--raw_output=all to show everything',
 			     type=str, nargs='?', const='all', default=None, choices=['all', 'kunit'])
 	parser.add_argument('--json',
 			    nargs='?',
-			    help='Stores test results in a JSON, and either '
-			    'prints to stdout or saves to file if a '
-			    'filename is specified',
+			    help='Prints parsed test results as JSON to stdout or a file if '
+			    'a filename is specified. Does nothing if --raw_output is set.',
 			    type=str, const='stdout', default=None, metavar='FILE')