diff --git a/git_footers.py b/git_footers.py index bfcf07c1af..a64acc4003 100755 --- a/git_footers.py +++ b/git_footers.py @@ -130,6 +130,8 @@ def add_footer(message, key, value, after_keys=None, before_keys=None): """ assert key == normalize_name(key), 'Use normalized key' new_footer = '%s: %s' % (key, value) + if not FOOTER_PATTERN.match(new_footer): + raise ValueError('Invalid footer %r' % new_footer) top_lines, footer_lines, _ = split_footers(message) if not footer_lines: diff --git a/presubmit_support.py b/presubmit_support.py index 368891623a..c27eb78865 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -1127,6 +1127,19 @@ class Change(object): self._description_without_tags = ( '\n'.join(description_without_tags).rstrip()) + def AddDescriptionFooter(self, key, value): + """Adds the given footer to the change description. + + Args: + key: A string with the key for the git footer. It must conform to + the git footers format (i.e. 'List-Of-Tokens') and will be case + normalized so that each token is title-cased. + value: A string with the value for the git footer. + """ + description = git_footers.add_footer( + self.FullDescriptionText(), git_footers.normalize_name(key), value) + self.SetDescriptionText(description) + def RepositoryRoot(self): """Returns the repository (checkout) root directory for this change, as an absolute path. @@ -1139,11 +1152,20 @@ class Change(object): raise AttributeError(self, attr) return self.tags.get(attr) + def GitFootersFromDescription(self): + """Return the git footers present in the description. + + Returns: + footers: A dict of {footer: [values]} containing a multimap of the footers + in the change description. + """ + return git_footers.parse_footers(self.FullDescriptionText()) + def BugsFromDescription(self): """Returns all bugs referenced in the commit description.""" tags = [b.strip() for b in self.tags.get('BUG', '').split(',') if b.strip()] footers = [] - parsed = git_footers.parse_footers(self._full_description) + parsed = self.GitFootersFromDescription() unsplit_footers = parsed.get('Bug', []) + parsed.get('Fixed', []) for unsplit_footer in unsplit_footers: footers += [b.strip() for b in unsplit_footer.split(',')] @@ -1160,7 +1182,7 @@ class Change(object): tags = [r.strip() for r in self.tags.get('TBR', '').split(',') if r.strip()] # TODO(agable): Remove support for 'Tbr:' when TBRs are programmatically # determined by self-CR+1s. - footers = git_footers.parse_footers(self._full_description).get('Tbr', []) + footers = self.GitFootersFromDescription().get('Tbr', []) return sorted(set(tags + footers)) # TODO(agable): Delete these once we're sure they're unused. diff --git a/tests/git_footers_test.py b/tests/git_footers_test.py index 9a5edfadba..619b51b27c 100755 --- a/tests/git_footers_test.py +++ b/tests/git_footers_test.py @@ -178,6 +178,9 @@ My commit message is my best friend. It is my life. I must master it. 'Header.\n\nBug: v8\nChange-Id: Ix\nN=t\nT=z') def testAddFooter(self): + with self.assertRaises(ValueError): + git_footers.add_footer('', 'Invalid Footer', 'Value') + self.assertEqual( git_footers.add_footer('', 'Key', 'Value'), '\nKey: Value') diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 18e183d247..ed9ceeeed7 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -1391,6 +1391,41 @@ class ChangeUnittest(PresubmitTestsBase): self.assertEqual('WHIZ=bang\nbar\nFOO=baz', change.FullDescriptionText()) self.assertEqual({'WHIZ': 'bang', 'FOO': 'baz'}, change.tags) + def testAddDescriptionFooter(self): + change = presubmit.Change( + '', 'foo\nDRU=ro\n\nChange-Id: asdf', self.fake_root_dir, [], 3, 5, '') + change.AddDescriptionFooter('my-footer', 'my-value') + self.assertEqual( + 'foo\nDRU=ro\n\nChange-Id: asdf\nMy-Footer: my-value', + change.FullDescriptionText()) + + def testAddDescriptionFooter_NoPreviousFooters(self): + change = presubmit.Change( + '', 'foo\nDRU=ro', self.fake_root_dir, [], 3, 5, '') + change.AddDescriptionFooter('my-footer', 'my-value') + self.assertEqual( + 'foo\nDRU=ro\n\nMy-Footer: my-value', change.FullDescriptionText()) + + def testAddDescriptionFooter_InvalidFooter(self): + change = presubmit.Change( + '', 'foo\nDRU=ro', self.fake_root_dir, [], 3, 5, '') + with self.assertRaises(ValueError): + change.AddDescriptionFooter('invalid.characters in:the', 'footer key') + + def testGitFootersFromDescription(self): + change = presubmit.Change( + '', 'foo\n\nChange-Id: asdf\nBug: 1\nBug: 2\nNo-Try: True', + self.fake_root_dir, [], 0, 0, '') + self.assertEqual({ + 'Change-Id': ['asdf'], + 'Bug': ['2', '1'], + 'No-Try': ['True'], + }, change.GitFootersFromDescription()) + + def testGitFootersFromDescription_NoFooters(self): + change = presubmit.Change('', 'foo', self.fake_root_dir, [], 0, 0, '') + self.assertEqual({}, change.GitFootersFromDescription()) + def testBugFromDescription_FixedAndBugGetDeduped(self): change = presubmit.Change( '', 'foo\n\nChange-Id: asdf\nBug: 1, 2\nFixed:2, 1 ',