Skip to content

Commit 65e16ad

Browse files
Add tests for merge rules validation engine
Adds comprehensive test coverage for the merge rules feature: - test/merge_rules/yaml_parsing.py.test: YAML parsing tests - test/merge_rules/pattern_matching.py.test: fnmatch pattern tests - test/merge_rules/approval_validation.py.test: approver validation tests - test/merge_rules/check_validation.py.test: CI check validation tests - test/merge_rules/ignore_flaky.py.test: ignore_flaky_failures flag tests Adds land command integration tests: - test/land/validate_rules_fail.py.test: validation blocks land - test/land/validate_rules_pass.py.test: validation passes and lands - test/land/dry_run.py.test: dry run mode validates but doesn't land Also adds helper functions to test_prelude.py: - set_pr_files, set_pr_reviews, set_pr_check_runs - Updated gh_land to support validate_rules, dry_run, comment_on_failure flags Co-authored-by: Cursor <cursoragent@cursor.com> ghstack-source-id: 4a5247e ghstack-comment-id: 3838814339 Pull-Request: #321
1 parent 952058f commit 65e16ad

File tree

9 files changed

+578
-1
lines changed

9 files changed

+578
-1
lines changed

src/ghstack/test_prelude.py

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@
5353
"get_github",
5454
"get_pr_reviewers",
5555
"get_pr_labels",
56+
"set_pr_files",
57+
"set_pr_reviews",
58+
"set_pr_check_runs",
5659
"tick",
5760
"captured_output",
5861
]
@@ -225,14 +228,25 @@ def gh_submit(
225228
return r
226229

227230

228-
def gh_land(pull_request: str) -> None:
231+
def gh_land(
232+
pull_request: str,
233+
*,
234+
validate_rules: bool = False,
235+
dry_run: bool = False,
236+
comment_on_failure: bool = False,
237+
rules_file: Optional[str] = None,
238+
) -> None:
229239
self = CTX
230240
return ghstack.land.main(
231241
remote_name="origin",
232242
pull_request=pull_request,
233243
github=self.github,
234244
sh=self.sh,
235245
github_url="github.com",
246+
validate_rules=validate_rules,
247+
dry_run=dry_run,
248+
comment_on_failure=comment_on_failure,
249+
rules_file=rules_file,
236250
)
237251

238252

@@ -412,6 +426,36 @@ def get_pr_labels(pr_number: int) -> List[str]:
412426
return pr.labels
413427

414428

429+
def set_pr_files(pr_number: int, files: List[str]) -> None:
430+
"""Set the list of changed files for a PR."""
431+
github = get_github()
432+
repo = github.state.repository("pytorch", "pytorch")
433+
pr = github.state.pull_request(repo, ghstack.github_fake.GitHubNumber(pr_number))
434+
pr.files = files
435+
436+
437+
def set_pr_reviews(pr_number: int, reviews: List[Tuple[str, str]]) -> None:
438+
"""Set reviews for a PR. reviews is list of (user, state) tuples."""
439+
github = get_github()
440+
repo = github.state.repository("pytorch", "pytorch")
441+
pr = github.state.pull_request(repo, ghstack.github_fake.GitHubNumber(pr_number))
442+
pr.reviews = [
443+
ghstack.github_fake.PullRequestReview(user=u, state=s) for u, s in reviews
444+
]
445+
446+
447+
def set_pr_check_runs(
448+
pr_number: int, checks: List[Tuple[str, str, Optional[str]]]
449+
) -> None:
450+
"""Set check runs for a PR. checks is list of (name, status, conclusion) tuples."""
451+
github = get_github()
452+
repo = github.state.repository("pytorch", "pytorch")
453+
pr = github.state.pull_request(repo, ghstack.github_fake.GitHubNumber(pr_number))
454+
pr.check_runs = [
455+
ghstack.github_fake.CheckRun(name=n, status=s, conclusion=c) for n, s, c in checks
456+
]
457+
458+
415459
def assert_eq(a: Any, b: Any) -> None:
416460
assert a == b, f"{a} != {b}"
417461

test/land/dry_run.py.test

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
from ghstack.test_prelude import *
2+
3+
import os
4+
import tempfile
5+
import ghstack.merge_rules
6+
from ghstack.github_fake import GitHubNumber, PullRequestReview, CheckRun
7+
8+
init_test()
9+
commit("A")
10+
(diff,) = gh_submit("Initial 1")
11+
pr_url = diff.pr_url
12+
13+
# Set up PR with files, approvals, and passing checks
14+
github = get_github()
15+
repo = github.state.repository("pytorch", "pytorch")
16+
pr = github.state.pull_request(repo, GitHubNumber(500))
17+
pr.files = ["src/core/module.py"]
18+
pr.reviews = [PullRequestReview(user="maintainer1", state="APPROVED")]
19+
pr.check_runs = [CheckRun(name="CI", status="completed", conclusion="success")]
20+
21+
# Create a temporary rules file
22+
rules_content = """
23+
- name: core-changes
24+
patterns: ["src/core/*"]
25+
approved_by: ["maintainer1"]
26+
mandatory_checks_name: ["CI"]
27+
"""
28+
29+
with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f:
30+
f.write(rules_content)
31+
rules_file = f.name
32+
33+
try:
34+
# Get the initial state of master
35+
initial_log = get_upstream_sh().git("log", "--oneline", "master")
36+
37+
# Dry run should NOT land the commit
38+
gh_land(pr_url, validate_rules=True, dry_run=True, rules_file=rules_file)
39+
40+
# Verify master is unchanged (no commit was landed)
41+
final_log = get_upstream_sh().git("log", "--oneline", "master")
42+
assert_eq(initial_log, final_log)
43+
44+
# The PR should still be open
45+
assert_eq(pr.closed, False)
46+
finally:
47+
os.unlink(rules_file)
48+
49+
ok()
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
from ghstack.test_prelude import *
2+
3+
import os
4+
import tempfile
5+
import ghstack.merge_rules
6+
from ghstack.github_fake import GitHubNumber
7+
8+
init_test()
9+
commit("A")
10+
(diff,) = gh_submit("Initial 1")
11+
pr_url = diff.pr_url
12+
13+
# Set up PR with files but no approvals
14+
github = get_github()
15+
repo = github.state.repository("pytorch", "pytorch")
16+
pr = github.state.pull_request(repo, GitHubNumber(500))
17+
pr.files = ["src/core/module.py"]
18+
pr.reviews = [] # No reviews
19+
20+
# Create a temporary rules file
21+
rules_content = """
22+
- name: core-changes
23+
patterns: ["src/core/*"]
24+
approved_by: ["maintainer1"]
25+
mandatory_checks_name: []
26+
"""
27+
28+
with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f:
29+
f.write(rules_content)
30+
rules_file = f.name
31+
32+
try:
33+
# Attempt to land with validation - should raise MergeValidationError
34+
assert_expected_raises_inline(
35+
ghstack.merge_rules.MergeValidationError,
36+
lambda: gh_land(pr_url, validate_rules=True, rules_file=rules_file),
37+
"""\
38+
Merge validation failed for PR #500
39+
Rule: core-changes
40+
Errors:
41+
- Missing required approval from: maintainer1""",
42+
)
43+
finally:
44+
os.unlink(rules_file)
45+
46+
ok()
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
from ghstack.test_prelude import *
2+
3+
import os
4+
import tempfile
5+
import ghstack.merge_rules
6+
from ghstack.github_fake import GitHubNumber, PullRequestReview, CheckRun
7+
8+
init_test()
9+
commit("A")
10+
(diff,) = gh_submit("Initial 1")
11+
pr_url = diff.pr_url
12+
13+
# Set up PR with files, approvals, and passing checks
14+
github = get_github()
15+
repo = github.state.repository("pytorch", "pytorch")
16+
pr = github.state.pull_request(repo, GitHubNumber(500))
17+
pr.files = ["src/core/module.py"]
18+
pr.reviews = [PullRequestReview(user="maintainer1", state="APPROVED")]
19+
pr.check_runs = [CheckRun(name="CI", status="completed", conclusion="success")]
20+
21+
# Create a temporary rules file
22+
rules_content = """
23+
- name: core-changes
24+
patterns: ["src/core/*"]
25+
approved_by: ["maintainer1"]
26+
mandatory_checks_name: ["CI"]
27+
"""
28+
29+
with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f:
30+
f.write(rules_content)
31+
rules_file = f.name
32+
33+
try:
34+
# Land with validation - should succeed
35+
gh_land(pr_url, validate_rules=True, rules_file=rules_file)
36+
37+
# Verify the commit was landed
38+
assert_expected_inline(
39+
get_upstream_sh().git("log", "--oneline", "master"),
40+
"""\
41+
d518c9f Commit A (#500)
42+
dc8bfe4 Initial commit""",
43+
)
44+
finally:
45+
os.unlink(rules_file)
46+
47+
ok()
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
from ghstack.test_prelude import *
2+
3+
import ghstack.merge_rules
4+
from ghstack.github_fake import PullRequestReview, GitHubNumber
5+
6+
init_test()
7+
commit("A")
8+
(A,) = gh_submit("Initial 1")
9+
10+
# Set up PR with files and NO approval
11+
github = get_github()
12+
repo = github.state.repository("pytorch", "pytorch")
13+
pr = github.state.pull_request(repo, GitHubNumber(500))
14+
pr.files = ["src/core/module.py"]
15+
pr.reviews = [] # No reviews yet
16+
17+
rules = [
18+
ghstack.merge_rules.MergeRule(
19+
name="core",
20+
patterns=["src/core/*"],
21+
approved_by=["maintainer1"],
22+
mandatory_checks_name=[],
23+
)
24+
]
25+
26+
validator = ghstack.merge_rules.MergeValidator(github, "pytorch", "pytorch")
27+
result = validator.validate_pr(500, rules)
28+
29+
# Should fail - missing approval
30+
assert_eq(result.valid, False)
31+
assert "Missing required approval" in result.errors[0]
32+
33+
# Add approval from wrong user - should still fail
34+
pr.reviews = [PullRequestReview(user="random_user", state="APPROVED")]
35+
result = validator.validate_pr(500, rules)
36+
assert_eq(result.valid, False)
37+
38+
# Add approval from required user - should pass
39+
pr.reviews = [PullRequestReview(user="maintainer1", state="APPROVED")]
40+
result = validator.validate_pr(500, rules)
41+
assert_eq(result.valid, True)
42+
43+
# Test multiple required approvers - any one should work
44+
rules = [
45+
ghstack.merge_rules.MergeRule(
46+
name="core",
47+
patterns=["src/core/*"],
48+
approved_by=["maintainer1", "maintainer2"],
49+
mandatory_checks_name=[],
50+
)
51+
]
52+
53+
pr.reviews = [PullRequestReview(user="maintainer2", state="APPROVED")]
54+
result = validator.validate_pr(500, rules)
55+
assert_eq(result.valid, True)
56+
57+
# Test that CHANGES_REQUESTED doesn't count as approval
58+
pr.reviews = [PullRequestReview(user="maintainer1", state="CHANGES_REQUESTED")]
59+
result = validator.validate_pr(500, rules)
60+
assert_eq(result.valid, False)
61+
62+
# Test that latest review state wins
63+
pr.reviews = [
64+
PullRequestReview(user="maintainer1", state="APPROVED"),
65+
PullRequestReview(user="maintainer1", state="CHANGES_REQUESTED"),
66+
]
67+
result = validator.validate_pr(500, rules)
68+
assert_eq(result.valid, False)
69+
70+
# Re-approval after changes_requested should work
71+
pr.reviews = [
72+
PullRequestReview(user="maintainer1", state="CHANGES_REQUESTED"),
73+
PullRequestReview(user="maintainer1", state="APPROVED"),
74+
]
75+
result = validator.validate_pr(500, rules)
76+
assert_eq(result.valid, True)
77+
78+
# Test "any" special approver - any approval should work
79+
rules = [
80+
ghstack.merge_rules.MergeRule(
81+
name="any-approval",
82+
patterns=["src/*"],
83+
approved_by=["any"],
84+
mandatory_checks_name=[],
85+
)
86+
]
87+
88+
pr.files = ["src/module.py"]
89+
pr.reviews = [PullRequestReview(user="anyone", state="APPROVED")]
90+
result = validator.validate_pr(500, rules)
91+
assert_eq(result.valid, True)
92+
93+
ok()

0 commit comments

Comments
 (0)