feat: horizontally stack test results message (#504)

Signed-off-by: joseph-sentry <[email protected]>

services/test_results.py

@@ -1,4 +1,5 @@
 import logging
+from collections import defaultdict
 from dataclasses import dataclass
 from hashlib import sha256
 from typing import List, Mapping, Sequence, Tuple
@@ -170,52 +171,33 @@ async def send_to_provider(self, message):
     def generate_test_description(
         self,
         fail: TestResultsNotificationFailure,
-        flake: TestResultsNotificationFlake | None = None,
     ):
-        envs = [f"- {env}" for env in fail.envs] or ["- default"]
-        env_section = "<br>".join(envs)
-        test_description = (
-            "<pre>"
-            f"**Testsuite:**<br>{fail.testsuite}<br><br>"
-            f"**Test name:**<br>{fail.testname}<br><br>"
-            f"**Envs:**<br>{env_section}<br>"
-            "</pre>"
-        )
+        has_class_name = "\x1f" in fail.testname
+        if has_class_name:
+            class_name, test_name = fail.testname.split("\x1f")
+            test_description = (
+                f"- **Class name:** {class_name}<br>**Test name:** {test_name}"
+            )
+        else:
+            test_description = f"- **Test name:** {fail.testname}"
 
-        if flake is not None:
-            if flake.is_new_flake:
-                test_description = f":snowflake::card_index_dividers: **Newly Detected Flake**<br>{test_description}"
-            else:
-                test_description = f":snowflake::card_index_dividers: **Known Flaky Test**<br>{test_description}"
+        if fail.envs:
+            envs = [f"  - {env}" for env in fail.envs]
+            env_section = "<br>".join(envs)
+            test_description = f"{test_description}\n**Flags:**\n{env_section}"
 
-        return test_description
+        return f"{test_description}<br><br>"
 
     def generate_failure_info(
         self,
         fail: TestResultsNotificationFailure,
-        flake: TestResultsNotificationFlake | None = None,
     ):
         if fail.failure_message is not None:
             failure_message = fail.failure_message
         else:
             failure_message = "No failure message available"
 
-        if flake is not None:
-            flake_messages = []
-            if FlakeSymptomType.FAILED_IN_DEFAULT_BRANCH in flake.flake_type:
-                flake_messages.append("Failure on default branch")
-            if FlakeSymptomType.CONSECUTIVE_DIFF_OUTCOMES in flake.flake_type:
-                flake_messages.append("Differing outcomes on the same commit")
-            if FlakeSymptomType.UNRELATED_MATCHING_FAILURES in flake.flake_type:
-                flake_messages.append("Matching failures on unrelated branches")
-            flake_section = "".join(
-                [
-                    f":snowflake: :card_index_dividers: **{msg}**<br>"
-                    for msg in flake_messages
-                ]
-            )
-            return f"{flake_section}<pre>{failure_message}</pre>"
-        return f"<pre>{failure_message}</pre>"
+        return f"  <pre>{failure_message}</pre>"
 
     def build_message(self, payload: TestResultsNotificationPayload) -> str:
         message = []
@@ -260,23 +242,35 @@ def build_message(self, payload: TestResultsNotificationPayload) -> str:
             results = f"Completed {completed} tests with **`{payload.failed} failed`**, {payload.passed} passed and {payload.skipped} skipped."
             message.append(results)
 
-        details = [
-            "<details><summary>View the full list of failed tests</summary>",
-            "",
-            "| **Test Description** | **Failure message** |",
-            "| :-- | :-- |",
-        ]
-
-        message += details
-
+        fail_dict = defaultdict(list)
+        flake_dict = defaultdict(list)
         for fail in payload.failures:
             flake = None
             if payload.flaky_tests is not None:
                 flake = payload.flaky_tests.get(fail.test_id, None)
-            test_description = self.generate_test_description(fail, flake)
-            failure_information = self.generate_failure_info(fail, flake)
-            single_test_row = f"| {test_description} | {failure_information} |"
-            message.append(single_test_row)
+
+            if flake is not None:
+                flake_dict[fail.testsuite].append(fail)
+            else:
+                fail_dict[fail.testsuite].append(fail)
+
+        if fail_dict:
+            message += [
+                "<details><summary>View the full list of failed tests</summary>",
+                "",
+            ]
+
+            self.process_dict(fail_dict, message)
+            message.append("</details>")
+
+        if flake_dict:
+            message += [
+                "<details><summary>View the full list of flaky tests</summary>",
+                "",
+            ]
+
+            self.process_dict(flake_dict, message)
+            message.append("</details>")
 
         return "\n".join(message)
 
@@ -333,6 +327,15 @@ async def error_comment(self):
 
         return (True, "comment_posted")
 
+    def process_dict(self, d, message):
+        for testsuite, fail_list in d.items():
+            message.append(f"## {testsuite}")
+            for fail in fail_list:
+                test_description = self.generate_test_description(fail)
+                message.append(test_description)
+                failure_information = self.generate_failure_info(fail)
+                message.append(failure_information)
+
 
 def latest_test_instances_for_a_given_commit(db_session, commit_id):
     """

services/tests/test_test_results.py

@@ -68,38 +68,24 @@ async def test_send_to_provider_fail(tn):
     assert res == False
 
 
-def test_generate_test_description(tn):
-    flags_hash = generate_flags_hash([])
-    test_id = generate_test_id(1, "testsuite", "testname", flags_hash)
-    fail = TestResultsNotificationFailure(
-        "hello world", "testsuite", "testname", [], test_id
-    )
-    res = tn.generate_test_description(fail)
-    assert (
-        res
-        == "<pre>**Testsuite:**<br>testsuite<br><br>**Test name:**<br>testname<br><br>**Envs:**<br>- default<br></pre>"
-    )
-
-
 @pytest.mark.parametrize(
-    "is_new_flake,flake_header",
-    [(False, "Known Flaky Test"), (True, "Newly Detected Flake")],
+    "testname,message",
+    [
+        ("testname", "- **Test name:** testname<br><br>"),
+        (
+            "Test\x1ftestname",
+            "- **Class name:** Test<br>**Test name:** testname<br><br>",
+        ),
+    ],
 )
-def test_generate_test_description_with_flake(tn, is_new_flake, flake_header):
+def test_generate_test_description(tn, testname, message):
     flags_hash = generate_flags_hash([])
-    test_id = generate_test_id(1, "testsuite", "testname", flags_hash)
+    test_id = generate_test_id(1, "testsuite", testname, flags_hash)
     fail = TestResultsNotificationFailure(
-        "hello world", "testsuite", "testname", [], test_id
-    )
-    flake = TestResultsNotificationFlake(
-        [],
-        is_new_flake,
-    )
-    res = tn.generate_test_description(fail, flake)
-    assert (
-        res
-        == f":snowflake::card_index_dividers: **{flake_header}**<br><pre>**Testsuite:**<br>testsuite<br><br>**Test name:**<br>testname<br><br>**Envs:**<br>- default<br></pre>"
+        "hello world", "testsuite", testname, [], test_id
     )
+    res = tn.generate_test_description(fail)
+    assert res == message
 
 
 def test_generate_failure_info(tn):
@@ -111,35 +97,7 @@ def test_generate_failure_info(tn):
 
     res = tn.generate_failure_info(fail)
 
-    assert res == "<pre>hello world</pre>"
-
-
-@pytest.mark.parametrize(
-    "flake_symptoms,message",
-    [
-        (
-            [FlakeSymptomType.FAILED_IN_DEFAULT_BRANCH],
-            ":snowflake: :card_index_dividers: **Failure on default branch**<br><pre>hello world</pre>",
-        ),
-        (
-            [
-                FlakeSymptomType.FAILED_IN_DEFAULT_BRANCH,
-                FlakeSymptomType.UNRELATED_MATCHING_FAILURES,
-            ],
-            ":snowflake: :card_index_dividers: **Failure on default branch**<br>:snowflake: :card_index_dividers: **Matching failures on unrelated branches**<br><pre>hello world</pre>",
-        ),
-    ],
-)
-def test_generate_failure_info_with_flake(tn, flake_symptoms, message):
-    flags_hash = generate_flags_hash([])
-    test_id = generate_test_id(1, "testsuite", "testname", flags_hash)
-    fail = TestResultsNotificationFailure(
-        "hello world", "testsuite", "testname", [], test_id
-    )
-    flake = TestResultsNotificationFlake(flake_symptoms, True)
-
-    res = tn.generate_failure_info(fail, flake)
-    assert res == message
+    assert res == "  <pre>hello world</pre>"
 
 
 def test_build_message(tn):
@@ -159,9 +117,10 @@ def test_build_message(tn):
 Completed 6 tests with **`1 failed`**, 2 passed and 3 skipped.
 <details><summary>View the full list of failed tests</summary>
 
-| **Test Description** | **Failure message** |
-| :-- | :-- |
-| <pre>**Testsuite:**<br>testsuite<br><br>**Test name:**<br>testname<br><br>**Envs:**<br>- default<br></pre> | <pre>hello world</pre> |"""
+## testsuite
+- **Test name:** testname<br><br>
+  <pre>hello world</pre>
+</details>"""
     )
 
 
@@ -185,11 +144,12 @@ def test_build_message_with_flake(tn):
 ### :x: Failed Test Results: 
 Completed 6 tests with **`1 failed`**(1 newly detected flaky), 2 passed and 3 skipped.
 - Total :snowflake:**1 flaky tests.**
-<details><summary>View the full list of failed tests</summary>
+<details><summary>View the full list of flaky tests</summary>
 
-| **Test Description** | **Failure message** |
-| :-- | :-- |
-| :snowflake::card_index_dividers: **Newly Detected Flake**<br><pre>**Testsuite:**<br>testsuite<br><br>**Test name:**<br>testname<br><br>**Envs:**<br>- default<br></pre> | :snowflake: :card_index_dividers: **Failure on default branch**<br><pre>hello world</pre> |"""
+## testsuite
+- **Test name:** testname<br><br>
+  <pre>hello world</pre>
+</details>"""
     )
 
 

tasks/tests/unit/test_test_results_finisher.py

@@ -112,7 +112,7 @@ def test_results_setup(mocker, dbsession):
     test1 = Test(
         id_=test_id1,
         repoid=repoid,
-        name=test_name + "0",
+        name="Class Name\x1f" + test_name + "0",
         testsuite=test_suite,
         flags_hash="a",
     )
@@ -132,7 +132,7 @@ def test_results_setup(mocker, dbsession):
     test3 = Test(
         id_=test_id3,
         repoid=repoid,
-        name=test_name + "2",
+        name="Other Class Name\x1f" + test_name + "2",
         testsuite=test_suite,
         flags_hash="",
     )
@@ -337,7 +337,7 @@ def test_upload_finisher_task_call(
         assert expected_result == result
         mock_repo_provider_comments.post_comment.assert_called_with(
             pull.pullid,
-            "**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 4 tests with **`4 failed`**, 0 passed and 0 skipped.\n<details><summary>View the full list of failed tests</summary>\n\n| **Test Description** | **Failure message** |\n| :-- | :-- |\n| <pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name0<br><br>**Envs:**<br>- 0<br></pre> | <pre>&lt;pre&gt;Fourth <br><br>&lt;/pre&gt; | test  | instance |</pre> |\n| <pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name1<br><br>**Envs:**<br>- 1<br></pre> | <pre>Shared failure message</pre> |\n| <pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name2<br><br>**Envs:**<br>- default<br></pre> | <pre>Shared failure message</pre> |\n| <pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name3<br><br>**Envs:**<br>- 0<br></pre> | <pre>No failure message available</pre> |",
+            "**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 4 tests with **`4 failed`**, 0 passed and 0 skipped.\n<details><summary>View the full list of failed tests</summary>\n\n## test_testsuite\n- **Class name:** Class Name<br>**Test name:** test_name0\n**Flags:**\n  - 0<br><br>\n  <pre>&lt;pre&gt;Fourth <br><br>&lt;/pre&gt; | test  | instance |</pre>\n- **Class name:** Other Class Name<br>**Test name:** test_name2<br><br>\n  <pre>Shared failure message</pre>\n- **Test name:** test_name1\n**Flags:**\n  - 1<br><br>\n  <pre>Shared failure message</pre>\n- **Test name:** test_name3\n**Flags:**\n  - 0<br><br>\n  <pre>No failure message available</pre>\n</details>",
         )
 
         mock_metrics.incr.assert_has_calls(
@@ -537,7 +537,7 @@ def test_upload_finisher_task_call_existing_comment(
         mock_repo_provider_comments.edit_comment.assert_called_with(
             pull.pullid,
             1,
-            "**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 4 tests with **`4 failed`**, 0 passed and 0 skipped.\n<details><summary>View the full list of failed tests</summary>\n\n| **Test Description** | **Failure message** |\n| :-- | :-- |\n| <pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name0<br><br>**Envs:**<br>- 0<br></pre> | <pre>&lt;pre&gt;Fourth <br><br>&lt;/pre&gt; | test  | instance |</pre> |\n| <pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name1<br><br>**Envs:**<br>- 1<br></pre> | <pre>Shared failure message</pre> |\n| <pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name2<br><br>**Envs:**<br>- default<br></pre> | <pre>Shared failure message</pre> |\n| <pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name3<br><br>**Envs:**<br>- 0<br></pre> | <pre>No failure message available</pre> |",
+            "**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 4 tests with **`4 failed`**, 0 passed and 0 skipped.\n<details><summary>View the full list of failed tests</summary>\n\n## test_testsuite\n- **Class name:** Class Name<br>**Test name:** test_name0\n**Flags:**\n  - 0<br><br>\n  <pre>&lt;pre&gt;Fourth <br><br>&lt;/pre&gt; | test  | instance |</pre>\n- **Class name:** Other Class Name<br>**Test name:** test_name2<br><br>\n  <pre>Shared failure message</pre>\n- **Test name:** test_name1\n**Flags:**\n  - 1<br><br>\n  <pre>Shared failure message</pre>\n- **Test name:** test_name3\n**Flags:**\n  - 0<br><br>\n  <pre>No failure message available</pre>\n</details>",
         )
 
         assert expected_result == result
@@ -641,13 +641,13 @@ def test_upload_finisher_task_call_with_flaky(
 
         mock_repo_provider_comments.post_comment.assert_called_with(
             pull.pullid,
-            "**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 4 tests with **`4 failed`**, 0 passed and 0 skipped.\n<details><summary>View the full list of failed tests</summary>\n\n| **Test Description** | **Failure message** |\n| :-- | :-- |\n| <pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name0<br><br>**Envs:**<br>- 0<br></pre> | <pre>&lt;pre&gt;Fourth <br><br>&lt;/pre&gt; | test  | instance |</pre> |\n| <pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name1<br><br>**Envs:**<br>- 1<br></pre> | <pre>Shared failure message</pre> |\n| <pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name2<br><br>**Envs:**<br>- default<br></pre> | <pre>Shared failure message</pre> |\n| <pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name3<br><br>**Envs:**<br>- 0<br></pre> | <pre>No failure message available</pre> |",
+            "**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 4 tests with **`4 failed`**, 0 passed and 0 skipped.\n<details><summary>View the full list of failed tests</summary>\n\n## test_testsuite\n- **Class name:** Class Name<br>**Test name:** test_name0\n**Flags:**\n  - 0<br><br>\n  <pre>&lt;pre&gt;Fourth <br><br>&lt;/pre&gt; | test  | instance |</pre>\n- **Class name:** Other Class Name<br>**Test name:** test_name2<br><br>\n  <pre>Shared failure message</pre>\n- **Test name:** test_name1\n**Flags:**\n  - 1<br><br>\n  <pre>Shared failure message</pre>\n- **Test name:** test_name3\n**Flags:**\n  - 0<br><br>\n  <pre>No failure message available</pre>\n</details>",
         )
 
         mock_repo_provider_comments.edit_comment.assert_called_with(
             pull.pullid,
             1,
-            "**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 4 tests with **`4 failed`**(4 newly detected flaky), 0 passed and 0 skipped.\n- Total :snowflake:**4 flaky tests.**\n<details><summary>View the full list of failed tests</summary>\n\n| **Test Description** | **Failure message** |\n| :-- | :-- |\n| :snowflake::card_index_dividers: **Newly Detected Flake**<br><pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name0<br><br>**Envs:**<br>- 0<br></pre> | :snowflake: :card_index_dividers: **Failure on default branch**<br><pre>&lt;pre&gt;Fourth <br><br>&lt;/pre&gt; | test  | instance |</pre> |\n| :snowflake::card_index_dividers: **Newly Detected Flake**<br><pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name1<br><br>**Envs:**<br>- 1<br></pre> | :snowflake: :card_index_dividers: **Failure on default branch**<br><pre>Shared failure message</pre> |\n| :snowflake::card_index_dividers: **Newly Detected Flake**<br><pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name2<br><br>**Envs:**<br>- default<br></pre> | :snowflake: :card_index_dividers: **Failure on default branch**<br><pre>Shared failure message</pre> |\n| :snowflake::card_index_dividers: **Newly Detected Flake**<br><pre>**Testsuite:**<br>test_testsuite<br><br>**Test name:**<br>test_name3<br><br>**Envs:**<br>- 0<br></pre> | :snowflake: :card_index_dividers: **Failure on default branch**<br><pre>No failure message available</pre> |",
+            "**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 4 tests with **`4 failed`**(4 newly detected flaky), 0 passed and 0 skipped.\n- Total :snowflake:**4 flaky tests.**\n<details><summary>View the full list of flaky tests</summary>\n\n## test_testsuite\n- **Class name:** Class Name<br>**Test name:** test_name0\n**Flags:**\n  - 0<br><br>\n  <pre>&lt;pre&gt;Fourth <br><br>&lt;/pre&gt; | test  | instance |</pre>\n- **Class name:** Other Class Name<br>**Test name:** test_name2<br><br>\n  <pre>Shared failure message</pre>\n- **Test name:** test_name1\n**Flags:**\n  - 1<br><br>\n  <pre>Shared failure message</pre>\n- **Test name:** test_name3\n**Flags:**\n  - 0<br><br>\n  <pre>No failure message available</pre>\n</details>",
         )
 
         mock_metrics.incr.assert_has_calls(