Skip to content

GitLab

T
tangled
Commit 0ae19723 authored by Fabrice de Gans-Riberi's avatar Fabrice de Gans-Riberi Committed by Commit Bot
Browse files
Options

Revert "Make expand_owners use dirmd/metadata if available" 

This reverts commit 7fc5d2f9.

Reason for revert: Broke win-archive-rel
https://ci.chromium.org/p/chromium/builders/ci/win-archive-rel/17696

Original change's description:
> Make expand_owners use dirmd/metadata if available
> 
> Fix tests that were pre-broken
> 
> Bug: 1102997
> Change-Id: Ice303cf2ab3f382ad6e0345b355aaa3e214d209a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2388950
> Commit-Queue: Anthony Polito <apolito@google.com>
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#808432}

TBR=asvitkine@chromium.org,caitlinfischer@google.com,apolito@google.com

Change-Id: Ie2f203e72377545c179d0713fd604f5ffb67af7e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1102997
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2419137Reviewed-by: default avatarFabrice de Gans-Riberi <fdegans@chromium.org>
Commit-Queue: Fabrice de Gans-Riberi <fdegans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808440}
parent edc693de
Hide whitespace changes
Inline Side-by-side
Showing with 11 additions and 119 deletions
tools/metrics/histograms/expand_owners.py
View file @ 0ae19723
......@@ -5,10 +5,7 @@
"""Functions for extracting emails and components from OWNERS files."""
import extract_histograms
import json
import os
import subprocess
import sys
import re
_EMAIL_PATTERN = r'^[\w\-\+\%\.]+\@[\w\-\+\%\.]+$'
......@@ -196,57 +193,6 @@ def _ExtractEmailAddressesFromOWNERS(path, depth=0):
return extracted_emails
def _ComponentFromDirmd(json_data, subpath):
"""Returns the component for a subpath based on dirmd output.
Returns an empty string if no component can be extracted
Args:
json_data: json object output from dirmd.
subpath: The subpath for the directory being queried, e.g. src/storage'.
"""
# If no component exists for the directory, or if METADATA migration is
# incomplete there will be no component information.
return json_data.get('dirs', {}).get(subpath,
{}).get('monorail',
{}).get('component', '')
def _ExtractComponentViaDirmd(path):
"""Returns the component for monorail issues at the given path.
Examples are 'Blink>Storage>FileAPI' and 'UI'.
Uses dirmd in third_party/depot_tools to parse metadata and walk parent
directories up to the top level of the repo.
Returns an empty string if no component can be extracted.
Args:
path: The path to an directory to query, e.g. 'src/storage'.
"""
# Verify that the paths are absolute and the root is a parent of the
# passed in path.
root_path = os.path.abspath(os.path.join(*_DIR_ABOVE_TOOLS))
path = os.path.abspath(path)
if not path.startswith(root_path):
raise Error('Path {} is not a subpath of the root path {}.'.format(
path, root_path))
subpath = path[len(root_path) + 1:] or '.' # E.g. content/public.
dirmd_exe = 'dirmd'
if sys.platform == 'win32':
dirmd_exe == 'dirmd.bat'
dirmd_path = os.path.join(*(_DIR_ABOVE_TOOLS +
['third_party', 'depot_tools', dirmd_exe]))
dirmd = subprocess.Popen([dirmd_path, 'compute', '--root', root_path, path],
stdout=subprocess.PIPE)
if dirmd.wait() != 0:
raise Error('dirmd failed.')
json_out = json.load(dirmd.stdout)
return _ComponentFromDirmd(json_out, subpath)
# TODO(crbug/1102997): remove once metadata migration is complete
def _ExtractComponentFromOWNERS(path):
"""Returns the string component associated with the file at the given path.
......@@ -415,9 +361,7 @@ def ExpandHistogramsOWNERS(histograms):
_UpdateHistogramOwners(histogram, owner, owners_to_add)
component = _ExtractComponentViaDirmd(os.path.dirname(path))
if not component:
component = _ExtractComponentFromOWNERS(path)
component = _ExtractComponentFromOWNERS(path)
if component and component not in components_with_dom_elements:
components_with_dom_elements.add(component)
_AddHistogramComponent(histogram, component)
tools/metrics/histograms/expand_owners_unittest.py
View file @ 0ae19723
......@@ -32,7 +32,6 @@ def _GetSrcRelativePath(path):
Args:
path: An absolute path, e.g. '/some/directory/chromium/src/tools/OWNERS'.
"""
# TODO(crbug/1126653): This fails if chromium is not in the path.
return path.split('chromium/')[1]
......@@ -63,61 +62,6 @@ class ExpandOwnersTest(unittest.TestCase):
super(ExpandOwnersTest, self).tearDown()
shutil.rmtree(self.temp_dir)
def testExpandOwnersUsesMetadataOverOwners(self):
"""Checks that DIR_METADATA is used if available"""
with open(os.path.join(self.temp_dir, 'DIR_METADATA'), "w+") as md:
md.write("\n".join(['monorail {', 'component: "Bees"', '}']))
absolute_path = _MakeOwnersFile('simple_OWNERS', self.temp_dir)
with open(absolute_path, 'w') as owners_file:
owners_file.write('\n'.join(
['amy@chromium.org', _DEFAULT_COMPONENT, 'rae@chromium.org']))
self.maxDiff = None
src_relative_path = _GetSrcRelativePath(absolute_path)
histograms = xml.dom.minidom.parseString("""
<histograms>
<histogram name="Caffeination" units="mg">
<owner>joe@chromium.org</owner>
<owner>{path}</owner>
<summary>I like coffee.</summary>
</histogram>
<histogram name="Maple.Syrup" units="units">
<owner>joe@chromium.org</owner>
<owner>{path}</owner>
<owner>kim@chromium.org</owner>
<summary>I like maple syrup, too.</summary>
</histogram>
</histograms>
""".format(path=src_relative_path))
expected_histograms = xml.dom.minidom.parseString("""
<histograms>
<histogram name="Caffeination" units="mg">
<owner>joe@chromium.org</owner>
<owner>amy@chromium.org</owner>
<owner>rae@chromium.org</owner>
<summary>I like coffee.</summary>
<component>Bees</component>
</histogram>
<histogram name="Maple.Syrup" units="units">
<owner>joe@chromium.org</owner>
<owner>amy@chromium.org</owner>
<owner>rae@chromium.org</owner>
<owner>kim@chromium.org</owner>
<summary>I like maple syrup, too.</summary>
<component>Bees</component>
</histogram>
</histograms>
""")
expand_owners.ExpandHistogramsOWNERS(histograms)
self.assertMultiLineEqual(histograms.toxml(), expected_histograms.toxml())
def testExpandOwnersWithSimpleOWNERSFilePath(self):
"""Checks that OWNERS files are expanded."""
absolute_path = _MakeOwnersFile('simple_OWNERS', self.temp_dir)
......@@ -382,7 +326,7 @@ class ExpandOwnersTest(unittest.TestCase):
with self.assertRaisesRegexp(
expand_owners.Error,
'The histogram Caffeination must have a valid primary owner, i.e. a '
'The histogram Caffeination must have a valid first owner, i.e. a '
'person with an @google.com or @chromium.org email address.'):
expand_owners.ExpandHistogramsOWNERS(histograms_without_valid_first_owner)
......@@ -405,7 +349,7 @@ class ExpandOwnersTest(unittest.TestCase):
with self.assertRaisesRegexp(
expand_owners.Error,
'The histogram Caffeination must have a valid primary owner, i.e. a '
'The histogram Caffeination must have a valid first owner, i.e. a '
'person with an @google.com or @chromium.org email address.'):
expand_owners.ExpandHistogramsOWNERS(histograms_without_valid_first_owner)
......@@ -428,7 +372,7 @@ class ExpandOwnersTest(unittest.TestCase):
with self.assertRaisesRegexp(
expand_owners.Error,
'The histogram Caffeination must have a valid primary owner, i.e. a '
'The histogram Caffeination must have a valid first owner, i.e. a '
'person with an @google.com or @chromium.org email address.'):
expand_owners.ExpandHistogramsOWNERS(histograms_without_valid_first_owner)
......@@ -476,6 +420,11 @@ class ExpandOwnersTest(unittest.TestCase):
r'No emails could be derived from .*empty_OWNERS\.'):
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
......@@ -500,7 +449,8 @@ class ExpandOwnersTest(unittest.TestCase):
</histograms>
""".format(src_relative_path))
self.assertIsNone(expand_owners.ExpandHistogramsOWNERS(histograms_string))
self.assertEqual(
expand_owners.ExpandHistogramsOWNERS(histograms_string), [])
def testExpandOwnersWithoutOWNERSPathPrefix(self):
"""Checks that an error is raised when the path is not well-formatted."""
......
tools/metrics/metrics_python_tests.py
View file @ 0ae19723
......@@ -23,8 +23,6 @@ def resolve(*paths):
sys.exit(
typ.main(tests=resolve(
'actions/extract_actions_test.py',
# TODO(crbug/1126653): Turn back on once tests can pass again.
# 'histograms/expand_owners_unittest.py',
'histograms/extract_histograms_test.py',
'histograms/generate_expired_histograms_array_unittest.py',
'histograms/pretty_print_test.py',
......
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