Skip to content

Commit eec9a13

Browse files
committed
validate-pr: accept context-only cherry-picks
Stable patch IDs include context lines, so legitimate cherry-picks can differ when nearby lines change. Compare zero-context patch IDs, then replay the upstream commit onto the local parent with merge-tree and require the resulting tree to match the local commit. Replay ordinary patch-ID matches too so identical changes applied at a different occurrence are rejected.
1 parent 2c7c305 commit eec9a13

3 files changed

Lines changed: 473 additions & 18 deletions

File tree

Lines changed: 327 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,327 @@
1+
#!/usr/bin/env python3
2+
"""Black-box regression tests for validate-pr cherry-pick matching."""
3+
4+
import os
5+
import pathlib
6+
import shutil
7+
import subprocess
8+
import sys
9+
import tempfile
10+
import unittest
11+
12+
13+
VALIDATE_PR = pathlib.Path(__file__).with_name("validate-pr")
14+
SUBJECT = "fixture: insert payload"
15+
UPSTREAM_SOB = "Signed-off-by: Upstream Author <upstream@example.com>"
16+
LOCAL_SOB = "Signed-off-by: Local Author <local@example.com>"
17+
18+
19+
class ValidatePrPatchIdTest(unittest.TestCase):
20+
def setUp(self):
21+
self._tmp = tempfile.TemporaryDirectory()
22+
self.addCleanup(self._tmp.cleanup)
23+
self.repo = pathlib.Path(self._tmp.name) / "repo"
24+
self.repo.mkdir()
25+
self.real_git = shutil.which("git")
26+
self.assertIsNotNone(self.real_git)
27+
28+
self._git("init")
29+
self._set_identity("Fixture Author", "fixture@example.com")
30+
self._write_fixture([
31+
"first-before",
32+
"anchor",
33+
"first-after",
34+
"spacer-1",
35+
"spacer-2",
36+
"spacer-3",
37+
"spacer-4",
38+
"second-before",
39+
"anchor",
40+
"second-after",
41+
])
42+
self.base = self._commit("fixture: base", LOCAL_SOB)
43+
44+
def _relocate_repo(self, name):
45+
relocated = self.repo.parent / name
46+
self.repo.rename(relocated)
47+
self.repo = relocated
48+
49+
def _git(self, *args, input_text=None):
50+
result = subprocess.run(
51+
["git", *args],
52+
cwd=self.repo,
53+
input=input_text,
54+
text=True,
55+
capture_output=True,
56+
)
57+
if result.returncode:
58+
self.fail(
59+
"git {} failed ({}):\n{}{}".format(
60+
" ".join(args), result.returncode,
61+
result.stdout, result.stderr))
62+
return result.stdout.strip()
63+
64+
def _set_identity(self, name, email):
65+
self._git("config", "user.name", name)
66+
self._git("config", "user.email", email)
67+
68+
def _write_fixture(self, lines):
69+
(self.repo / "fixture.txt").write_text("\n".join(lines) + "\n")
70+
71+
def _read_fixture(self):
72+
return (self.repo / "fixture.txt").read_text().splitlines()
73+
74+
def _use_identical_occurrence_fixture(self):
75+
block = [
76+
"same-before-4",
77+
"same-before-3",
78+
"same-before-2",
79+
"same-before-1",
80+
"anchor",
81+
"same-after-1",
82+
"same-after-2",
83+
"same-after-3",
84+
"same-after-4",
85+
]
86+
separator = ["separator-{}".format(i) for i in range(1, 8)]
87+
self._write_fixture(["prefix", *block, *separator, *block, "suffix"])
88+
self.base = self._commit("fixture: duplicate context", LOCAL_SOB)
89+
90+
def _commit(self, subject, body):
91+
self._git("add", "fixture.txt")
92+
self._git("commit", "-F", "-",
93+
input_text="{}\n\n{}\n".format(subject, body))
94+
return self._git("rev-parse", "HEAD")
95+
96+
def _patch_id(self, commit):
97+
patch = self._git("show", commit)
98+
output = self._git("patch-id", "--stable", input_text=patch)
99+
return output.split()[0]
100+
101+
def _build_case(self, change, context_parent=True):
102+
self._git("checkout", "-b", "upstream-topic", self.base)
103+
self._set_identity("Upstream Author", "upstream@example.com")
104+
lines = self._read_fixture()
105+
lines.insert(lines.index("anchor") + 1, "payload")
106+
self._write_fixture(lines)
107+
upstream = self._commit(SUBJECT, UPSTREAM_SOB)
108+
self._git("update-ref", "refs/remotes/upstream/linux", upstream)
109+
110+
self._git("checkout", "-b", "local", self.base)
111+
self._set_identity("Local Author", "local@example.com")
112+
if context_parent:
113+
lines = self._read_fixture()
114+
lines[0] = "local-first-before"
115+
self._write_fixture(lines)
116+
self._commit("fixture: adjust local context", LOCAL_SOB)
117+
parent = self._git("rev-parse", "HEAD")
118+
119+
if change == "exact":
120+
self._git("cherry-pick", "-x", "--signoff", upstream)
121+
else:
122+
lines = self._read_fixture()
123+
anchors = [i for i, line in enumerate(lines) if line == "anchor"]
124+
if change == "context":
125+
lines.insert(anchors[0] + 1, "payload")
126+
elif change == "mutated":
127+
lines.insert(anchors[0] + 1, "payload-mutated")
128+
elif change == "wrong-occurrence":
129+
lines.insert(anchors[1] + 1, "payload")
130+
else:
131+
self.fail("unknown fixture change: {}".format(change))
132+
self._write_fixture(lines)
133+
self._commit(
134+
SUBJECT,
135+
"{}\n\n(cherry picked from commit {})\n{}".format(
136+
UPSTREAM_SOB, upstream, LOCAL_SOB))
137+
138+
local = self._git("rev-parse", "HEAD")
139+
return parent, local
140+
141+
def _fake_git_env(self, mode):
142+
fake_bin = self.repo / "fake-bin"
143+
fake_bin.mkdir(exist_ok=True)
144+
wrapper = fake_bin / "git"
145+
wrapper.write_text("""#!/usr/bin/env python3
146+
import os
147+
import subprocess
148+
import sys
149+
150+
real_git = os.environ["VALIDATE_PR_REAL_GIT"]
151+
mode = os.environ["VALIDATE_PR_FAKE_GIT_MODE"]
152+
args = sys.argv[1:]
153+
154+
if mode == "show-failure" and args and args[0] == "show":
155+
result = subprocess.run([real_git, *args], capture_output=True)
156+
sys.stdout.buffer.write(result.stdout)
157+
sys.stderr.buffer.write(result.stderr)
158+
sys.exit(1)
159+
160+
if (args[:2] == ["patch-id", "--stable"] and
161+
mode in ("patch-id-malformed", "patch-id-extra-line")):
162+
sys.stdin.buffer.read()
163+
if mode == "patch-id-malformed":
164+
sys.stdout.write("not-a-patch-id not-a-commit-id\\n")
165+
sys.exit(0)
166+
if mode == "patch-id-extra-line":
167+
sys.stdout.write("{} {}\\n{} {}\\n".format(
168+
"a" * 40, "b" * 40, "c" * 40, "d" * 40))
169+
sys.exit(0)
170+
171+
if (mode == "rev-parse-invalid-utf8" and
172+
args[:3] == ["rev-parse", "--git-path", "objects"]):
173+
sys.stdout.buffer.write(b"\\xff\\n")
174+
sys.exit(0)
175+
176+
if mode == "merge-tree-failure" and args and args[0] == "merge-tree":
177+
sys.stderr.write("synthetic merge-tree failure\\n")
178+
sys.exit(2)
179+
180+
if mode == "merge-tree-extra-line" and args and args[0] == "merge-tree":
181+
result = subprocess.run([real_git, *args], capture_output=True)
182+
sys.stdout.buffer.write(result.stdout)
183+
if result.stdout and not result.stdout.endswith(b"\\n"):
184+
sys.stdout.buffer.write(b"\\n")
185+
sys.stdout.buffer.write(b"unexpected-extra-line\\n")
186+
sys.stderr.buffer.write(result.stderr)
187+
sys.exit(result.returncode)
188+
189+
os.execv(real_git, [real_git, *args])
190+
""")
191+
wrapper.chmod(0o755)
192+
env = os.environ.copy()
193+
env["PATH"] = str(fake_bin) + os.pathsep + env.get("PATH", "")
194+
env["VALIDATE_PR_REAL_GIT"] = self.real_git
195+
env["VALIDATE_PR_FAKE_GIT_MODE"] = mode
196+
return env
197+
198+
def _validate(self, parent, local, git_mode=None):
199+
return subprocess.run(
200+
[sys.executable, str(VALIDATE_PR),
201+
"{}..{}".format(parent, local), "upstream", "linux",
202+
"--no-update"],
203+
cwd=self.repo,
204+
env=self._fake_git_env(git_mode) if git_mode else None,
205+
text=True,
206+
capture_output=True,
207+
)
208+
209+
@staticmethod
210+
def _output(result):
211+
return "stdout:\n{}\nstderr:\n{}".format(
212+
result.stdout, result.stderr)
213+
214+
def _patch_id_status(self, result, local):
215+
marker = "│ {} │".format(local[:12])
216+
for line in result.stdout.splitlines():
217+
if marker in line:
218+
cells = line.split("│")
219+
self.assertGreaterEqual(len(cells), 6, self._output(result))
220+
return cells[3].strip()
221+
self.fail("no digest row for {}:\n{}".format(
222+
local[:12], self._output(result)))
223+
224+
def test_accepts_context_only_replay(self):
225+
parent, local = self._build_case("context")
226+
227+
result = self._validate(parent, local)
228+
229+
self.assertEqual(result.returncode, 0, self._output(result))
230+
self.assertEqual(self._patch_id_status(result, local), "context")
231+
232+
def test_accepts_context_only_replay_with_colon_in_object_path(self):
233+
self._relocate_repo("repo:colon")
234+
parent, local = self._build_case("context")
235+
236+
result = self._validate(parent, local)
237+
238+
self.assertEqual(result.returncode, 0, self._output(result))
239+
self.assertEqual(self._patch_id_status(result, local), "context")
240+
241+
def test_accepts_exact_cherry_pick(self):
242+
parent, local = self._build_case("exact", context_parent=False)
243+
244+
result = self._validate(parent, local)
245+
246+
self.assertEqual(result.returncode, 0, self._output(result))
247+
self.assertEqual(self._patch_id_status(result, local), "match")
248+
249+
def test_rejects_git_show_failure_with_patch_output(self):
250+
parent, local = self._build_case("exact", context_parent=False)
251+
252+
result = self._validate(parent, local, "show-failure")
253+
254+
self.assertEqual(result.returncode, 1, self._output(result))
255+
self.assertIn("patch-ID mismatch with upstream", result.stdout)
256+
257+
def test_rejects_malformed_patch_id_output(self):
258+
parent, local = self._build_case("exact", context_parent=False)
259+
260+
result = self._validate(parent, local, "patch-id-malformed")
261+
262+
self.assertEqual(result.returncode, 1, self._output(result))
263+
self.assertIn("patch-ID mismatch with upstream", result.stdout)
264+
265+
def test_rejects_merge_tree_output_with_extra_line(self):
266+
parent, local = self._build_case("context")
267+
268+
result = self._validate(parent, local, "merge-tree-extra-line")
269+
270+
self.assertEqual(result.returncode, 1, self._output(result))
271+
self.assertIn("patch-ID mismatch with upstream", result.stdout)
272+
273+
def test_reports_replay_environment_failure(self):
274+
parent, local = self._build_case("exact", context_parent=False)
275+
276+
result = self._validate(parent, local, "rev-parse-invalid-utf8")
277+
278+
self.assertEqual(result.returncode, 1, self._output(result))
279+
self.assertIn("unable to verify patch replay", result.stderr)
280+
281+
def test_reports_merge_tree_failure(self):
282+
parent, local = self._build_case("exact", context_parent=False)
283+
284+
result = self._validate(parent, local, "merge-tree-failure")
285+
286+
self.assertEqual(result.returncode, 1, self._output(result))
287+
self.assertIn("synthetic merge-tree failure", result.stderr)
288+
289+
def test_rejects_multiple_patch_id_output_lines(self):
290+
parent, local = self._build_case("exact", context_parent=False)
291+
292+
result = self._validate(parent, local, "patch-id-extra-line")
293+
294+
self.assertEqual(result.returncode, 1, self._output(result))
295+
self.assertIn("patch-ID mismatch with upstream", result.stdout)
296+
297+
def test_rejects_mutated_payload(self):
298+
parent, local = self._build_case("mutated")
299+
300+
result = self._validate(parent, local)
301+
302+
self.assertEqual(result.returncode, 1, self._output(result))
303+
self.assertIn("patch-ID mismatch with upstream", result.stdout)
304+
305+
def test_rejects_same_change_at_different_occurrence(self):
306+
parent, local = self._build_case("wrong-occurrence")
307+
308+
result = self._validate(parent, local)
309+
310+
self.assertEqual(result.returncode, 1, self._output(result))
311+
self.assertIn("patch-ID mismatch with upstream", result.stdout)
312+
313+
def test_rejects_equal_patch_id_at_different_occurrence(self):
314+
self._use_identical_occurrence_fixture()
315+
parent, local = self._build_case(
316+
"wrong-occurrence", context_parent=False)
317+
upstream = self._git("rev-parse", "refs/remotes/upstream/linux")
318+
319+
self.assertEqual(self._patch_id(local), self._patch_id(upstream))
320+
result = self._validate(parent, local)
321+
322+
self.assertEqual(result.returncode, 1, self._output(result))
323+
self.assertIn("patch-ID mismatch with upstream", result.stdout)
324+
325+
326+
if __name__ == "__main__":
327+
unittest.main()

0 commit comments

Comments
 (0)