From 708f3dbd609c134088d9ff786b9eba185909cf29 Mon Sep 17 00:00:00 2001 From: joseph-sentry <136376984+joseph-sentry@users.noreply.github.com> Date: Wed, 19 Jun 2024 11:48:54 -0400 Subject: [PATCH] feat: horizontally stack test results message (#504) Signed-off-by: joseph-sentry --- services/test_results.py | 95 ++++++++++--------- services/tests/test_test_results.py | 86 +++++------------ .../tests/unit/test_test_results_finisher.py | 12 +-- 3 files changed, 78 insertions(+), 115 deletions(-) diff --git a/services/test_results.py b/services/test_results.py index e2a08d707..c00bdfe15 100644 --- a/services/test_results.py +++ b/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 = "
".join(envs) - test_description = ( - "
"
-            f"**Testsuite:**
{fail.testsuite}

"
-            f"**Test name:**
{fail.testname}

"
-            f"**Envs:**
{env_section}
"
-            "
" - ) + 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}
**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**
{test_description}" - else: - test_description = f":snowflake::card_index_dividers: **Known Flaky Test**
{test_description}" + if fail.envs: + envs = [f" - {env}" for env in fail.envs] + env_section = "
".join(envs) + test_description = f"{test_description}\n**Flags:**\n{env_section}" - return test_description + return f"{test_description}

" 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}**
" - for msg in flake_messages - ] - ) - return f"{flake_section}
{failure_message}
" - return f"
{failure_message}
" + return f" 
{failure_message}
" 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 = [ - "
View the full list of failed tests", - "", - "| **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 += [ + "
View the full list of failed tests", + "", + ] + + self.process_dict(fail_dict, message) + message.append("
") + + if flake_dict: + message += [ + "
View the full list of flaky tests", + "", + ] + + self.process_dict(flake_dict, message) + message.append("
") 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): """ diff --git a/services/tests/test_test_results.py b/services/tests/test_test_results.py index d75ae4ce6..e7bf8dd73 100644 --- a/services/tests/test_test_results.py +++ b/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 - == "
**Testsuite:**
testsuite

**Test name:**
testname

**Envs:**
- default
" - ) - - @pytest.mark.parametrize( - "is_new_flake,flake_header", - [(False, "Known Flaky Test"), (True, "Newly Detected Flake")], + "testname,message", + [ + ("testname", "- **Test name:** testname

"), + ( + "Test\x1ftestname", + "- **Class name:** Test
**Test name:** testname

", + ), + ], ) -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}**
**Testsuite:**
testsuite

**Test name:**
testname

**Envs:**
- default
" + "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 == "
hello world
" - - -@pytest.mark.parametrize( - "flake_symptoms,message", - [ - ( - [FlakeSymptomType.FAILED_IN_DEFAULT_BRANCH], - ":snowflake: :card_index_dividers: **Failure on default branch**
hello world
", - ), - ( - [ - FlakeSymptomType.FAILED_IN_DEFAULT_BRANCH, - FlakeSymptomType.UNRELATED_MATCHING_FAILURES, - ], - ":snowflake: :card_index_dividers: **Failure on default branch**
:snowflake: :card_index_dividers: **Matching failures on unrelated branches**
hello world
", - ), - ], -) -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 == " 
hello world
" 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.
View the full list of failed tests -| **Test Description** | **Failure message** | -| :-- | :-- | -| 
**Testsuite:**
testsuite

**Test name:**
testname

**Envs:**
- default
| 
hello world
|""" +## testsuite +- **Test name:** testname

+ 
hello world
+
""" ) @@ -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.** -
View the full list of failed tests +
View the full list of flaky tests -| **Test Description** | **Failure message** | -| :-- | :-- | -| :snowflake::card_index_dividers: **Newly Detected Flake**
**Testsuite:**
testsuite

**Test name:**
testname

**Envs:**
- default
| :snowflake: :card_index_dividers: **Failure on default branch**
hello world
|""" +## testsuite +- **Test name:** testname

+ 
hello world
+
""" ) diff --git a/tasks/tests/unit/test_test_results_finisher.py b/tasks/tests/unit/test_test_results_finisher.py index be6bad5b5..15e27fb34 100644 --- a/tasks/tests/unit/test_test_results_finisher.py +++ b/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
View the full list of failed tests\n\n| **Test Description** | **Failure message** |\n| :-- | :-- |\n| 
**Testsuite:**
test_testsuite

**Test name:**
test_name0

**Envs:**
- 0
| 
<pre>Fourth 

</pre> | test  | instance |
|\n| 
**Testsuite:**
test_testsuite

**Test name:**
test_name1

**Envs:**
- 1
| 
Shared failure message
|\n| 
**Testsuite:**
test_testsuite

**Test name:**
test_name2

**Envs:**
- default
| 
Shared failure message
|\n| 
**Testsuite:**
test_testsuite

