From 07bdcc2b0dd181823702cc8f842a0b47c4cf3324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gonz=C3=A1lez?= Date: Fri, 11 Aug 2023 14:59:03 +0100 Subject: [PATCH 1/7] 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 | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index c6891bb43..fde07159e 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -73,15 +73,18 @@ def execute_reference_driver_tests(ref_component, driver_component, outcome_file Results.log("Error: failed to run reference/driver components") sys.exit(ret_val) -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_driver_vs_reference(outcomes, component_ref, component_driver, ignored_suites, ignored_test=None): @@ -122,10 +125,10 @@ def analyze_driver_vs_reference(outcomes, component_ref, component_driver, result = False return result -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): @@ -151,10 +154,9 @@ by a semicolon. def do_analyze_coverage(outcome_file, args): """Perform coverage analysis.""" - del args # unused outcomes = read_outcome_file(outcome_file) Results.log("\n*** Analyze coverage ***\n") - results = analyze_outcomes(outcomes) + results = analyze_outcomes(outcomes, args['allow_list']) return results.error_count == 0 def do_analyze_driver_vs_reference(outcome_file, args): @@ -175,7 +177,9 @@ def do_analyze_driver_vs_reference(outcome_file, args): TASKS = { 'analyze_coverage': { 'test_function': do_analyze_coverage, - 'args': {} + 'args': { + 'allow_list': [], + } }, # There are 2 options to use analyze_driver_vs_reference_xxx locally: # 1. Run tests and then analysis: From b401e113ff3421d2f2c7bba5368bf0eb37920dff 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/7] 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 fde07159e..24f4da773 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -73,15 +73,16 @@ def execute_reference_driver_tests(ref_component, driver_component, outcome_file Results.log("Error: failed to run reference/driver components") sys.exit(ret_val) -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) @@ -125,10 +126,11 @@ def analyze_driver_vs_reference(outcomes, component_ref, component_driver, result = False return result -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): @@ -156,7 +158,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 def do_analyze_driver_vs_reference(outcome_file, args): @@ -179,6 +181,7 @@ TASKS = { 'test_function': do_analyze_coverage, 'args': { 'allow_list': [], + 'full_coverage': False, } }, # There are 2 options to use analyze_driver_vs_reference_xxx locally: @@ -430,6 +433,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: @@ -449,6 +457,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 358c6c644a49d5b86a3f24d3e69edbfb10e4c11d 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/7] 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 | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 24f4da773..e5abae738 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -180,7 +180,11 @@ TASKS = { 'analyze_coverage': { 'test_function': do_analyze_coverage, 'args': { - 'allow_list': [], + 'allow_list': [ + 'test_suite_psa_crypto_metadata;Asymmetric signature: ' + 'pure EdDSA', + 'test_suite_psa_crypto_metadata;Cipher: XTS' + ], 'full_coverage': False, } }, From 7ebb18fbd678d2a454c3111b9d8536886073377a 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/7] 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 e5abae738..230fc2f3e 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -85,7 +85,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_driver_vs_reference(outcomes, component_ref, component_driver, ignored_suites, ignored_test=None): From 5022311c9de839a0d4e22a3be47cd569e63d33ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gonz=C3=A1lez?= Date: Tue, 22 Aug 2023 09:52:06 +0100 Subject: [PATCH 5/7] Tidy up allow list definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Don't break string literals in the allow list definition * Comment each test that belongs to the allow list is there. Signed-off-by: Tomás González --- tests/scripts/analyze_outcomes.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 230fc2f3e..ea1172ae2 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -184,9 +184,10 @@ TASKS = { 'test_function': do_analyze_coverage, 'args': { 'allow_list': [ - 'test_suite_psa_crypto_metadata;Asymmetric signature: ' - 'pure EdDSA', - 'test_suite_psa_crypto_metadata;Cipher: XTS' + # 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 a0631446b530759dce94d9b50e1fccb11de62cd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gonz=C3=A1lez?= Date: Tue, 22 Aug 2023 12:17:57 +0100 Subject: [PATCH 6/7] Correct analyze_outcomes.py 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 ea1172ae2..c8bf0799b 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -86,7 +86,7 @@ def analyze_coverage(results, outcomes, allow_list, full_coverage): elif hits != 0 and key in allow_list: # Test Case should be removed from the allow list. if full_coverage: - results.error('Allow listed test case was executed: {}', key) + results.error('Allow listed test case was executed: {}', key) else: results.warning('Allow listed test case was executed: {}', key) From d43cab3f5c09bdff40649bade124450bdb05c84f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gonz=C3=A1lez?= Date: Thu, 24 Aug 2023 09:12:40 +0100 Subject: [PATCH 7/7] 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 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index c8bf0799b..3b91bfb19 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -188,10 +188,10 @@ TASKS = { 'test_suite_psa_crypto_metadata;Asymmetric signature: pure EdDSA', # Algorithm not supported yet 'test_suite_psa_crypto_metadata;Cipher: XTS', - ], + ], 'full_coverage': False, } - }, + }, # There are 2 options to use analyze_driver_vs_reference_xxx locally: # 1. Run tests and then analysis: # - tests/scripts/all.sh --outcome-file "$PWD/out.csv"