diff --git a/.github/workflows/request_test_data.yml b/.github/workflows/request_test_data.yml index 318f681..f8f62ea 100644 --- a/.github/workflows/request_test_data.yml +++ b/.github/workflows/request_test_data.yml @@ -7,7 +7,8 @@ name: Request Test Data on: pull_request_target: - types: [opened, synchronize, reopened] + # labeled: a maintainer applying the bot-test label fires the flow at once + types: [opened, synchronize, reopened, labeled] paths: - 'scripts/artifacts/**' diff --git a/admin/scripts/check_pr_test_data.py b/admin/scripts/check_pr_test_data.py index f0e242e..f35e3f5 100644 --- a/admin/scripts/check_pr_test_data.py +++ b/admin/scripts/check_pr_test_data.py @@ -33,6 +33,7 @@ REPO_ROOT = Path(__file__).resolve().parents[2] MANIFEST_PATH = REPO_ROOT / "admin" / "image_manifest.json" MARKER = "" +TEST_LABEL = "bot-test" LABEL_ASK = "needs-test-data" LABEL_FIXTURE = "fixture-needed" LABELS = { @@ -220,6 +221,22 @@ def desired_labels(fixture, ask): return labels +def should_skip(login, permission, label_names): + """Returns (skip, reason) for authors the bot would not normally address. + + A maintainer-applied bot-test label overrides the skip so the whole flow + can be exercised end to end on any PR. Applying labels needs triage + access, so contributors cannot trigger that on themselves. + """ + if TEST_LABEL in label_names: + return False, f"'{TEST_LABEL}' label present; treating the author as external" + if login.endswith("[bot]"): + return True, f"author {login} is a bot" + if permission in ("admin", "write"): + return True, f"author {login} has {permission} access" + return False, f"author {login} has {permission} access" + + def find_marker_comment(token, repo, pr_number): for comment in paginate(token, f"{API}/repos/{repo}/issues/{pr_number}/comments"): if MARKER in comment.get("body", ""): @@ -270,12 +287,12 @@ def main(): pr = api_request(token, f"{API}/repos/{repo}/pulls/{pr_number}") login = pr["user"]["login"] - if login.endswith("[bot]"): - print(f"Author {login} is a bot; skipping.") - return + label_names = {l["name"] for l in pr.get("labels", [])} permission = author_permission(token, repo, login) - if permission in ("admin", "write"): - print(f"Author {login} has {permission} access; skipping.") + skip, reason = should_skip(login, permission, label_names) + print(reason) + if skip: + print("Skipping.") if not dry_run: return print("DRY_RUN: continuing anyway to show the decision.") diff --git a/admin/test/scripts/test_check_pr_test_data.py b/admin/test/scripts/test_check_pr_test_data.py index 2d6abdd..31d47c9 100644 --- a/admin/test/scripts/test_check_pr_test_data.py +++ b/admin/test/scripts/test_check_pr_test_data.py @@ -131,6 +131,23 @@ def test_no_em_dashes_anywhere(self): self.assertNotIn('—', body) +class ShouldSkipTests(unittest.TestCase): + def test_write_and_admin_authors_skip(self): + self.assertTrue(bot.should_skip('dev', 'write', set())[0]) + self.assertTrue(bot.should_skip('dev', 'admin', set())[0]) + + def test_bot_authors_skip(self): + self.assertTrue(bot.should_skip('dependabot[bot]', 'none', set())[0]) + + def test_external_authors_do_not_skip(self): + self.assertFalse(bot.should_skip('someone', 'read', set())[0]) + self.assertFalse(bot.should_skip('someone', 'none', set())[0]) + + def test_bot_test_label_overrides_every_skip(self): + self.assertFalse(bot.should_skip('dev', 'admin', {bot.TEST_LABEL})[0]) + self.assertFalse(bot.should_skip('dependabot[bot]', 'none', {bot.TEST_LABEL})[0]) + + class DesiredLabelsTests(unittest.TestCase): def test_matrix(self): self.assertEqual(bot.desired_labels({}, []), set())