From b9e11dd014ff0f69779785bd91e33aa72517456e Mon Sep 17 00:00:00 2001
From: Frank Niessink <frank@niessink.com>
Date: Thu, 26 Sep 2024 17:40:53 +0200
Subject: [PATCH] Fix incorrect tests count for JUnit XML.

The 'tests' metric with JUnit XML files as source would report incorrect results if the JUnit XML files contain test case names that are not unique across test suites.

Fixes #9872.
---
 .../src/source_collectors/junit/tests.py      |  5 +++-
 .../source_collectors/junit/test_tests.py     | 24 ++++++++++++++-----
 docs/src/changelog.md                         | 10 ++++++++
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/components/collector/src/source_collectors/junit/tests.py b/components/collector/src/source_collectors/junit/tests.py
index 02b096a32b..2112e5091c 100644
--- a/components/collector/src/source_collectors/junit/tests.py
+++ b/components/collector/src/source_collectors/junit/tests.py
@@ -39,5 +39,8 @@ def _include_entity(self, entity: Entity) -> bool:
     @staticmethod
     def __entity(case_node: Element, case_result: str) -> Entity:
         """Transform a test case into a test case entity."""
+        class_name = case_node.get("classname", "")
         name = case_node.get("name", "<nameless test case>")
-        return Entity(key=name, name=name, class_name=case_node.get("classname", ""), test_result=case_result)
+        key = f"{class_name}:{name}"
+        old_key = name  # Key was changed after Quality-time 5.16.1, enable migration of user entity data
+        return Entity(key=key, old_key=old_key, name=name, class_name=class_name, test_result=case_result)
diff --git a/components/collector/tests/source_collectors/junit/test_tests.py b/components/collector/tests/source_collectors/junit/test_tests.py
index 9557b1430d..19ff01395e 100644
--- a/components/collector/tests/source_collectors/junit/test_tests.py
+++ b/components/collector/tests/source_collectors/junit/test_tests.py
@@ -12,11 +12,11 @@ def setUp(self):
         """Extend to set up JUnit test data."""
         super().setUp()
         self.expected_entities = [
-            {"key": "tc1", "name": "tc1", "class_name": "cn", "test_result": "passed"},
-            {"key": "tc2", "name": "tc2", "class_name": "cn", "test_result": "passed"},
-            {"key": "tc3", "name": "tc3", "class_name": "cn", "test_result": "failed"},
-            {"key": "tc4", "name": "tc4", "class_name": "cn", "test_result": "errored"},
-            {"key": "tc5", "name": "tc5", "class_name": "cn", "test_result": "skipped"},
+            {"key": "cn:tc1", "old_key": "tc1", "name": "tc1", "class_name": "cn", "test_result": "passed"},
+            {"key": "cn:tc2", "old_key": "tc2", "name": "tc2", "class_name": "cn", "test_result": "passed"},
+            {"key": "cn:tc3", "old_key": "tc3", "name": "tc3", "class_name": "cn", "test_result": "failed"},
+            {"key": "cn:tc4", "old_key": "tc4", "name": "tc4", "class_name": "cn", "test_result": "errored"},
+            {"key": "cn:tc5", "old_key": "tc5", "name": "tc5", "class_name": "cn", "test_result": "skipped"},
         ]
 
     async def test_tests(self):
@@ -32,7 +32,7 @@ async def test_failed_tests(self):
             response,
             value="1",
             total="5",
-            entities=[{"key": "tc3", "name": "tc3", "class_name": "cn", "test_result": "failed"}],
+            entities=[{"key": "cn:tc3", "old_key": "tc3", "name": "tc3", "class_name": "cn", "test_result": "failed"}],
         )
 
     async def test_zipped_junit_report(self):
@@ -45,3 +45,15 @@ async def test_empty_test_suites(self):
         """Test that a JUnit XML file with an empty testsuites node works."""
         response = await self.collect(get_request_text=self.JUNIT_XML_EMPTY_TEST_SUITES)
         self.assert_measurement(response, value="0", total="0")
+
+    async def test_repeated_test_case_names(self):
+        """Test that repeated test case names are distinguished."""
+        junit_xml_repeated_test_case_name = """<testsuites>
+        <testsuite timestamp="2009-12-19T17:58:59">
+            <testcase name="tc1" classname="cn1"/>
+            <testcase name="tc2" classname="cn1"/>
+            <testcase name="tc1" classname="cn2"/>
+            <testcase name="tc2" classname="cn2"/>
+        </testsuite></testsuites>"""
+        response = await self.collect(get_request_text=junit_xml_repeated_test_case_name)
+        self.assert_measurement(response, value="4", total="4")
diff --git a/docs/src/changelog.md b/docs/src/changelog.md
index 5bfea04b6f..72361964ab 100644
--- a/docs/src/changelog.md
+++ b/docs/src/changelog.md
@@ -8,6 +8,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 <!-- The line "## <square-bracket>Unreleased</square-bracket>" is replaced by the release/release.py script with the new release version and release date. -->
 
+## [Unreleased]
+
+### Deployment notes
+
+If your currently installed *Quality-time* version is not v5.16.1, please first check the upgrade path in the [versioning policy](versioning.md).
+
+### Fixed
+
+- The 'tests' metric with JUnit XML files as source would report incorrect results if the JUnit XML files contain test case names that are not unique across test suites. Fixes [#9872](https://github.com/ICTU/quality-time/issues/9872).
+
 ## v5.16.1 - 2024-09-26
 
 ### Deployment notes