make tests work, implement 'set noparent', owners propagating down

tests/owners_unittest.py

Index: tests/owners_unittest.py
diff --git a/tests/owners_unittest.py b/tests/owners_unittest.py
index cc4e0d834d87d9c6aa5c2cfafc09bb1660001aee..cf1e2d9c22d59efda1a19c684b3b4b9cc22ad020 100755
--- a/tests/owners_unittest.py
+++ b/tests/owners_unittest.py
@@ -19,15 +19,17 @@ peter = 'peter@example.com'
 
 def owners_file(*email_addresses, **kwargs):
   s = ''
+  if kwargs.get('comment'):
+    s += '# %s\n' % kwargs.get('comment')
   if kwargs.get('noparent'):
-    s = 'set noparent\n'
+    s += 'set noparent\n'
   return s + '\n'.join(email_addresses) + '\n'
 
 
 def test_repo():
   return filesystem_mock.MockFileSystem(files={
     '/DEPS' : '',
-    '/OWNERS': owners_file('*'),
+    '/OWNERS': owners_file(owners.EVERYONE),
     '/base/vlog.h': '',
     '/chrome/OWNERS': owners_file(ben, brett),
     '/chrome/gpu/OWNERS': owners_file(ken),
@@ -35,7 +37,7 @@ def test_repo():
     '/chrome/renderer/OWNERS': owners_file(peter),
     '/chrome/renderer/gpu/gpu_channel_host.h': '',
     '/chrome/renderer/safe_browsing/scorer.h': '',
-    '/content/OWNERS': owners_file(john, darin, noparent=True),
+    '/content/OWNERS': owners_file(john, darin, comment='foo', noparent=True),
     '/content/content.gyp': '',
   })
 
@@ -53,70 +55,76 @@ class OwnersDatabaseTest(unittest.TestCase):
     os_path = os_path or self.repo
     return owners.Database(root, fopen, os_path)
 
-  def assertReviewersFor(self, files, expected_reviewers):
-    db = self.db()
-    self.assertEquals(db.ReviewersFor(set(files)), set(expected_reviewers))
+  def test_Constructor(self):
+    self.assertNotEquals(self.db(), None)
 
-  def assertCoveredBy(self, files, reviewers):
+  def assert_CoveredBy(self, files, reviewers):
     db = self.db()
     self.assertTrue(db.FilesAreCoveredBy(set(files), set(reviewers)))
 
-  def assertNotCoveredBy(self, files, reviewers, unreviewed_files):
+  def test_CoveredBy_Everyone(self):
+    self.assert_CoveredBy(['DEPS'], [john])
+    self.assert_CoveredBy(['DEPS'], [darin])
+
+  def test_CoveredBy_Explicit(self):
+    self.assert_CoveredBy(['content/content.gyp'], [john])
+    self.assert_CoveredBy(['chrome/gpu/OWNERS'], [ken])
+
+  def test_CoveredBy_OwnersPropagatesDown(self):
+    self.assert_CoveredBy(['chrome/gpu/OWNERS'], [ben])
+    self.assert_CoveredBy(['/chrome/renderer/gpu/gpu_channel_host.h'], [peter])
+
+  def assert_NotCoveredBy(self, files, reviewers, unreviewed_files):
     db = self.db()
     self.assertEquals(db.FilesNotCoveredBy(set(files), set(reviewers)),
                       set(unreviewed_files))
 
-  def test_constructor(self):
-    self.assertNotEquals(self.db(), None)
+  def test_NotCoveredBy_NeedAtLeastOneReviewer(self):
+    self.assert_NotCoveredBy(['DEPS'], [], ['DEPS'])
 
