Skip to content

Commit 39a32b2

Browse files
authored
[viable/strict] Match required checks exactly (re.fullmatch) (#8438)
Stack from [ghstack](https://github.com/ezyang/ghstack/tree/0.15.0) (oldest at bottom): * #8439 * __->__ #8438 A bare required check like "pull" no longer prefix-matches unrelated workflows (e.g. "pull-test-sandbox"); a successful such workflow no longer satisfies the check either. Explicit regex patterns still work, and empty entries are skipped so a stray one can't stall promotion. I think this makes sense to avoid suddenly pulling in an experimental workflow like trunk-experiment or trunk-test, and suddenly it's blocking viable/strict often. Test: python -m unittest tools.tests.test_fetch_latest_green_commit
1 parent f9a9b06 commit 39a32b2

2 files changed

Lines changed: 73 additions & 5 deletions

File tree

tools/scripts/fetch_latest_green_commit.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@ def is_green(
114114
) -> Tuple[bool, str]:
115115
workflow_checks = get_commit_results(commit, results)
116116

117-
regex = {check: False for check in requires}
117+
# Skip empty entries; fullmatch("") never matches a real workflow name.
118+
regex = {check: False for check in requires if check}
118119

119120
for check in workflow_checks:
120121
jobName = check["name"]
@@ -125,7 +126,9 @@ def is_green(
125126
workflow_name = check["workflowName"]
126127
conclusion = check["conclusion"]
127128
for required_check in regex:
128-
if re.match(required_check, workflow_name, flags=re.IGNORECASE):
129+
# Full match: "pull" won't also match "pull-test-sandbox".
130+
# Explicit regex still works (e.g. "^Apple$").
131+
if re.fullmatch(required_check, workflow_name, flags=re.IGNORECASE):
129132
if conclusion not in ["success", "skipped"]:
130133
return (
131134
False,

tools/tests/test_fetch_latest_green_commit.py

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,13 @@
1818
"Close stale pull requests",
1919
"Update S3 HTML indices for download.pytorch.org",
2020
"Create Release",
21+
"Apple",
22+
"pull-test-sandbox",
2123
]
2224

23-
requires = ["pull", "trunk", "lint", "linux-binary"]
25+
# Matched with re.fullmatch: exact name ("lint" matches "Lint" via IGNORECASE)
26+
# or explicit regex ("^Apple$"). "pull" must NOT match "pull-test-sandbox".
27+
requires = ["pull", "trunk", "lint", "linux-binary-libtorch-pre-cxx11", "^Apple$"]
2428

2529

2630
def set_workflow_job_status(
@@ -117,7 +121,7 @@ def test_skippable_skipped(
117121
workflow_checks = set_workflow_job_status(
118122
workflow_checks, "docker-release-builds", "skipped"
119123
)
120-
self.assertTrue(is_green("sha", requires, workflow_checks))
124+
self.assertTrue(is_green("sha", requires, workflow_checks)[0])
121125

122126
@mock.patch(
123127
"tools.scripts.fetch_latest_green_commit.get_commit_results",
@@ -163,8 +167,69 @@ def test_no_workflows(
163167
self.assertFalse(result[0])
164168
self.assertEqual(
165169
result[1],
166-
"missing required workflows: pull, trunk, lint, linux-binary",
170+
"missing required workflows: pull, trunk, lint, "
171+
"linux-binary-libtorch-pre-cxx11, ^Apple$",
172+
)
173+
174+
@mock.patch(
175+
"tools.scripts.fetch_latest_green_commit.get_commit_results",
176+
return_value=TestChecks().make_test_checks(),
177+
)
178+
def test_prefix_does_not_match(
179+
self, mock_get_commit_results: Any, mock_fetch_unstable_issues: Any
180+
) -> None:
181+
"""A required "pull" must not match "pull-test-sandbox"."""
182+
workflow_checks = mock_get_commit_results()
183+
workflow_checks = set_workflow_job_status(
184+
workflow_checks, "pull-test-sandbox", "failed"
167185
)
186+
result = is_green("sha", requires, workflow_checks)
187+
self.assertTrue(result[0])
188+
189+
@mock.patch(
190+
"tools.scripts.fetch_latest_green_commit.get_commit_results",
191+
return_value=TestChecks().make_test_checks(),
192+
)
193+
def test_regex_required_check(
194+
self, mock_get_commit_results: Any, mock_fetch_unstable_issues: Any
195+
) -> None:
196+
"""An explicit regex required check (ex: "^Apple$") still matches."""
197+
workflow_checks = mock_get_commit_results()
198+
workflow_checks = set_workflow_job_status(workflow_checks, "Apple", "failed")
199+
result = is_green("sha", requires, workflow_checks)
200+
self.assertFalse(result[0])
201+
self.assertEqual(result[1], "Apple was not successful, test/job failed")
202+
203+
@mock.patch(
204+
"tools.scripts.fetch_latest_green_commit.get_commit_results",
205+
return_value=[
206+
WorkflowCheck(
207+
workflowName="pull-test-sandbox",
208+
name="test/job",
209+
jobName="job",
210+
conclusion="success",
211+
)._asdict()
212+
],
213+
)
214+
def test_prefix_success_does_not_satisfy(
215+
self, mock_get_commit_results: Any, mock_fetch_unstable_issues: Any
216+
) -> None:
217+
"""A prefix-named workflow does not satisfy an exact required check."""
218+
result = is_green("sha", ["pull"], mock_get_commit_results())
219+
self.assertFalse(result[0])
220+
self.assertEqual(result[1], "missing required workflows: pull")
221+
222+
@mock.patch(
223+
"tools.scripts.fetch_latest_green_commit.get_commit_results",
224+
return_value=TestChecks().make_test_checks(),
225+
)
226+
def test_empty_required_check_ignored(
227+
self, mock_get_commit_results: Any, mock_fetch_unstable_issues: Any
228+
) -> None:
229+
"""An empty required-check entry is skipped, not left unsatisfiable."""
230+
workflow_checks = mock_get_commit_results()
231+
result = is_green("sha", requires + [""], workflow_checks)
232+
self.assertTrue(result[0])
168233

169234

170235
if __name__ == "__main__":

0 commit comments

Comments
 (0)