From d4fc8fb1f778f25d5d2ab833ff48673c616dff9b Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Thu, 24 Sep 2026 10:16:24 -0300 Subject: [PATCH 1/3] [Bug] th-cli: warn on unmatched test IDs in --tests-list build_test_selection() previously dropped unrecognised test IDs silently. It now returns missing_ids alongside the selection dict, mirroring load_tc_params_mapping(), and run_tests surfaces a warning for any IDs that didn't match a known test case. Closes [#1138](https://github.com/project-chip/certification-tool/issues/1138) --- tests/test_run_tests.py | 5 +++- tests/test_utils.py | 46 ++++++++++++++++++++++++++---------- th_cli/commands/run_tests.py | 8 ++++++- th_cli/utils.py | 19 +++++++++++---- 4 files changed, 58 insertions(+), 20 deletions(-) diff --git a/tests/test_run_tests.py b/tests/test_run_tests.py index 7890082..a59d773 100644 --- a/tests/test_run_tests.py +++ b/tests/test_run_tests.py @@ -570,7 +570,10 @@ def test_run_tests_test_selection_building( with patch( "th_cli.commands.run_tests.convert_nested_to_dict", return_value=sample_default_config_dict ): - mock_build_test_selection.return_value = {"mock_collection": {"mock_suite": {"mock": 1}}} + mock_build_test_selection.return_value = ( + {"mock_collection": {"mock_suite": {"mock": 1}}}, + [], + ) mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() mock_socket.expected_test_case_count.return_value = 0 diff --git a/tests/test_utils.py b/tests/test_utils.py index ce4ffac..d412178 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -42,7 +42,7 @@ def test_build_test_selection_success(self, sample_test_collections: api_models. tests_list = ["TC-ACE-1.1", "TC_ACE_1_3"] # Act - result = build_test_selection(sample_test_collections, tests_list) + result, missing_ids = build_test_selection(sample_test_collections, tests_list) # Assert assert isinstance(result, dict) @@ -51,6 +51,7 @@ def test_build_test_selection_success(self, sample_test_collections: api_models. assert "FirstChipToolSuite" in result["SDK YAML Tests"] assert "TC-ACE-1.1" in result["SDK YAML Tests"]["FirstChipToolSuite"] assert result["SDK YAML Tests"]["FirstChipToolSuite"]["TC-ACE-1.1"] == 1 + assert missing_ids == [] def test_build_test_selection_no_matches(self, sample_test_collections: api_models.TestCollections) -> None: """Test test selection building with no matching tests.""" @@ -58,11 +59,12 @@ def test_build_test_selection_no_matches(self, sample_test_collections: api_mode tests_list = ["TC-NONEXISTENT-1.1"] # Act - result = build_test_selection(sample_test_collections, tests_list) + result, missing_ids = build_test_selection(sample_test_collections, tests_list) # Assert assert isinstance(result, dict) assert len(result) == 0 + assert missing_ids == ["TC-NONEXISTENT-1.1"] def test_build_test_selection_mixed_formats(self, sample_test_collections: api_models.TestCollections) -> None: """Test test selection building with mixed ID formats.""" @@ -70,12 +72,13 @@ def test_build_test_selection_mixed_formats(self, sample_test_collections: api_m tests_list = ["TC-ACE-1.1", "TC_ACE_1_3", "TC.ACE.1.2"] # Act - result = build_test_selection(sample_test_collections, tests_list) + result, missing_ids = build_test_selection(sample_test_collections, tests_list) # Assert assert isinstance(result, dict) # Should normalize formats and find matches assert len(result) > 0 + assert missing_ids == [] def test_build_test_selection_empty_list(self, sample_test_collections: api_models.TestCollections) -> None: """Test test selection building with empty test list.""" @@ -83,11 +86,12 @@ def test_build_test_selection_empty_list(self, sample_test_collections: api_mode tests_list = [] # Act - result = build_test_selection(sample_test_collections, tests_list) + result, missing_ids = build_test_selection(sample_test_collections, tests_list) # Assert assert isinstance(result, dict) assert len(result) == 0 + assert missing_ids == [] def test_build_test_selection_whitespace_handling( self, sample_test_collections: api_models.TestCollections @@ -97,10 +101,25 @@ def test_build_test_selection_whitespace_handling( tests_list = [" TC-ACE-1.1 ", "\tTC_ACE_1_3\t"] # Act - result = build_test_selection(sample_test_collections, tests_list) + result, missing_ids = build_test_selection(sample_test_collections, tests_list) # Assert assert isinstance(result, dict) + assert missing_ids == [] + + def test_build_test_selection_unmatched_ids_reported( + self, sample_test_collections: api_models.TestCollections + ) -> None: + """Test that unmatched IDs are reported alongside valid matches, preserving original spelling.""" + # Arrange + tests_list = ["TC-ACE-1.1", "TC-TYPO-9.9"] + + # Act + result, missing_ids = build_test_selection(sample_test_collections, tests_list) + + # Assert + assert "TC-ACE-1.1" in result["SDK YAML Tests"]["FirstChipToolSuite"] + assert missing_ids == ["TC-TYPO-9.9"] @pytest.mark.unit @@ -740,11 +759,12 @@ def test_build_test_selection_case_insensitive(self, sample_test_collections: ap tests_list = ["tc-ace-1.1", "TC_ACE_1_3"] # Act - result = build_test_selection(sample_test_collections, tests_list) + result, missing_ids = build_test_selection(sample_test_collections, tests_list) # Assert assert isinstance(result, dict) # Should still find matches despite case differences in normalization + assert missing_ids == [] def test_convert_nested_to_dict_special_attributes(self) -> None: """Test that special attributes are properly filtered.""" @@ -808,7 +828,7 @@ def test_lowercase_input_matches_collection_entry( self, sample_test_collections: api_models.TestCollections ) -> None: """All-lowercase input 'tc-ace-1.1' matches the collection entry 'TC-ACE-1.1'.""" - result = build_test_selection(sample_test_collections, ["tc-ace-1.1"]) + result, _ = build_test_selection(sample_test_collections, ["tc-ace-1.1"]) assert "SDK YAML Tests" in result assert "FirstChipToolSuite" in result["SDK YAML Tests"] @@ -819,7 +839,7 @@ def test_mixed_case_input_matches_collection_entry( self, sample_test_collections: api_models.TestCollections ) -> None: """Mixed-case input 'Tc-Ace-1.1' matches the collection entry 'TC-ACE-1.1'.""" - result = build_test_selection(sample_test_collections, ["Tc-Ace-1.1"]) + result, _ = build_test_selection(sample_test_collections, ["Tc-Ace-1.1"]) suite = result.get("SDK YAML Tests", {}).get("FirstChipToolSuite", {}) assert "TC-ACE-1.1" in suite @@ -829,7 +849,7 @@ def test_lowercase_underscore_format_matches_python_test( self, sample_test_collections: api_models.TestCollections ) -> None: """Lowercase 'tc_ace_1_3' matches the Python collection entry 'TC_ACE_1_3'.""" - result = build_test_selection(sample_test_collections, ["tc_ace_1_3"]) + result, _ = build_test_selection(sample_test_collections, ["tc_ace_1_3"]) assert "SDK Python Tests" in result assert "Python Testing Suite" in result["SDK Python Tests"] @@ -838,7 +858,7 @@ def test_lowercase_underscore_format_matches_python_test( def test_uppercase_input_still_matches(self, sample_test_collections: api_models.TestCollections) -> None: """Existing all-uppercase input continues to work after the change.""" - result = build_test_selection(sample_test_collections, ["TC-ACE-1.2"]) + result, _ = build_test_selection(sample_test_collections, ["TC-ACE-1.2"]) suite = result.get("SDK YAML Tests", {}).get("FirstChipToolSuite", {}) assert "TC-ACE-1.2" in suite @@ -848,7 +868,7 @@ def test_original_collection_key_preserved_in_output( self, sample_test_collections: api_models.TestCollections ) -> None: """Output uses the original collection key, not the normalised form.""" - result = build_test_selection(sample_test_collections, ["tc-ace-1.1"]) + result, _ = build_test_selection(sample_test_collections, ["tc-ace-1.1"]) suite = result.get("SDK YAML Tests", {}).get("FirstChipToolSuite", {}) assert "TC-ACE-1.1" in suite # original key preserved @@ -857,7 +877,7 @@ def test_original_collection_key_preserved_in_output( def test_multiple_mixed_case_ids_all_resolved(self, sample_test_collections: api_models.TestCollections) -> None: """Multiple IDs in varying cases are all matched in a single call.""" - result = build_test_selection( + result, _ = build_test_selection( sample_test_collections, ["tc-ace-1.1", "TC-ACE-1.2", "Tc-Cc-1.1"], ) @@ -869,7 +889,7 @@ def test_multiple_mixed_case_ids_all_resolved(self, sample_test_collections: api def test_no_false_positives_for_unrelated_ids(self, sample_test_collections: api_models.TestCollections) -> None: """Selecting one ID by lowercase does not accidentally select other IDs.""" - result = build_test_selection(sample_test_collections, ["tc-ace-1.1"]) + result, _ = build_test_selection(sample_test_collections, ["tc-ace-1.1"]) suite = result.get("SDK YAML Tests", {}).get("FirstChipToolSuite", {}) assert "TC-ACE-1.2" not in suite diff --git a/th_cli/commands/run_tests.py b/th_cli/commands/run_tests.py index 554a21d..66f7bf5 100644 --- a/th_cli/commands/run_tests.py +++ b/th_cli/commands/run_tests.py @@ -299,7 +299,13 @@ async def run_tests( # Retrieve available test collections to build test selection test_collections = await test_collections_api.read_test_collections_api_v1_test_collections__get() - selected_tests_dict = build_test_selection(test_collections, validated_test_ids) + selected_tests_dict, unmatched_test_ids = build_test_selection(test_collections, validated_test_ids) + if unmatched_test_ids: + click.echo( + colorize_warning( + f"No matching test case found for: {', '.join(unmatched_test_ids)} — these will be skipped" + ) + ) click.echo(colorize_key_value("Selected tests", json.dumps(selected_tests_dict, indent=JSON_INDENT))) diff --git a/th_cli/utils.py b/th_cli/utils.py index 2c804ad..4d9d51c 100644 --- a/th_cli/utils.py +++ b/th_cli/utils.py @@ -56,7 +56,7 @@ def __json_string(object: Any) -> str: return json.dumps(object.model_dump(), indent=4, default=str) -def build_test_selection(test_collections, tests_list) -> dict: +def build_test_selection(test_collections, tests_list) -> tuple[dict, list[str]]: """Build the test selection JSON structure from test_collections and tests_list. Args: @@ -64,7 +64,11 @@ def build_test_selection(test_collections, tests_list) -> dict: tests_list: List of test IDs to select Returns: - dict: Dictionary containing selected tests organized by collection and suite + A tuple of: + - ``selected_tests`` – dict containing selected tests organized by + collection and suite. + - ``missing_ids`` – list of TC IDs from *tests_list* that had no + matching test case in *test_collections* (may be empty). Example: tests_list = ["TC-ACE-1.1", "TC_ACE_1_3"] @@ -83,8 +87,12 @@ def build_test_selection(test_collections, tests_list) -> dict: """ selected_tests = {} - # Convert test IDs to a set for faster lookup and normalize them (case-insensitive) - tests_set = {test_id.strip().replace("-", "_").replace(".", "_").upper() for test_id in tests_list} + # Map normalized (case-insensitive) test IDs back to their original spelling, + # so unmatched entries can be reported to the caller as requested. + unmatched_ids = { + test_id.strip().replace("-", "_").replace(".", "_").upper(): test_id.strip() for test_id in tests_list + } + tests_set = set(unmatched_ids) # Iterate through test collections for collection_name, collection in test_collections.test_collections.items(): @@ -100,6 +108,7 @@ def build_test_selection(test_collections, tests_list) -> dict: normalized_test_case_id = test_case_id.replace("-", "_").replace(".", "_").upper() if normalized_test_case_id in tests_set: selected_tests[collection_name][suite_name][test_case_id] = 1 + unmatched_ids.pop(normalized_test_case_id, None) # Remove empty collections and suites selected_tests = { @@ -108,7 +117,7 @@ def build_test_selection(test_collections, tests_list) -> dict: if any(suites.values()) } - return selected_tests + return selected_tests, list(unmatched_ids.values()) def load_json_config(config_path: str) -> dict[str, Any]: From 5520808daf3abbe69e6dfa968f3cee838c9abb2c Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Thu, 24 Sep 2026 14:06:57 -0300 Subject: [PATCH 2/3] Address CodeRabbit review: preserve duplicate-normalized IDs, cover CLI warning build_test_selection() previously overwrote earlier original spellings when two different --tests-list entries normalized to the same key, silently dropping one from missing_ids. Now groups all original spellings per normalized key. Also extends the run_tests command test to actually exercise the unmatched-ID warning path end-to-end. --- tests/test_run_tests.py | 7 +++++-- tests/test_utils.py | 14 ++++++++++++++ th_cli/utils.py | 17 +++++++++++------ 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/tests/test_run_tests.py b/tests/test_run_tests.py index a59d773..0735cdb 100644 --- a/tests/test_run_tests.py +++ b/tests/test_run_tests.py @@ -572,7 +572,7 @@ def test_run_tests_test_selection_building( ): mock_build_test_selection.return_value = ( {"mock_collection": {"mock_suite": {"mock": 1}}}, - [], + ["TC-TYPO-9.9"], ) mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() @@ -583,13 +583,16 @@ def test_run_tests_test_selection_building( mock_socket_class.return_value = mock_socket # Act - result = cli_runner.invoke(run_tests, ["--tests-list", "TC-ACE-1.1,TC-ACE-1.2"]) + result = cli_runner.invoke(run_tests, ["--tests-list", "TC-ACE-1.1,TC-ACE-1.2,TC-TYPO-9.9"]) # Assert assert result.exit_code == 0 mock_build_test_selection.assert_called_once() # Verify the test selection is displayed assert "Selected tests" in result.output + # Verify the unmatched ID is surfaced as a warning + assert "TC-TYPO-9.9" in result.output + assert "these will be skipped" in result.output def test_run_tests_logger_configuration( self, diff --git a/tests/test_utils.py b/tests/test_utils.py index d412178..39a5585 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -121,6 +121,20 @@ def test_build_test_selection_unmatched_ids_reported( assert "TC-ACE-1.1" in result["SDK YAML Tests"]["FirstChipToolSuite"] assert missing_ids == ["TC-TYPO-9.9"] + def test_build_test_selection_duplicate_normalized_unmatched_ids_all_reported( + self, sample_test_collections: api_models.TestCollections + ) -> None: + """Distinct unmatched IDs that normalize to the same key are all reported, not just the last one.""" + # Arrange + tests_list = ["TC-TYPO-9.9", "TC_TYPO_9_9", "tc.typo.9.9"] + + # Act + result, missing_ids = build_test_selection(sample_test_collections, tests_list) + + # Assert + assert result == {} + assert missing_ids == ["TC-TYPO-9.9", "TC_TYPO_9_9", "tc.typo.9.9"] + @pytest.mark.unit class TestConvertNestedToDict: diff --git a/th_cli/utils.py b/th_cli/utils.py index 4d9d51c..7b507c9 100644 --- a/th_cli/utils.py +++ b/th_cli/utils.py @@ -87,11 +87,14 @@ def build_test_selection(test_collections, tests_list) -> tuple[dict, list[str]] """ selected_tests = {} - # Map normalized (case-insensitive) test IDs back to their original spelling, - # so unmatched entries can be reported to the caller as requested. - unmatched_ids = { - test_id.strip().replace("-", "_").replace(".", "_").upper(): test_id.strip() for test_id in tests_list - } + # Map normalized (case-insensitive) test IDs back to their original spelling(s), + # so unmatched entries can be reported to the caller as requested. Multiple + # original spellings can normalize to the same key (e.g. "TC-TYPO-9.9" and + # "TC_TYPO_9_9"), so track them all rather than overwriting. + unmatched_ids: dict[str, list[str]] = {} + for test_id in tests_list: + normalized_id = test_id.strip().replace("-", "_").replace(".", "_").upper() + unmatched_ids.setdefault(normalized_id, []).append(test_id.strip()) tests_set = set(unmatched_ids) # Iterate through test collections @@ -117,7 +120,9 @@ def build_test_selection(test_collections, tests_list) -> tuple[dict, list[str]] if any(suites.values()) } - return selected_tests, list(unmatched_ids.values()) + missing_ids = [original_id for ids in unmatched_ids.values() for original_id in ids] + + return selected_tests, missing_ids def load_json_config(config_path: str) -> dict[str, Any]: From 794933d624142534e5d5fb632db70ed9238c81cd Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Thu, 24 Sep 2026 15:04:53 -0300 Subject: [PATCH 3/3] [Bug] th-cli: abort run-tests when --tests-list has unmatched IDs Warning-and-continuing on unmatched test IDs still started a test run with fewer tests than requested (or zero, if none matched). Raise a CLIError instead so the run never starts until every listed ID resolves to a known test case. Also fixes test_run_tests_various_test_lists, which had been silently passing bogus/nonexistent test IDs (TC-MCORE_FS-*, TC_CADMIN_1_3_4/102, lowercase and dot-separated IDs rejected by validate_test_ids) that this stricter check now correctly surfaces. --- tests/test_run_tests.py | 53 ++++++++++++++++++++++++++++++------ th_cli/commands/run_tests.py | 6 ++-- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/tests/test_run_tests.py b/tests/test_run_tests.py index 0735cdb..e1dc7d9 100644 --- a/tests/test_run_tests.py +++ b/tests/test_run_tests.py @@ -491,10 +491,9 @@ def test_run_tests_required_tests_list_parameter(self, cli_runner: CliRunner) -> "TC-ACE-1.1,TC-ACE-1.2", "TC_ACE_1_1,TC_ACE_1_2,TC_ACE_1_3", "TC_ACE_1_1,TC_ACE_1_2,TC_ACE_1_3,TC_ACE_1_3-custom", - "TC-ACE-1.1, TC-ACE-1.2, TC-ACE-1.3", # with spaces - "TC-MCORE_FS-1.1, TC-MCORE_FS-1_2, TC_MCORE_FS-1.2", - "TC_CADMIN_1_3_4", - "TC_CADMIN_1_3_102", + "TC-ACE-1.1, TC-ACE-1.2, TC-CC-1.1", # with spaces + "TC-ACE-1.1, TC_ACE_1_2, TC-CC-1.1", # mixed separators + "TC_ACE_1_3", ], ) def test_run_tests_various_test_lists( @@ -572,7 +571,7 @@ def test_run_tests_test_selection_building( ): mock_build_test_selection.return_value = ( {"mock_collection": {"mock_suite": {"mock": 1}}}, - ["TC-TYPO-9.9"], + [], ) mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() @@ -583,16 +582,54 @@ def test_run_tests_test_selection_building( mock_socket_class.return_value = mock_socket # Act - result = cli_runner.invoke(run_tests, ["--tests-list", "TC-ACE-1.1,TC-ACE-1.2,TC-TYPO-9.9"]) + result = cli_runner.invoke(run_tests, ["--tests-list", "TC-ACE-1.1,TC-ACE-1.2"]) # Assert assert result.exit_code == 0 mock_build_test_selection.assert_called_once() # Verify the test selection is displayed assert "Selected tests" in result.output - # Verify the unmatched ID is surfaced as a warning + + def test_run_tests_aborts_on_unmatched_test_ids( + self, + cli_runner: CliRunner, + mock_async_apis: Mock, + sample_test_collections: api_models.TestCollections, + sample_test_run_execution: api_models.TestRunExecutionWithChildren, + sample_default_config_dict: dict, + ) -> None: + """Test that the run aborts before creating a test run if any --tests-list ID is unmatched.""" + # Arrange + project_api = mock_async_apis.projects_api.default_config_api_v1_projects_default_config_get + test_collection_api = mock_async_apis.test_collections_api.read_test_collections_api_v1_test_collections__get + test_run_executions_api = mock_async_apis.test_run_executions_api + cli_api = test_run_executions_api.create_cli_test_run_execution_api_v1_test_run_executions_cli_post + + project_api.return_value = sample_default_config_dict + test_collection_api.return_value = sample_test_collections + + with patch("th_cli.commands.run_tests.AsyncApis", return_value=mock_async_apis): + with patch( + "th_cli.commands.run_tests.test_logging.configure_logger_for_run", return_value="./test_logs/test.log" + ): + with patch("th_cli.commands.run_tests.build_test_selection") as mock_build_test_selection: + with patch( + "th_cli.commands.run_tests.convert_nested_to_dict", return_value=sample_default_config_dict + ): + mock_build_test_selection.return_value = ( + {"mock_collection": {"mock_suite": {"mock": 1}}}, + ["TC-TYPO-9.9"], + ) + + # Act + result = cli_runner.invoke(run_tests, ["--tests-list", "TC-ACE-1.1,TC-TYPO-9.9"]) + + # Assert + assert result.exit_code == 1 assert "TC-TYPO-9.9" in result.output - assert "these will be skipped" in result.output + assert "No matching test case found" in result.output + # The run must never be created when a requested ID is unmatched. + cli_api.assert_not_called() def test_run_tests_logger_configuration( self, diff --git a/th_cli/commands/run_tests.py b/th_cli/commands/run_tests.py index 66f7bf5..50659ce 100644 --- a/th_cli/commands/run_tests.py +++ b/th_cli/commands/run_tests.py @@ -301,10 +301,8 @@ async def run_tests( test_collections = await test_collections_api.read_test_collections_api_v1_test_collections__get() selected_tests_dict, unmatched_test_ids = build_test_selection(test_collections, validated_test_ids) if unmatched_test_ids: - click.echo( - colorize_warning( - f"No matching test case found for: {', '.join(unmatched_test_ids)} — these will be skipped" - ) + raise CLIError( + f"No matching test case found for: {', '.join(unmatched_test_ids)} — aborting before starting the run" ) click.echo(colorize_key_value("Selected tests", json.dumps(selected_tests_dict, indent=JSON_INDENT)))