From 9b0c33d15d204f80a44ef64da41bc572ba753761 Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Fri, 31 Oct 2025 17:01:56 -0700 Subject: [PATCH 1/2] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?= =?UTF-8?q?anges=20to=20main=20this=20commit=20is=20based=20on?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.7 [skip ci] --- premerge/advisor/advisor_lib.py | 39 +++++++++++++++++++++++++--- premerge/advisor/advisor_lib_test.py | 9 +++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/premerge/advisor/advisor_lib.py b/premerge/advisor/advisor_lib.py index 3348928f6..8b0b4a2cf 100644 --- a/premerge/advisor/advisor_lib.py +++ b/premerge/advisor/advisor_lib.py @@ -36,6 +36,7 @@ class TestExplanationRequest[TypedDict]: } EXPLAINED_HEAD_MAX_COMMIT_INDEX_DIFFERENCE = 5 +EXPLAINED_FLAKY_MIN_COMMIT_RANGE = 200 def _create_table(table_name: str, connection: sqlite3.Connection): @@ -131,6 +132,29 @@ def _try_explain_failing_at_head( return None +def _try_explain_flaky_failure( + db_connection: sqlite3.Connection, + test_failure: TestFailure, + platform: str, +) -> FailureExplanation | None: + test_name_matches = db_connection.execute( + "SELECT failure_message, commit_index FROM failures WHERE source_type='postcommit' AND platform=?", + (platform,), + ).fetchall() + commit_indices = [] + for failure_message, commit_index in test_name_matches: + if failure_message == test_failure.message: + commit_indices.append(commit_index) + commit_range = max(commit_indices) - min(commit_indices) + if commit_range > EXPLAINED_FLAKY_MIN_COMMIT_RANGE: + return { + "name": test_failure["name"], + "explained": True, + "reason": "This test is flaky in main.", + } + return None + + def explain_failures( explanation_request: TestExplanationRequest, repository_path: str, @@ -138,18 +162,27 @@ def explain_failures( ) -> list[FailureExplanation]: explanations = [] for test_failure in explanation_request["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( db_connection, test_failure, explanation_request["base_commit_sha"], - git_utils.get_commit_index( - explanation_request["base_commit_sha"], repository_path, db_connection - ), + commit_index, explanation_request["platform"], ) if explained_at_head: explanations.append(explained_at_head) continue + explained_as_flaky = _try_explain_flaky_failure( + db_connection, + test_failure, + explanation_request["platform"], + ) + if explained_as_flaky: + explanations.append(explained_as_flaky) + continue explanations.append( {"name": test_failure["name"], "explained": False, "reason": None} ) diff --git a/premerge/advisor/advisor_lib_test.py b/premerge/advisor/advisor_lib_test.py index ebbfe59b5..71cc0bc63 100644 --- a/premerge/advisor/advisor_lib_test.py +++ b/premerge/advisor/advisor_lib_test.py @@ -280,3 +280,12 @@ def test_no_explain_different_message(self): } ], ) + + def test_explain_flaky(self): + pass + + def test_no_explain_flaky_pullrequest_data(self): + pass + + def test_no_explain_flaky_long_subsequence(self): + pass From eb6b6d8c0cc8ccfd498ecd294850248ab24ecfb1 Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Mon, 10 Nov 2025 16:48:22 -0800 Subject: [PATCH 2/2] Add comment Created using spr 1.3.7 --- premerge/advisor/advisor_lib.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/premerge/advisor/advisor_lib.py b/premerge/advisor/advisor_lib.py index 3d21ea167..520496abc 100644 --- a/premerge/advisor/advisor_lib.py +++ b/premerge/advisor/advisor_lib.py @@ -137,6 +137,25 @@ def _try_explain_flaky_failure( test_failure: TestFailure, platform: str, ) -> FailureExplanation | None: + """See if a failure is flaky at head. + + This function looks at a test failure and tries to see if the failure is + a known flake at head. It does this heuristically, by seeing if there have + been at least two failures across more than 200 commits. This has the + advantage of being a simple heuristic and performant. We do not + explicitly handle the case where a test has been failing continiously + for this amount of time as this is an OOM more range than any non-flaky + tests have stayed in tree. + + Args: + db_connection: The database connection. + test_failure: The test failure to try and explain. + platform: The platform the test failed on. + + Returns: + Either None, if the test could not be explained as flaky, or a + FailureExplanation object explaining the test failure. + """ test_name_matches = db_connection.execute( "SELECT failure_message, commit_index FROM failures WHERE source_type='postcommit' AND platform=?", (platform,),