Commit 8fe57796 authored by Christopher Lam's avatar Christopher Lam Committed by Commit Bot

[histograms/validate_format.py] Allow OWNERS files to have no new OWNERs.

This CL fixes an issue where validate_format.py would catch OWNERS files
that were 'empty' because they only had OWNERS that were already in
the list of <owner> emails.

Bug: 1055668
Change-Id: Ie832a566d3c99cfaaa0ad621def8f990190b5bda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2076639
Commit-Queue: calamity <calamity@chromium.org>
Reviewed-by: default avatarIlya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745314}
parent f4de028c
...@@ -229,6 +229,8 @@ def _MakeOwners(document, path, emails_with_dom_elements): ...@@ -229,6 +229,8 @@ def _MakeOwners(document, path, emails_with_dom_elements):
owner_elements = [] owner_elements = []
# TODO(crbug.com/987709): An OWNERS file API would be ideal. # TODO(crbug.com/987709): An OWNERS file API would be ideal.
emails_from_owners_file = _ExtractEmailAddressesFromOWNERS(path) emails_from_owners_file = _ExtractEmailAddressesFromOWNERS(path)
if not emails_from_owners_file:
raise Error('No emails could be derived from {}.'.format(path))
# A list is used to respect the order of email addresses in the OWNERS file. # A list is used to respect the order of email addresses in the OWNERS file.
deduped_emails_from_owners_file = [] deduped_emails_from_owners_file = []
...@@ -332,7 +334,7 @@ def ExpandHistogramsOWNERS(histograms): ...@@ -332,7 +334,7 @@ def ExpandHistogramsOWNERS(histograms):
owners_to_add = _MakeOwners( owners_to_add = _MakeOwners(
owner.ownerDocument, path, emails_with_dom_elements) owner.ownerDocument, path, emails_with_dom_elements)
if not owners_to_add: if not owners_to_add:
raise Error('No emails could be derived from {}.'.format(path)) continue
_UpdateHistogramOwners(histogram, owner, owners_to_add) _UpdateHistogramOwners(histogram, owner, owners_to_add)
......
...@@ -295,7 +295,7 @@ class ExpandOwnersTest(unittest.TestCase): ...@@ -295,7 +295,7 @@ class ExpandOwnersTest(unittest.TestCase):
<histograms> <histograms>
<histogram name="Caffeination" units="mg"> <histogram name="Caffeination" units="mg">
<owner>joe@chormium.org</owner> <owner>joe@chromium.org</owner>
<owner>src/medium/medium/roast/OWNERS</owner> <owner>src/medium/medium/roast/OWNERS</owner>
<summary>I like coffee.</summary> <summary>I like coffee.</summary>
</histogram> </histogram>
...@@ -320,7 +320,7 @@ class ExpandOwnersTest(unittest.TestCase): ...@@ -320,7 +320,7 @@ class ExpandOwnersTest(unittest.TestCase):
<histograms> <histograms>
<histogram name="Caffeination" units="mg"> <histogram name="Caffeination" units="mg">
<owner>joe@chormium.org</owner> <owner>joe@chromium.org</owner>
<owner>{}</owner> <owner>{}</owner>
<summary>I like coffee.</summary> <summary>I like coffee.</summary>
</histogram> </histogram>
...@@ -333,13 +333,42 @@ class ExpandOwnersTest(unittest.TestCase): ...@@ -333,13 +333,42 @@ class ExpandOwnersTest(unittest.TestCase):
r'No emails could be derived from .*empty_OWNERS\.'): r'No emails could be derived from .*empty_OWNERS\.'):
expand_owners.ExpandHistogramsOWNERS(histograms_without_owners_from_file) expand_owners.ExpandHistogramsOWNERS(histograms_without_owners_from_file)
with self.assertRaisesRegexp(
expand_owners.Error,
r'The file at .*src/medium/medium/roast/OWNERS does not exist\.'):
expand_owners.ExpandHistogramsOWNERS(histograms_with_fake_file_path)
def testExpandOwnersWithSameOwners(self):
"""Checks that no error is raised when all owners in a file are already in <owner> elements."""
absolute_path = _MakeOwnersFile('same_OWNERS', self.temp_dir)
src_relative_path = _GetSrcRelativePath(absolute_path)
with open(absolute_path, 'w') as owners_file:
owners_file.write(
'joe@chromium.org') # Write to the file so that it exists.
histograms_string = xml.dom.minidom.parseString("""
<histograms>
<histogram name="Caffeination" units="mg">
<owner>joe@chromium.org</owner>
<owner>{}</owner>
<summary>I like coffee.</summary>
</histogram>
</histograms>
""".format(src_relative_path))
self.assertEqual(
expand_owners.ExpandHistogramsOWNERS(histograms_string), [])
def testExpandOwnersWithoutOWNERSPathPrefix(self): def testExpandOwnersWithoutOWNERSPathPrefix(self):
"""Checks that an error is raised when the path is not well-formatted.""" """Checks that an error is raised when the path is not well-formatted."""
histograms_without_src_prefix = xml.dom.minidom.parseString(""" histograms_without_src_prefix = xml.dom.minidom.parseString("""
<histograms> <histograms>
<histogram name="Caffeination" units="mg"> <histogram name="Caffeination" units="mg">
<owner>joe@chormium.org</owner> <owner>joe@chromium.org</owner>
<owner>latte/OWNERS</owner> <owner>latte/OWNERS</owner>
<summary>I like coffee.</summary> <summary>I like coffee.</summary>
</histogram> </histogram>
...@@ -358,7 +387,7 @@ class ExpandOwnersTest(unittest.TestCase): ...@@ -358,7 +387,7 @@ class ExpandOwnersTest(unittest.TestCase):
<histograms> <histograms>
<histogram name="Caffeination" units="mg"> <histogram name="Caffeination" units="mg">
<owner>joe@chormium.org</owner> <owner>joe@chromium.org</owner>
<owner>src/latte/file</owner> <owner>src/latte/file</owner>
<summary>I like coffee.</summary> <summary>I like coffee.</summary>
</histogram> </histogram>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment