Skip to content

Commit

Permalink
Fix PR review reminder DAG for case where base branch is 2+ levels de…
Browse files Browse the repository at this point in the history
…ep (#3182)
  • Loading branch information
AetherUnbound authored Oct 12, 2023
1 parent 7239399 commit b27bf14
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ def get_min_required_approvals(gh: GitHubAPI, pr: dict) -> int:
else:
raise e

if "required_pull_request_reviews" not in branch_protection_rules:
# This can happen in the rare case where a PR is multiple branches deep,
# e.g. it depends on a branch which depends on a branch which depends on main.
# In that case, default to the rules for `main` as a safe default.
branch_protection_rules = get_branch_protection(gh, repo, "main")

return branch_protection_rules["required_pull_request_reviews"][
"required_approving_review_count"
]
Expand Down
34 changes: 34 additions & 0 deletions catalog/tests/dags/maintenance/test_pr_review_reminders.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,3 +548,37 @@ def test_ignores_created_at_and_pings_if_urgent_ready_for_review_event_exists(
post_reminders("not_set", dry_run=False)

assert pull["number"] in github["posted_comments"]


def test_falls_back_to_main_on_multiple_branch_levels(
github,
):
# No need to parametrize this, only need to test the one case
# Make branch protection rules for a branch one-off main
github["branch_protection"]["openverse"]["non_main"] = {}
past_due_pull = make_pull(Urgency.LOW, old=True, base_branch="not_main")
past_due_pull["requested_reviewers"] = [
make_requested_reviewer(f"reviewer-due-{i}") for i in range(2)
]

min_required_approvals = 4

# Always use `main` to exercise fallback for non-main branches
_setup_branch_protection_for_branch(
github,
repo="openverse",
branch="main",
min_required_approvals=min_required_approvals,
)

github["pulls"] += [past_due_pull]
github["pull_reviews"][past_due_pull["number"]] = [
make_review("APPROVED"),
] * min_required_approvals
github["events"][past_due_pull["number"]] = make_urgent_events(
Urgency.LOW, ["review_requested"]
)

post_reminders("not_set", dry_run=False)

assert past_due_pull["number"] not in github["posted_comments"]

0 comments on commit b27bf14

Please sign in to comment.