Re: [PATCH 2/3] kunit: tool: Support skipped tests in kunit_tool

On Thu, May 27, 2021 at 1:22 AM David Gow davidgow@google.com wrote:

On Thu, May 27, 2021 at 3:10 AM Daniel Latypov dlatypov@google.com wrote:

...

On Wed, May 26, 2021 at 1:11 AM 'David Gow' via KUnit Development kunit-dev@googlegroups.com wrote:

...

Add support for the SKIP directive to kunit_tool's TAP parser.

Skipped tests now show up as such in the printed summary. The number of skipped tests is counted, and if all tests in a suite are skipped, the suite is also marked as skipped. Otherwise, skipped tests do affect the suite result.

Example output: [00:22:34] ======== [SKIPPED] example_skip ======== [00:22:34] [SKIPPED] example_skip_test # SKIP this test should be skipped [00:22:34] [SKIPPED] example_mark_skipped_test # SKIP this test should be skipped [00:22:34] ============================================================ [00:22:34] Testing complete. 2 tests run. 0 failed. 0 crashed. 2 skipped.

Signed-off-by: David Gow davidgow@google.com

tools/testing/kunit/kunit_parser.py | 47 +++++++++++++++++++------- tools/testing/kunit/kunit_tool_test.py | 22 ++++++++++++

This seems to be missing the added test files.

Whoops, yes: I'll add these back in v2.

...

...

2 files changed, 57 insertions(+), 12 deletions(-)

diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index e8bcc139702e..6b5dd26b479d 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -43,6 +43,7 @@ class TestCase(object): class TestStatus(Enum): SUCCESS = auto() FAILURE = auto()

•   SKIPPED = auto()
    TEST_CRASHED = auto()
    NO_TESTS = auto()
    FAILURE_TO_PARSE_TESTS = auto()
  

@@ -108,6 +109,8 @@ def save_non_diagnostic(lines: List[str], test_case: TestCase) -> None:

OkNotOkResult = namedtuple('OkNotOkResult', ['is_ok','description', 'text'])

+OK_NOT_OK_SKIP = re.compile(r'^[\s]*(ok|not ok) [0-9]+ - (.*) # SKIP(.*)$')

OK_NOT_OK_SUBTEST = re.compile(r'^[\s]+(ok|not ok) [0-9]+ - (.*)$')

OK_NOT_OK_MODULE = re.compile(r'^(ok|not ok) ([0-9]+) - (.*)$') @@ -125,6 +128,10 @@ def parse_ok_not_ok_test_case(lines: List[str], test_case: TestCase) -> bool: if match: test_case.log.append(lines.pop(0)) test_case.name = match.group(2)

•           skip_match = OK_NOT_OK_SKIP.match(line)
  

•           if skip_match:
  

•                   test_case.status = TestStatus.SKIPPED
  

•                   return True
            if test_case.status == TestStatus.TEST_CRASHED:
                    return True
            if match.group(1) == 'ok':
  

@@ -188,16 +195,16 @@ def parse_subtest_plan(lines: List[str]) -> Optional[int]: return None

def max_status(left: TestStatus, right: TestStatus) -> TestStatus:

•   if left == TestStatus.TEST_CRASHED or right == TestStatus.TEST_CRASHED:
  

•   if left == right:
  

•           return left
  

•   elif left == TestStatus.TEST_CRASHED or right == TestStatus.TEST_CRASHED:
            return TestStatus.TEST_CRASHED
    elif left == TestStatus.FAILURE or right == TestStatus.FAILURE:
            return TestStatus.FAILURE
  

•   elif left != TestStatus.SUCCESS:
  

•           return left
  

•   elif right != TestStatus.SUCCESS:
  

•   elif left == TestStatus.SKIPPED:
            return right
    else:
  

•           return TestStatus.SUCCESS
  

•           return left
  
  