**Test name:**
test_name3

**Envs:**
- 0
| 
No failure message available
|", + "**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
View the full list of failed tests\n\n## test_testsuite\n- **Class name:** Class Name
**Test name:** test_name0\n**Flags:**\n - 0

\n 
<pre>Fourth 

</pre> | test  | instance |
\n- **Class name:** Other Class Name
**Test name:** test_name2

\n 
Shared failure message
\n- **Test name:** test_name1\n**Flags:**\n - 1

\n 
Shared failure message
\n- **Test name:** test_name3\n**Flags:**\n - 0

\n 
No failure message available
\n
", ) 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
View the full list of failed tests\n\n| **Test Description** | **Failure message** |\n| :-- | :-- |\n| 
**Testsuite:**
test_testsuite

**Test name:**
test_name0

**Envs:**
- 0
| 
<pre>Fourth 

</pre> | test  | instance |
|\n| 
**Testsuite:**
test_testsuite

**Test name:**
test_name1

**Envs:**
- 1
| 
Shared failure message
|\n| 
**Testsuite:**
test_testsuite

**Test name:**
test_name2

**Envs:**
- default
| 
Shared failure message
|\n| 
**Testsuite:**
test_testsuite

**Test name:**
test_name3

**Envs:**
- 0
| 
No failure message available
|", + "**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
View the full list of failed tests\n\n## test_testsuite\n- **Class name:** Class Name
**Test name:** test_name0\n**Flags:**\n - 0

\n 
<pre>Fourth 

</pre> | test  | instance |
\n- **Class name:** Other Class Name
**Test name:** test_name2

\n 
Shared failure message
\n- **Test name:** test_name1\n**Flags:**\n - 1

\n 
Shared failure message
\n- **Test name:** test_name3\n**Flags:**\n - 0

\n 
No failure message available
\n
", ) 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
View the full list of failed tests\n\n| **Test Description** | **Failure message** |\n| :-- | :-- |\n| 
**Testsuite:**
test_testsuite

**Test name:**
test_name0

**Envs:**
- 0
| 
<pre>Fourth 

</pre> | test  | instance |
|\n| 
**Testsuite:**
test_testsuite

**Test name:**
test_name1

**Envs:**
- 1
| 
Shared failure message
|\n| 
**Testsuite:**
test_testsuite

**Test name:**
test_name2

**Envs:**
- default
| 
Shared failure message
|\n| 
**Testsuite:**
test_testsuite

**Test name:**
test_name3

**Envs:**
- 0
| 
No failure message available
|", + "**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
View the full list of failed tests\n\n## test_testsuite\n- **Class name:** Class Name
**Test name:** test_name0\n**Flags:**\n - 0

\n 
<pre>Fourth 

</pre> | test  | instance |
\n- **Class name:** Other Class Name
**Test name:** test_name2

\n 
Shared failure message
\n- **Test name:** test_name1\n**Flags:**\n - 1

\n 
Shared failure message
\n- **Test name:** test_name3\n**Flags:**\n - 0

\n 
No failure message available
\n
", ) 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
View the full list of failed tests\n\n| **Test Description** | **Failure message** |\n| :-- | :-- |\n| :snowflake::card_index_dividers: **Newly Detected Flake**
**Testsuite:**
test_testsuite

**Test name:**
test_name0

**Envs:**
- 0
| :snowflake: :card_index_dividers: **Failure on default branch**
<pre>Fourth 

</pre> | test  | instance |
|\n| :snowflake::card_index_dividers: **Newly Detected Flake**
**Testsuite:**
test_testsuite

**Test name:**
test_name1

**Envs:**
- 1
| :snowflake: :card_index_dividers: **Failure on default branch**
Shared failure message
|\n| :snowflake::card_index_dividers: **Newly Detected Flake**
**Testsuite:**
test_testsuite

**Test name:**
test_name2

**Envs:**
- default
| :snowflake: :card_index_dividers: **Failure on default branch**
Shared failure message
|\n| :snowflake::card_index_dividers: **Newly Detected Flake**
**Testsuite:**
test_testsuite

**Test name:**
test_name3

**Envs:**
- 0
| :snowflake: :card_index_dividers: **Failure on default branch**
No failure message available
|", + "**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
View the full list of flaky tests\n\n## test_testsuite\n- **Class name:** Class Name
**Test name:** test_name0\n**Flags:**\n - 0

\n 
<pre>Fourth 

</pre> | test  | instance |
\n- **Class name:** Other Class Name
**Test name:** test_name2

\n 
Shared failure message
\n- **Test name:** test_name1\n**Flags:**\n - 1

\n 
Shared failure message
\n- **Test name:** test_name3\n**Flags:**\n - 0

\n 
No failure message available
\n
", ) mock_metrics.incr.assert_has_calls(