-  def test_owners_for(self):
-    self.assertReviewersFor(['DEPS'], [owners.ANYONE])
-    self.assertReviewersFor(['content/content.gyp'], [john, darin])
-    self.assertReviewersFor(['chrome/gpu/gpu_channel.h'], [ken])
-
-  def test_covered_by(self):
-    self.assertCoveredBy(['DEPS'], [john])
-    self.assertCoveredBy(['DEPS'], [darin])
-    self.assertCoveredBy(['content/content.gyp'], [john])
-    self.assertCoveredBy(['chrome/gpu/OWNERS'], [ken])
-    self.assertCoveredBy(['chrome/gpu/OWNERS'], [ben])
-
-  def test_not_covered_by(self):
-    self.assertNotCoveredBy(['DEPS'], [], ['DEPS'])
-    self.assertNotCoveredBy(['content/content.gyp'], [ben],
-      ['content/content.gyp'])
-    self.assertNotCoveredBy(
-      ['chrome/gpu/gpu_channel.h', 'chrome/renderer/gpu/gpu_channel_host.h'],
-      [peter], ['chrome/gpu/gpu_channel.h'])
-    self.assertNotCoveredBy(
+  def test_NotCoveredBy_OwnersPropagatesDown(self):
+    self.assert_NotCoveredBy(
       ['chrome/gpu/gpu_channel.h', 'chrome/renderer/gpu/gpu_channel_host.h'],
       [ben], [])
 
-  def test_comments_in_owners_file(self):
-    # pylint: disable=W0212
+  def test_NotCoveredBy_PartialCovering(self):
+    self.assert_NotCoveredBy(
+      ['content/content.gyp', 'chrome/renderer/gpu/gpu_channel_host.h'],
+      [peter], ['content/content.gyp'])
+
+  def test_NotCoveredBy_SetNoParentWorks(self):
+    self.assert_NotCoveredBy(['content/content.gyp'], [ben],
+                             ['content/content.gyp'])
+
+  def assert_ReviewersFor(self, files, expected_reviewers):
     db = self.db()
-    # Tests that this doesn't raise an error.
-    db._ReadOwnersFile('OWNERS', 'DEPS')
+    self.assertEquals(db.ReviewersFor(set(files)), set(expected_reviewers))
+
+  def test_ReviewersFor_BasicFunctionality(self):
+    self.assert_ReviewersFor(['chrome/gpu/gpu_channel.h'],
+                             [ken, ben, brett, owners.EVERYONE])
+
+  def test_ReviewersFor_SetNoParentWorks(self):
+    self.assert_ReviewersFor(['content/content.gyp'], [john, darin])
 
-  def test_syntax_error_in_owners_file(self):
-    # pylint: disable=W0212
+  def test_ReviewersFor_WildcardDir(self):
+    self.assert_ReviewersFor(['DEPS'], [owners.EVERYONE])
+
+  def assert_SyntaxError(self, owners_file_contents):
     db = self.db()
-    self.files['/foo/OWNERS'] = '{}\n'
-    self.files['/foo/DEPS'] = '# DEPS\n'
-    self.assertRaises(owners.SyntaxErrorInOwnersFile, db._ReadOwnersFile,
-      '/foo/OWNERS', '/foo/DEPS')
-
-    self.files['/bar/OWNERS'] = 'set myparentislinus\n'
-    self.files['/bar/DEPS'] = '# DEPS\n'
-    self.assertRaises(owners.SyntaxErrorInOwnersFile, db._ReadOwnersFile,
-      '/bar/OWNERS', '/bar/DEPS')
-
-  def test_owners_propagates_down(self):
-    self.assertCoveredBy(['/chrome/renderer/gpu/gpu_channel_host.h'], [peter])
-
-  def test_set_noparent(self):
-    self.assertNotCoveredBy(['/content/content.gyp'], [peter],
-      ['/content/content.gyp'])
+    self.files['/foo/OWNERS'] = owners_file_contents
+    self.files['/foo/DEPS'] = ''
+    self.assertRaises(owners.SyntaxErrorInOwnersFile, db.ReviewersFor,
+      ['/foo/DEPS'])
+
+  def test_SyntaxError_UnknownToken(self):
+    self.assert_SyntaxError('{}\n')
+
+  def test_SyntaxError_UnknownSet(self):
+    self.assert_SyntaxError('set myparentisbillgates\n')

[M-A Ruel, 2011/03/08 20:24:03]

mydadisbillgates would be more specific. :)

[Dirk Pranke, 2011/03/08 22:05:27]

Heh. Done.

+
+  def test_SyntaxError_BadEmail(self):
+    self.assert_SyntaxError('ben\n')
 
 
 if __name__ == '__main__':