From 2fdd503c4e8ad26b20708c3092953b4d894b419a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gonz=C3=A1lez?= Date: Wed, 23 Aug 2023 16:43:26 +0100 Subject: [PATCH 1/5] Add allow list for non-executed test cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The allow list explicits which test cases are allowed to not be executed when testing. This may be, for example, because a feature is yet to be developed but the test for that feature is already in our code base. Signed-off-by: Tomás González --- tests/scripts/analyze_outcomes.py | 64 ++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 9 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index d06a0596f..390cbbc72 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -9,6 +9,7 @@ less likely to be useful. import argparse import sys import traceback +import re import check_test_cases @@ -50,20 +51,23 @@ class TestCaseOutcomes: """ return len(self.successes) + len(self.failures) -def analyze_coverage(results, outcomes): +def analyze_coverage(results, outcomes, allow_list): """Check that all available test cases are executed at least once.""" available = check_test_cases.collect_available_test_cases() for key in available: hits = outcomes[key].hits() if key in outcomes else 0 - if hits == 0: + if hits == 0 and key not in allow_list: # Make this a warning, not an error, as long as we haven't # fixed this branch to have full coverage of test cases. results.warning('Test case not executed: {}', key) + elif hits != 0 and key in allow_list: + # Test Case should be removed from the allow list. + results.warning('Allow listed test case was executed: {}', key) -def analyze_outcomes(outcomes): +def analyze_outcomes(outcomes, allow_list): """Run all analyses on the given outcome collection.""" results = Results() - analyze_coverage(results, outcomes) + analyze_coverage(results, outcomes, allow_list) return results def read_outcome_file(outcome_file): @@ -87,20 +91,62 @@ by a semicolon. outcomes[key].failures.append(setup) return outcomes -def analyze_outcome_file(outcome_file): - """Analyze the given outcome file.""" +def do_analyze_coverage(outcome_file, args): + """Perform coverage analysis.""" outcomes = read_outcome_file(outcome_file) - return analyze_outcomes(outcomes) + Results.log("\n*** Analyze coverage ***\n") + results = analyze_outcomes(outcomes, args['allow_list']) + return results.error_count == 0 + +# List of tasks with a function that can handle this task and additional arguments if required +TASKS = { + 'analyze_coverage': { + 'test_function': do_analyze_coverage, + 'args': { + 'allow_list': [], + } + }, +} def main(): try: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument('outcomes', metavar='OUTCOMES.CSV', help='Outcome file to analyze') + parser.add_argument('task', default='all', nargs='?', + help='Analysis to be done. By default, run all tasks. ' + 'With one or more TASK, run only those. ' + 'TASK can be the name of a single task or ' + 'comma/space-separated list of tasks. ') + parser.add_argument('--list', action='store_true', + help='List all available tasks and exit.') options = parser.parse_args() - results = analyze_outcome_file(options.outcomes) - if results.error_count > 0: + + if options.list: + for task in TASKS: + Results.log(task) + sys.exit(0) + + result = True + + if options.task == 'all': + tasks = TASKS.keys() + else: + tasks = re.split(r'[, ]+', options.task) + + for task in tasks: + if task not in TASKS: + Results.log('Error: invalid task: {}'.format(task)) + sys.exit(1) + + for task in TASKS: + if task in tasks: + if not TASKS[task]['test_function'](options.outcomes, TASKS[task]['args']): + result = False + + if result is False: sys.exit(1) + Results.log("SUCCESS :-)") except Exception: # pylint: disable=broad-except # Print the backtrace and exit explicitly with our chosen status. traceback.print_exc() From 45d49595b7080a9d6a978a92dc6649814438071e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gonz=C3=A1lez?= Date: Fri, 11 Aug 2023 15:22:04 +0100 Subject: [PATCH 2/5] Add a flag for requiring full coverage in coverage tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce the --require-full-coverage in analyze_outcomes.py so that when analyze_outcomes.py --require-full-coverage is called, those tests that are not executed and are not in the allowed list issue an error instead of a warning. Note that it is useful to run analyze_outcomes.py on incomplete test results, so this error mode needs to remain optional in the long term. Signed-off-by: Tomás González --- tests/scripts/analyze_outcomes.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 390cbbc72..14a9b864e 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -51,23 +51,25 @@ class TestCaseOutcomes: """ return len(self.successes) + len(self.failures) -def analyze_coverage(results, outcomes, allow_list): +def analyze_coverage(results, outcomes, allow_list, full_coverage): """Check that all available test cases are executed at least once.""" available = check_test_cases.collect_available_test_cases() for key in available: hits = outcomes[key].hits() if key in outcomes else 0 if hits == 0 and key not in allow_list: - # Make this a warning, not an error, as long as we haven't - # fixed this branch to have full coverage of test cases. - results.warning('Test case not executed: {}', key) + if full_coverage: + results.error('Test case not executed: {}', key) + else: + results.warning('Test case not executed: {}', key) elif hits != 0 and key in allow_list: # Test Case should be removed from the allow list. results.warning('Allow listed test case was executed: {}', key) -def analyze_outcomes(outcomes, allow_list): +def analyze_outcomes(outcomes, args): """Run all analyses on the given outcome collection.""" results = Results() - analyze_coverage(results, outcomes, allow_list) + analyze_coverage(results, outcomes, args['allow_list'], + args['full_coverage']) return results def read_outcome_file(outcome_file): @@ -95,7 +97,7 @@ def do_analyze_coverage(outcome_file, args): """Perform coverage analysis.""" outcomes = read_outcome_file(outcome_file) Results.log("\n*** Analyze coverage ***\n") - results = analyze_outcomes(outcomes, args['allow_list']) + results = analyze_outcomes(outcomes, args) return results.error_count == 0 # List of tasks with a function that can handle this task and additional arguments if required @@ -104,6 +106,7 @@ TASKS = { 'test_function': do_analyze_coverage, 'args': { 'allow_list': [], + 'full_coverage': False, } }, } @@ -120,6 +123,11 @@ def main(): 'comma/space-separated list of tasks. ') parser.add_argument('--list', action='store_true', help='List all available tasks and exit.') + parser.add_argument('--require-full-coverage', action='store_true', + dest='full_coverage', help="Require all available " + "test cases to be executed and issue an error " + "otherwise. This flag is ignored if 'task' is " + "neither 'all' nor 'analyze_coverage'") options = parser.parse_args() if options.list: @@ -139,6 +147,9 @@ def main(): Results.log('Error: invalid task: {}'.format(task)) sys.exit(1) + TASKS['analyze_coverage']['args']['full_coverage'] = \ + options.full_coverage + for task in TASKS: if task in tasks: if not TASKS[task]['test_function'](options.outcomes, TASKS[task]['args']): From c8957333498b14ab8a83dca0708848dae535d720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gonz=C3=A1lez?= Date: Mon, 14 Aug 2023 15:43:46 +0100 Subject: [PATCH 3/5] Add EdDSA and XTS to the allow list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As specified in https://github.com/Mbed-TLS/mbedtls/issues/5390#issuecomment-1669585707 EdDSA and XTS tests are legitimately never executed, so add them to the allow list. Signed-off-by: Tomás González --- tests/scripts/analyze_outcomes.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 14a9b864e..4f381bd6b 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -105,7 +105,12 @@ TASKS = { 'analyze_coverage': { 'test_function': do_analyze_coverage, 'args': { - 'allow_list': [], + 'allow_list': [ + # Algorithm not supported yet + 'test_suite_psa_crypto_metadata;Asymmetric signature: pure EdDSA', + # Algorithm not supported yet + 'test_suite_psa_crypto_metadata;Cipher: XTS', + ], 'full_coverage': False, } }, From 14b36ef54a581391f86d161ab4ff61457684f9ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gonz=C3=A1lez?= Date: Tue, 22 Aug 2023 09:40:23 +0100 Subject: [PATCH 4/5] Make non-executed tests that are not in the allow list an error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Turn the warnings produced when finding non-executed tests that are not in the allow list into errors. Signed-off-by: Tomás González --- tests/scripts/analyze_outcomes.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 4f381bd6b..0bdfcf98a 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -63,7 +63,10 @@ def analyze_coverage(results, outcomes, allow_list, full_coverage): results.warning('Test case not executed: {}', key) elif hits != 0 and key in allow_list: # Test Case should be removed from the allow list. - results.warning('Allow listed test case was executed: {}', key) + if full_coverage: + results.error('Allow listed test case was executed: {}', key) + else: + results.warning('Allow listed test case was executed: {}', key) def analyze_outcomes(outcomes, args): """Run all analyses on the given outcome collection.""" From 1cf437bc579f1b5220ffeaa430fc9372a56379e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gonz=C3=A1lez?= Date: Thu, 24 Aug 2023 09:27:28 +0100 Subject: [PATCH 5/5] Correct analyze_outcomes identation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás González --- tests/scripts/analyze_outcomes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 0bdfcf98a..d50a04e61 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -113,7 +113,7 @@ TASKS = { 'test_suite_psa_crypto_metadata;Asymmetric signature: pure EdDSA', # Algorithm not supported yet 'test_suite_psa_crypto_metadata;Cipher: XTS', - ], + ], 'full_coverage': False, } },