[CI] Explain Flaky Failures

This patch adds functionality to the premerge advisor to explain away
flaky failures. For this, we just look to see that the failure observed
is failing for a sufficient amount of time, in our case 200 commits.

Pull Request: llvm#641

Author: boomanaiden154

premerge/advisor/advisor_lib.py

@@ -143,8 +143,10 @@ def _try_explain_flaky_failure(
     ).fetchall()
     commit_indices = []
     for failure_message, commit_index in test_name_matches:
-        if failure_message == test_failure.message:
+        if failure_message == test_failure["message"]:
             commit_indices.append(commit_index)
+    if len(commit_indices) == 0:
+        return None
     commit_range = max(commit_indices) - min(commit_indices)
     if commit_range > EXPLAINED_FLAKY_MIN_COMMIT_RANGE:
         return {
@@ -165,23 +167,26 @@ def explain_failures(
         commit_index = git_utils.get_commit_index(
             explanation_request["base_commit_sha"], repository_path, db_connection
         )
-        explained_at_head = _try_explain_failing_at_head(
+        # We want to try and explain flaky failures first. Otherwise we might
+        # explain a flaky failure as a failure at head if there is a recent
+        # failure in the last couple of commits.
+        explained_as_flaky = _try_explain_flaky_failure(
             db_connection,
             test_failure,
-            explanation_request["base_commit_sha"],
-            commit_index,
             explanation_request["platform"],
         )
-        if explained_at_head:
-            explanations.append(explained_at_head)
+        if explained_as_flaky:
+            explanations.append(explained_as_flaky)
             continue
-        explained_as_flaky = _try_explain_flaky_failure(
+        explained_at_head = _try_explain_failing_at_head(
             db_connection,
             test_failure,
+            explanation_request["base_commit_sha"],
+            commit_index,
             explanation_request["platform"],
         )
-        if explained_as_flaky:
-            explanations.append(explained_as_flaky)
+        if explained_at_head:
+            explanations.append(explained_at_head)
             continue
         explanations.append(
             {"name": test_failure["name"], "explained": False, "reason": None}

premerge/advisor/advisor_lib_test.py

@@ -86,6 +86,9 @@ def setUp(self):
             [
                 ("8d29a3bb6f3d92d65bf5811b53bf42bf63685359", 1),
                 ("6b7064686b706f7064656d6f6e68756e74657273", 2),
+                ("6a6f73687561747265656a6f7368756174726565", 201),
+                ("6269677375726269677375726269677375726269", 202),
+                ("6d746c616e676c65796d746c616e676c65796d74", 203),
             ],
         )
         self.repository_path_dir = tempfile.TemporaryDirectory()
@@ -281,11 +284,102 @@ def test_no_explain_different_message(self):
             ],
         )
 
+    def _setup_flaky_test_info(
+        self,
+        source_type="postcommit",
+        message="failed in way 1",
+        second_failure_sha="6269677375726269677375726269677375726269",
+    ):
+        failures_info = [
+            {
+                "source_type": source_type,
+                "base_commit_sha": "8d29a3bb6f3d92d65bf5811b53bf42bf63685359",
+                "source_id": "10000",
+                "failures": [
+                    {"name": "a.ll", "message": message},
+                ],
+                "platform": "linux-x86_64",
+            },
+            {
+                "source_type": source_type,
+                "base_commit_sha": second_failure_sha,
+                "source_id": "100001",
+                "failures": [
+                    {"name": "a.ll", "message": message},
+                ],
+                "platform": "linux-x86_64",
+            },
+        ]
+        for failure_info in failures_info:
+            advisor_lib.upload_failures(
+                failure_info, self.db_connection, self.repository_path
+            )
+
+    def _get_flaky_test_explanations(self):
+        explanation_request = {
+            "failures": [{"name": "a.ll", "message": "failed in way 1"}],
+            "base_commit_sha": "6d746c616e676c65796d746c616e676c65796d74",
+            "platform": "linux-x86_64",
+        }
+        return advisor_lib.explain_failures(
+            explanation_request, self.repository_path, self.db_connection
+        )
+
     def test_explain_flaky(self):
-        pass
+        self._setup_flaky_test_info()
+        self.assertListEqual(
+            self._get_flaky_test_explanations(),
+            [
+                {
+                    "name": "a.ll",
+                    "explained": True,
+                    "reason": "This test is flaky in main.",
+                }
+            ],
+        )
+
+    def test_no_explain_flaky_different_message(self):
+        self._setup_flaky_test_info(message="failed in way 2")
+        self.assertListEqual(
+            self._get_flaky_test_explanations(),
+            [
+                {
+                    "name": "a.ll",
+                    "explained": False,
+                    "reason": None,
+                }
+            ],
+        )
 
+    # Test that we do not explain away flaky failures from pull request data.
+    # PRs might have the same failures multiple times across a large span of
+    # base commits, which might accidentally trigger the heuristic.
     def test_no_explain_flaky_pullrequest_data(self):
-        pass
+        self._setup_flaky_test_info(source_type="pull_request")
+        self.assertListEqual(
+            self._get_flaky_test_explanations(),
+            [
+                {
+                    "name": "a.ll",
+                    "explained": False,
+                    "reason": None,
+                }
+            ],
+        )
 
-    def test_no_explain_flaky_long_subsequence(self):
-        pass
+    # Test that if all of the flaky failures are within a small range, we do
+    # not report this as a flaky failure.
+    def test_no_explain_flaky_small_range(self):
+        self._setup_flaky_test_info(
+            second_failure_sha="6b7064686b706f7064656d6f6e68756e74657273"
+        )
+        self.assertListEqual(
+            self._get_flaky_test_explanations(),
+            [
+                {
+                    "name": "a.ll",
+                    "explained": False,
+                    "reason": None,
+                }
+            ],
+        )