From 113875d53c7a8007d930ff67101cb5585601a642 Mon Sep 17 00:00:00 2001 From: Dmitrii Golovanov Date: Sun, 24 Nov 2024 18:02:19 +0100 Subject: [PATCH] twister: testplan: Detect duplicates on `--no-detailed-test-id` Detect duplicate TestSuites on load, and raise error when it happens with `--no-detailed-test-id` option which shortens TestSuite name excluding the test project path prefix, thus increasing chances for duplicates. Without this check, only the last duplicated test configuration was selected while others silently ignored. Signed-off-by: Dmitrii Golovanov --- scripts/pylib/twister/twisterlib/testplan.py | 13 +++-- scripts/tests/twister/test_testplan.py | 55 +++++++++++++++++--- 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/scripts/pylib/twister/twisterlib/testplan.py b/scripts/pylib/twister/twisterlib/testplan.py index 22c206dfa2c..d4031a96364 100755 --- a/scripts/pylib/twister/twisterlib/testplan.py +++ b/scripts/pylib/twister/twisterlib/testplan.py @@ -188,6 +188,8 @@ class TestPlan: num = self.add_testsuites(testsuite_filter=self.run_individual_testsuite) if num == 0: raise TwisterRuntimeError("No test cases found at the specified location...") + if self.load_errors: + raise TwisterRuntimeError(f"Found {self.load_errors} errors loading {num} test configurations.") self.find_subtests() # get list of scenarios we have parsed into one list @@ -197,9 +199,6 @@ class TestPlan: self.report_duplicates() self.parse_configuration(config_file=self.env.test_config) - if self.load_errors: - raise TwisterRuntimeError("Errors while loading configurations") - # handle quarantine ql = self.options.quarantine_list qv = self.options.quarantine_verify @@ -605,10 +604,18 @@ class TestPlan: suite.add_subcases(suite_dict, subcases, ztest_suite_names) else: suite.add_subcases(suite_dict) + if testsuite_filter: scenario = os.path.basename(suite.name) if suite.name and (suite.name in testsuite_filter or scenario in testsuite_filter): self.testsuites[suite.name] = suite + elif suite.name in self.testsuites: + msg = f"test suite '{suite.name}' in '{suite.yamlfile}' is already added" + if suite.yamlfile == self.testsuites[suite.name].yamlfile: + logger.debug(f"Skip - {msg}") + else: + msg = f"Duplicate {msg} from '{self.testsuites[suite.name].yamlfile}'" + raise TwisterRuntimeError(msg) else: self.testsuites[suite.name] = suite diff --git a/scripts/tests/twister/test_testplan.py b/scripts/tests/twister/test_testplan.py index 0bf83ee9edb..a885d541f15 100644 --- a/scripts/tests/twister/test_testplan.py +++ b/scripts/tests/twister/test_testplan.py @@ -1309,19 +1309,35 @@ def test_testplan_get_all_tests(): TESTDATA_9 = [ - ([], False, 7), - ([], True, 5), - (['good_test/dummy.common.1', 'good_test/dummy.common.2', 'good_test/dummy.common.3'], False, 3), - (['good_test/dummy.common.1', 'good_test/dummy.common.2', 'good_test/dummy.common.3'], True, 0), + ([], False, True, 11, 1), + ([], False, False, 7, 2), + ([], True, False, 9, 1), + ([], True, True, 9, 1), + ([], True, False, 9, 1), + (['good_test/dummy.common.1', 'good_test/dummy.common.2', 'good_test/dummy.common.3'], False, True, 3, 1), + (['good_test/dummy.common.1', 'good_test/dummy.common.2', + 'duplicate_test/dummy.common.1', 'duplicate_test/dummy.common.2'], False, True, 4, 1), + (['dummy.common.1', 'dummy.common.2'], False, False, 2, 1), + (['good_test/dummy.common.1', 'good_test/dummy.common.2', 'good_test/dummy.common.3'], True, True, 0, 1), ] @pytest.mark.parametrize( - 'testsuite_filter, use_alt_root, expected_suite_count', + 'testsuite_filter, use_alt_root, detailed_id, expected_suite_count, expected_errors', TESTDATA_9, - ids=['no testsuite filter', 'no testsuite filter, alt root', - 'testsuite filter', 'testsuite filter, alt root'] + ids=[ + 'no testsuite filter, detailed id', + 'no testsuite filter, short id', + 'no testsuite filter, alt root, detailed id', + 'no filter, alt root, detailed id', + 'no filter, alt root, short id', + 'testsuite filter', + 'testsuite filter and valid duplicate', + 'testsuite filter, short id and duplicate', + 'testsuite filter, alt root', + ] ) -def test_testplan_add_testsuites(tmp_path, testsuite_filter, use_alt_root, expected_suite_count): +def test_testplan_add_testsuites(tmp_path, testsuite_filter, use_alt_root, detailed_id, + expected_errors, expected_suite_count): # tmp_path # ├ tests <- test root # │ ├ good_test @@ -1330,6 +1346,8 @@ def test_testplan_add_testsuites(tmp_path, testsuite_filter, use_alt_root, expec # │ │ └ testcase.yaml # │ ├ good_sample # │ │ └ sample.yaml + # │ ├ duplicate_test + # │ │ └ testcase.yaml # │ └ others # │ └ other.txt # └ other_tests <- alternate test root @@ -1381,6 +1399,25 @@ tests: samplefile_1 = tmp_good_sample_dir / 'sample.yaml' samplefile_1.write_text(samplecase_yaml_1) + tmp_duplicate_test_dir = tmp_test_root_dir / 'duplicate_test' + tmp_duplicate_test_dir.mkdir() + # The duplicate needs to have the same number of tests as these configurations + # can be read either with duplicate_test first, or good_test first, so number + # of selected tests needs to be the same in both situations. + testcase_yaml_4 = """\ +tests: + dummy.common.1: + build_on_all: true + dummy.common.2: + build_on_all: true + dummy.common.3: + build_on_all: true + dummy.special: + build_on_all: false +""" + testfile_4 = tmp_duplicate_test_dir / 'testcase.yaml' + testfile_4.write_text(testcase_yaml_4) + tmp_other_dir = tmp_test_root_dir / 'others' tmp_other_dir.mkdir() _ = tmp_other_dir / 'other.txt' @@ -1402,6 +1439,7 @@ tests: env = mock.Mock( test_roots=[tmp_test_root_dir], + options=mock.Mock(detailed_test_id=detailed_id), alt_config_root=[tmp_alt_test_root_dir] if use_alt_root else [] ) @@ -1410,6 +1448,7 @@ tests: res = testplan.add_testsuites(testsuite_filter) assert res == expected_suite_count + assert testplan.load_errors == expected_errors def test_testplan_str():