aiden
/
depot_tools
mirror of https://chromium.googlesource.com/chromium/tools/depot_tools
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

use PEP-8 naming for method names in owners.py, tests/owners_unittest.py

Browse Source 
Review URL: http://codereview.chromium.org/6646007

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@77521 0039d316-1c4b-4281-b951-d872f2087c98
experimental/szager/collated-output
dpranke@chromium.org 15 years ago
parent 5fe603e8c7
commit 7eea259694
2 changed files with 66 additions and 66 deletions
Show Stats Download Patch File Download Diff File

56
owners.py
Unescape Escape View File

@ -61,73 +61,73 @@ class Database(object):
# (This is implicitly true for the root directory). # (This is implicitly true for the root directory).
self.stop_looking = set(['']) self.stop_looking = set([''])
def ReviewersFor(self, files): def reviewers_for(self, files):
"""Returns a suggested set of reviewers that will cover the set of files. """Returns a suggested set of reviewers that will cover the set of files.
files is a set of paths relative to (and under) self.root.""" files is a set of paths relative to (and under) self.root."""
self._CheckPaths(files) self._check_paths(files)
self._LoadDataNeededFor(files) self._load_data_needed_for(files)
return self._CoveringSetOfOwnersFor(files) return self._covering_set_of_owners_for(files)
def FilesAreCoveredBy(self, files, reviewers): def files_are_covered_by(self, files, reviewers):
"""Returns whether every file is owned by at least one reviewer.""" """Returns whether every file is owned by at least one reviewer."""
return not self.FilesNotCoveredBy(files, reviewers) return not self.files_not_covered_by(files, reviewers)
def FilesNotCoveredBy(self, files, reviewers): def files_not_covered_by(self, files, reviewers):
"""Returns the set of files that are not owned by at least one reviewer.""" """Returns the set of files that are not owned by at least one reviewer."""
self._CheckPaths(files) self._check_paths(files)
self._CheckReviewers(reviewers) self._check_reviewers(reviewers)
if not reviewers: if not reviewers:
return files return files
self._LoadDataNeededFor(files) self._load_data_needed_for(files)
files_by_dir = self._FilesByDir(files) files_by_dir = self._files_by_dir(files)
covered_dirs = self._DirsCoveredBy(reviewers) covered_dirs = self._dirs_covered_by(reviewers)
uncovered_files = [] uncovered_files = []
for d, files_in_d in files_by_dir.iteritems(): for d, files_in_d in files_by_dir.iteritems():
if not self._IsDirCoveredBy(d, covered_dirs): if not self._is_dir_covered_by(d, covered_dirs):
uncovered_files.extend(files_in_d) uncovered_files.extend(files_in_d)
return set(uncovered_files) return set(uncovered_files)
def _CheckPaths(self, files): def _check_paths(self, files):
def _isunder(f, pfx): def _is_under(f, pfx):
return self.os_path.abspath(self.os_path.join(pfx, f)).startswith(pfx) return self.os_path.abspath(self.os_path.join(pfx, f)).startswith(pfx)
assert all(_isunder(f, self.os_path.abspath(self.root)) for f in files) assert all(_is_under(f, self.os_path.abspath(self.root)) for f in files)
def _CheckReviewers(self, reviewers): def _check_reviewers(self, reviewers):
"""Verifies each reviewer is a valid email address.""" """Verifies each reviewer is a valid email address."""
assert all(self.email_regexp.match(r) for r in reviewers) assert all(self.email_regexp.match(r) for r in reviewers)
def _FilesByDir(self, files): def _files_by_dir(self, files):
dirs = {} dirs = {}
for f in files: for f in files:
dirs.setdefault(self.os_path.dirname(f), []).append(f) dirs.setdefault(self.os_path.dirname(f), []).append(f)
return dirs return dirs
def _DirsCoveredBy(self, reviewers): def _dirs_covered_by(self, reviewers):
dirs = self.owned_by[EVERYONE] dirs = self.owned_by[EVERYONE]
for r in reviewers: for r in reviewers:
dirs = dirs | self.owned_by.get(r, set()) dirs = dirs | self.owned_by.get(r, set())
return dirs return dirs
def _StopLooking(self, dirname): def _stop_looking(self, dirname):
return dirname in self.stop_looking return dirname in self.stop_looking
def _IsDirCoveredBy(self, dirname, covered_dirs): def _is_dir_covered_by(self, dirname, covered_dirs):
while not dirname in covered_dirs and not self._StopLooking(dirname): while not dirname in covered_dirs and not self._stop_looking(dirname):
dirname = self.os_path.dirname(dirname) dirname = self.os_path.dirname(dirname)
return dirname in covered_dirs return dirname in covered_dirs
def _LoadDataNeededFor(self, files): def _load_data_needed_for(self, files):
for f in files: for f in files:
dirpath = self.os_path.dirname(f) dirpath = self.os_path.dirname(f)
while not dirpath in self.owners_for: while not dirpath in self.owners_for:
self._ReadOwnersInDir(dirpath) self._read_owners_in_dir(dirpath)
if self._StopLooking(dirpath): if self._stop_looking(dirpath):
break break
dirpath = self.os_path.dirname(dirpath) dirpath = self.os_path.dirname(dirpath)
def _ReadOwnersInDir(self, dirpath): def _read_owners_in_dir(self, dirpath):
owners_path = self.os_path.join(self.root, dirpath, 'OWNERS') owners_path = self.os_path.join(self.root, dirpath, 'OWNERS')
if not self.os_path.exists(owners_path): if not self.os_path.exists(owners_path):
return return
@ -147,7 +147,7 @@ class Database(object):
continue continue
raise SyntaxErrorInOwnersFile(owners_path, lineno, line) raise SyntaxErrorInOwnersFile(owners_path, lineno, line)
def _CoveringSetOfOwnersFor(self, files): def _covering_set_of_owners_for(self, files):
# TODO(dpranke): implement the greedy algorithm for covering sets, and # TODO(dpranke): implement the greedy algorithm for covering sets, and
# consider returning multiple options in case there are several equally # consider returning multiple options in case there are several equally
# short combinations of owners. # short combinations of owners.
@ -156,7 +156,7 @@ class Database(object):
dirname = self.os_path.dirname(f) dirname = self.os_path.dirname(f)
while dirname in self.owners_for: while dirname in self.owners_for:
every_owner |= self.owners_for[dirname] every_owner |= self.owners_for[dirname]
if self._StopLooking(dirname): if self._stop_looking(dirname):
break break
dirname = self.os_path.dirname(dirname) dirname = self.os_path.dirname(dirname)
return every_owner return every_owner