def parse_ok_not_ok_test_suite(lines: List[str], test_suite: TestSuite, @@ -214,6 +221,9 @@ def parse_ok_not_ok_test_suite(lines: List[str], test_suite.status = TestStatus.SUCCESS else: test_suite.status = TestStatus.FAILURE

•           skip_match = OK_NOT_OK_SKIP.match(line)
  

•           if skip_match:
  

•                   test_suite.status = TestStatus.SKIPPED
            suite_index = int(match.group(2))
            if suite_index != expected_suite_index:
                    print_with_timestamp(
  

@@ -224,8 +234,8 @@ def parse_ok_not_ok_test_suite(lines: List[str], else: return False

-def bubble_up_errors(statuses: Iterable[TestStatus]) -> TestStatus:

•   return reduce(max_status, statuses, TestStatus.SUCCESS)
  

+def bubble_up_errors(status_list: Iterable[TestStatus]) -> TestStatus:

•   return reduce(max_status, status_list, TestStatus.SKIPPED)
  
  

def bubble_up_test_case_errors(test_suite: TestSuite) -> TestStatus: max_test_case_status = bubble_up_errors(x.status for x in test_suite.cases) @@ -315,9 +325,12 @@ def print_and_count_results(test_result: TestResult) -> Tuple[int, int, int]:

Btw, this type annotation is out of date.

Oops: will fix and/or replace with the below.

...

But I think an ever growing Tuple is too cumbersome, how about this?

Yeah, this does seem cleaner: I'll put this or something like it in v2.

...

diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index 6b5dd26b479d..055ee1e4d19d 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -6,6 +6,7 @@ # Author: Felix Guo felixguoxiuping@gmail.com # Author: Brendan Higgins brendanhiggins@google.com

+from dataclasses import dataclass import re

from collections import namedtuple @@ -321,11 +322,19 @@ def parse_test_result(lines: List[str]) -> TestResult: else: return TestResult(TestStatus.NO_TESTS, [], lines)

-def print_and_count_results(test_result: TestResult) -> Tuple[int, int, int]:

•   total_tests = 0
  

•   failed_tests = 0
  

•   crashed_tests = 0
  

•   skipped_tests = 0
  

+# Note: This would require Python 3.7. We currently only required 3.6 (enum.auto). We can do it by hand to avoid that, if we want.

Hmm... I'm generally loath to increase the version requirement for something this simple, so might look into doing a version of this without the dataclass.

I think the same argument applies to enum.auto when we can just manually assign values :P

But yes, I'd suggest not using it. You'd just need to manually write the __init__() in that case (you can't use namedtuple since we need to modify the fields, but also I prefer having type annotations on my fields).

I only used @dataclass to make my example easier to write since I'm lazy.

...

+@dataclass +class TestCounts:

•   passed: int = 0
  

•   failed: int = 0
  

•   skipped: int = 0
  

•   crashed: int = 0
  

•   def total(self) -> int:
  

•           return self.passed + self.failed + self.skipped + self.crashed
  

+def print_and_count_results(test_result: TestResult) -> TestCounts:

•   counts = TestCounts()
    for test_suite in test_result.suites:
            if test_suite.status == TestStatus.SUCCESS:
                    print_suite_divider(green('[PASSED] ') +
  

test_suite.name) @@ -336,39 +345,33 @@ def print_and_count_results(test_result: TestResult) -> Tuple[int, int, int]: else: print_suite_divider(red('[FAILED] ') + test_suite.name) for test_case in test_suite.cases:

•                   total_tests += 1
                    if test_case.status == TestStatus.SUCCESS:
  

•                           counts.passed += 1
                            print_with_timestamp(green('[PASSED]
  

') + test_case.name) elif test_case.status == TestStatus.SKIPPED:

•                           skipped_tests += 1
  

•                           counts.skipped += 1
                            print_with_timestamp(yellow('[SKIPPED]
  

') + test_case.name) elif test_case.status == TestStatus.TEST_CRASHED:

•                           crashed_tests += 1
  

•                           counts.crashed += 1
                            print_with_timestamp(red('[CRASHED] '
  

• test_case.name)) print_log(map(yellow, test_case.log)) print_with_timestamp('') else:

•                           failed_tests += 1
  

•                           counts.failed += 1
                            print_with_timestamp(red('[FAILED] ')
  

• test_case.name) print_log(map(yellow, test_case.log)) print_with_timestamp('')

•   return total_tests, failed_tests, crashed_tests, skipped_tests
  

•   return counts
  
  

def parse_run_tests(kernel_output) -> TestResult:

•   total_tests = 0
  

•   failed_tests = 0
  

•   crashed_tests = 0
  

•   skipped_tests = 0
  

•   counts = TestCounts()
    test_result =
  

parse_test_result(list(isolate_kunit_output(kernel_output))) if test_result.status == TestStatus.NO_TESTS: print(red('[ERROR] ') + yellow('no tests run!')) elif test_result.status == TestStatus.FAILURE_TO_PARSE_TESTS: print(red('[ERROR] ') + yellow('could not parse test results!')) else:

•           (total_tests,
  

•            failed_tests,
  

•            crashed_tests,
  

•            skipped_tests) = print_and_count_results(test_result)
  

•           counts = print_and_count_results(test_result)
    print_with_timestamp(DIVIDER)
    if test_result.status == TestStatus.SUCCESS:
            fmt = green
  

@@ -378,5 +381,5 @@ def parse_run_tests(kernel_output) -> TestResult: fmt =red print_with_timestamp( fmt('Testing complete. %d tests run. %d failed. %d crashed. %d skipped.' %

•               (total_tests, failed_tests, crashed_tests, skipped_tests)))
  

•               (counts.total(), counts.failed, counts.crashed,
  

counts.skipped))) return test_result

...

    total_tests = 0
    failed_tests = 0
    crashed_tests = 0

•   skipped_tests = 0
    for test_suite in test_result.suites:
            if test_suite.status == TestStatus.SUCCESS:
                    print_suite_divider(green('[PASSED] ') + test_suite.name)
  

•           elif test_suite.status == TestStatus.SKIPPED:
  

•                   print_suite_divider(yellow('[SKIPPED] ') + test_suite.name)
            elif test_suite.status == TestStatus.TEST_CRASHED:
                    print_suite_divider(red('[CRASHED] ' + test_suite.name))
            else:
  

@@ -326,6 +339,9 @@ def print_and_count_results(test_result: TestResult) -> Tuple[int, int, int]: total_tests += 1 if test_case.status == TestStatus.SUCCESS: print_with_timestamp(green('[PASSED] ') + test_case.name)

•                   elif test_case.status == TestStatus.SKIPPED:
  

•                           skipped_tests += 1
  

•                           print_with_timestamp(yellow('[SKIPPED] ') + test_case.name)
                    elif test_case.status == TestStatus.TEST_CRASHED:
                            crashed_tests += 1
                            print_with_timestamp(red('[CRASHED] ' + test_case.name))
  

@@ -336,12 +352,13 @@ def print_and_count_results(test_result: TestResult) -> Tuple[int, int, int]: print_with_timestamp(red('[FAILED] ') + test_case.name) print_log(map(yellow, test_case.log)) print_with_timestamp('')

•   return total_tests, failed_tests, crashed_tests
  

•   return total_tests, failed_tests, crashed_tests, skipped_tests
  
  

def parse_run_tests(kernel_output) -> TestResult: total_tests = 0 failed_tests = 0 crashed_tests = 0

•   skipped_tests = 0
    test_result = parse_test_result(list(isolate_kunit_output(kernel_output)))
    if test_result.status == TestStatus.NO_TESTS:
            print(red('[ERROR] ') + yellow('no tests run!'))
  

@@ -350,10 +367,16 @@ def parse_run_tests(kernel_output) -> TestResult: else: (total_tests, failed_tests,

•            crashed_tests) = print_and_count_results(test_result)
  

•            crashed_tests,
  

•            skipped_tests) = print_and_count_results(test_result)
    print_with_timestamp(DIVIDER)
  

•   fmt = green if test_result.status == TestStatus.SUCCESS else red
  

•   if test_result.status == TestStatus.SUCCESS:
  

•           fmt = green
  

•   elif test_result.status == TestStatus.SKIPPED:
  

•           fmt = yellow
  

•   else:
  

•           fmt =red
    print_with_timestamp(
  

•           fmt('Testing complete. %d tests run. %d failed. %d crashed.' %
  

•               (total_tests, failed_tests, crashed_tests)))
  

•           fmt('Testing complete. %d tests run. %d failed. %d crashed. %d skipped.' %
  

•               (total_tests, failed_tests, crashed_tests, skipped_tests)))
    return test_result
  

diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py index 2e809dd956a7..a51e70cafcc1 100755 --- a/tools/testing/kunit/kunit_tool_test.py +++ b/tools/testing/kunit/kunit_tool_test.py @@ -183,6 +183,28 @@ class KUnitParserTest(unittest.TestCase): kunit_parser.TestStatus.TEST_CRASHED, result.status)

•   def test_skipped_test(self):
  

•           skipped_log = test_data_path('test_skip_tests.log')
  

•           file = open(skipped_log)
  

•           result = kunit_parser.parse_run_tests(file.readlines())
  

•           # A skipped test does not fail the whole suite.
  

•           self.assertEqual(
  

•                   kunit_parser.TestStatus.SUCCESS,
  

•                   result.status)
  

•           file.close()
  

•   def test_skipped_all_tests(self):
  

•           skipped_log = test_data_path('test_skip_all_tests.log')
  

•           file = open(skipped_log)
  

•           result = kunit_parser.parse_run_tests(file.readlines())
  

•           self.assertEqual(
  

•                   kunit_parser.TestStatus.SKIPPED,
  

•                   result.status)
  

•           file.close()
  

•   def test_ignores_prefix_printk_time(self):
            prefix_log = test_data_path('test_config_printk_time.log')
            with open(prefix_log) as file:
  

-- 2.31.1.818.g46aad6cb9e-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/20210526081112.3652290-2-davidgo....