Skip to content

Commit 113eb21

Browse files
committed
Validate a branch that we parse when running cherry_picker --continue
When running cherry_picker --continue we count on being able to get certain information from the branch's name which cherry_picker constructed earlier. Verify as best we can that the branch name is one which cherry_picker could have constructed. Relies on the changes here: #265 (which does the work of one of the validations)
1 parent ff8a587 commit 113eb21

File tree

2 files changed

+50
-11
lines changed

2 files changed

+50
-11
lines changed

cherry_picker/cherry_picker/cherry_picker.py

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,7 @@ def upstream(self):
7474

7575
@property
7676
def sorted_branches(self):
77-
def version_from_branch(branch):
78-
try:
79-
return tuple(map(int, re.match(r'^.*(?P<version>\d+(\.\d+)+).*$', branch).groupdict()['version'].split('.')))
80-
except AttributeError as attr_err:
81-
raise ValueError(f'Branch {branch} seems to not have a version in its name.') from attr_err
82-
77+
"""Return the branches to cherry-pick to, sorted by version"""
8378
return sorted(
8479
self.branches,
8580
reverse=True,
@@ -423,9 +418,36 @@ def get_base_branch(cherry_pick_branch):
423418
return '2.7' from 'backport-sha-2.7'
424419
"""
425420
prefix, sha, base_branch = cherry_pick_branch.split('-', 2)
421+
422+
if prefix != 'backport':
423+
raise ValueError('branch name is not prefixed with "backport-". Is this a cherry_picker branch?')
424+
425+
if not re.match('[0-9a-f]{7,40}', sha):
426+
raise ValueError(f'branch name has an invalid sha: {sha}')
427+
428+
cmd = ['git', 'log', '-r', sha]
429+
try:
430+
subprocess.check_output(cmd, stderr=subprocess.STDOUT)
431+
except subprocess.SubprocessError:
432+
raise ValueError(f'The sha listed in the branch name, {sha}, is not present in the repository')
433+
434+
# Subject the parsed base_branch to the same tests as when we generated it
435+
# This throws a ValueError if the base_branch doesn't need our requirements
436+
version_from_branch(base_branch)
437+
426438
return base_branch
427439

428440

441+
def version_from_branch(branch):
442+
"""
443+
return version information from a git branch name
444+
"""
445+
try:
446+
return tuple(map(int, re.match(r'^.*(?P<version>\d+(\.\d+)+).*$', branch).groupdict()['version'].split('.')))
447+
except AttributeError as attr_err:
448+
raise ValueError(f'Branch {branch} seems to not have a version in its name.') from attr_err
449+
450+
429451
def get_current_branch():
430452
"""
431453
Return the current branch

cherry_picker/cherry_picker/test.py

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,36 @@ def changedir(d):
3232
os.chdir(cwd)
3333

3434

35-
def test_get_base_branch():
36-
# The format of cherry-pick branches we create are "backport-{SHA}-{base_branch}"
37-
cherry_pick_branch = 'backport-afc23f4-2.7'
35+
@mock.patch('subprocess.check_output')
36+
def test_get_base_branch(subprocess_check_output):
37+
# The format of cherry-pick branches we create are::
38+
# backport-{SHA}-{base_branch}
39+
subprocess_check_output.return_value = b'22a594a0047d7706537ff2ac676cdc0f1dcb329c'
40+
cherry_pick_branch = 'backport-22a594a-2.7'
3841
result = get_base_branch(cherry_pick_branch)
3942
assert result == '2.7'
4043

4144

42-
def test_get_base_branch_which_has_dashes():
43-
cherry_pick_branch ='backport-afc23f4-baseprefix-2.7-basesuffix'
45+
@mock.patch('subprocess.check_output')
46+
def test_get_base_branch_which_has_dashes(subprocess_check_output):
47+
subprocess_check_output.return_value = b'22a594a0047d7706537ff2ac676cdc0f1dcb329c'
48+
cherry_pick_branch = 'backport-22a594a-baseprefix-2.7-basesuffix'
4449
result = get_base_branch(cherry_pick_branch)
4550
assert result == 'baseprefix-2.7-basesuffix'
4651

4752

53+
@pytest.mark.parametrize('cherry_pick_branch', ['backport-22a594a', # Not enough fields
54+
'prefix-22a594a-2.7', # Not the prefix we were expecting
55+
'backport-22a594a-base', # No version info in the base branch
56+
]
57+
)
58+
@mock.patch('subprocess.check_output')
59+
def test_get_base_branch_invalid(subprocess_check_output, cherry_pick_branch):
60+
subprocess_check_output.return_value = b'22a594a0047d7706537ff2ac676cdc0f1dcb329c'
61+
with pytest.raises(ValueError):
62+
get_base_branch(cherry_pick_branch)
63+
64+
4865
@mock.patch('subprocess.check_output')
4966
def test_get_current_branch(subprocess_check_output):
5067
subprocess_check_output.return_value = b'master'

0 commit comments

Comments
 (0)