76
tests/owners_unittest.py
Unescape Escape View File

@ -55,76 +55,76 @@ class OwnersDatabaseTest(unittest.TestCase):
os_path = os_path or self.repo os_path = os_path or self.repo
return owners.Database(root, fopen, os_path) return owners.Database(root, fopen, os_path)
def test_Constructor(self): def test_constructor(self):
self.assertNotEquals(self.db(), None) self.assertNotEquals(self.db(), None)
def assert_CoveredBy(self, files, reviewers): def assert_covered_by(self, files, reviewers):
db = self.db() db = self.db()
self.assertTrue(db.FilesAreCoveredBy(set(files), set(reviewers))) self.assertTrue(db.files_are_covered_by(set(files), set(reviewers)))
def test_CoveredBy_Everyone(self): def test_covered_by__everyone(self):
self.assert_CoveredBy(['DEPS'], [john]) self.assert_covered_by(['DEPS'], [john])
self.assert_CoveredBy(['DEPS'], [darin]) self.assert_covered_by(['DEPS'], [darin])
def test_CoveredBy_Explicit(self): def test_covered_by__explicit(self):
self.assert_CoveredBy(['content/content.gyp'], [john]) self.assert_covered_by(['content/content.gyp'], [john])
self.assert_CoveredBy(['chrome/gpu/OWNERS'], [ken]) self.assert_covered_by(['chrome/gpu/OWNERS'], [ken])
def test_CoveredBy_OwnersPropagatesDown(self): def test_covered_by__owners_propagates_down(self):
self.assert_CoveredBy(['chrome/gpu/OWNERS'], [ben]) self.assert_covered_by(['chrome/gpu/OWNERS'], [ben])
self.assert_CoveredBy(['/chrome/renderer/gpu/gpu_channel_host.h'], [peter]) self.assert_covered_by(['/chrome/renderer/gpu/gpu_channel_host.h'], [peter])
def assert_NotCoveredBy(self, files, reviewers, unreviewed_files): def assert_not_covered_by(self, files, reviewers, unreviewed_files):
db = self.db() db = self.db()
self.assertEquals(db.FilesNotCoveredBy(set(files), set(reviewers)), self.assertEquals(db.files_not_covered_by(set(files), set(reviewers)),
set(unreviewed_files)) set(unreviewed_files))
def test_NotCoveredBy_NeedAtLeastOneReviewer(self): def test_not_covered_by__need_at_least_one_reviewer(self):
self.assert_NotCoveredBy(['DEPS'], [], ['DEPS']) self.assert_not_covered_by(['DEPS'], [], ['DEPS'])
def test_NotCoveredBy_OwnersPropagatesDown(self): def test_not_covered_by__owners_propagates_down(self):
self.assert_NotCoveredBy( self.assert_not_covered_by(
['chrome/gpu/gpu_channel.h', 'chrome/renderer/gpu/gpu_channel_host.h'], ['chrome/gpu/gpu_channel.h', 'chrome/renderer/gpu/gpu_channel_host.h'],
[ben], []) [ben], [])
def test_NotCoveredBy_PartialCovering(self): def test_not_covered_by__partial_covering(self):
self.assert_NotCoveredBy( self.assert_not_covered_by(
['content/content.gyp', 'chrome/renderer/gpu/gpu_channel_host.h'], ['content/content.gyp', 'chrome/renderer/gpu/gpu_channel_host.h'],
[peter], ['content/content.gyp']) [peter], ['content/content.gyp'])
def test_NotCoveredBy_SetNoParentWorks(self): def test_not_covered_by__set_noparent_works(self):
self.assert_NotCoveredBy(['content/content.gyp'], [ben], self.assert_not_covered_by(['content/content.gyp'], [ben],
['content/content.gyp']) ['content/content.gyp'])
def assert_ReviewersFor(self, files, expected_reviewers): def assert_reviewers_for(self, files, expected_reviewers):
db = self.db() db = self.db()
self.assertEquals(db.ReviewersFor(set(files)), set(expected_reviewers)) self.assertEquals(db.reviewers_for(set(files)), set(expected_reviewers))
def test_ReviewersFor_BasicFunctionality(self): def test_reviewers_for__basic_functionality(self):
self.assert_ReviewersFor(['chrome/gpu/gpu_channel.h'], self.assert_reviewers_for(['chrome/gpu/gpu_channel.h'],
[ken, ben, brett, owners.EVERYONE]) [ken, ben, brett, owners.EVERYONE])
def test_ReviewersFor_SetNoParentWorks(self): def test_reviewers_for__set_noparent_works(self):
self.assert_ReviewersFor(['content/content.gyp'], [john, darin]) self.assert_reviewers_for(['content/content.gyp'], [john, darin])
def test_ReviewersFor_WildcardDir(self): def test_reviewers_for__wildcard_dir(self):
self.assert_ReviewersFor(['DEPS'], [owners.EVERYONE]) self.assert_reviewers_for(['DEPS'], [owners.EVERYONE])
def assert_SyntaxError(self, owners_file_contents): def assert_syntax_error(self, owners_file_contents):
db = self.db() db = self.db()
self.files['/foo/OWNERS'] = owners_file_contents self.files['/foo/OWNERS'] = owners_file_contents
self.files['/foo/DEPS'] = '' self.files['/foo/DEPS'] = ''
self.assertRaises(owners.SyntaxErrorInOwnersFile, db.ReviewersFor, self.assertRaises(owners.SyntaxErrorInOwnersFile, db.reviewers_for,
['/foo/DEPS']) ['/foo/DEPS'])
def test_SyntaxError_UnknownToken(self): def test_syntax_error__unknown_token(self):
self.assert_SyntaxError('{}\n') self.assert_syntax_error('{}\n')
def test_SyntaxError_UnknownSet(self): def test_syntax_error__unknown_set(self):
self.assert_SyntaxError('set myfatherisbillgates\n') self.assert_syntax_error('set myfatherisbillgates\n')
def test_SyntaxError_BadEmail(self): def test_syntax_error__bad_email(self):
self.assert_SyntaxError('ben\n') self.assert_syntax_error('ben\n')
if __name__ == '__main__': if __name__ == '__main__':

Write Preview
Loading…
Cancel
Save