Add more tests for owners.py, remove unneeded code, make syntax errors more helpful

Review URL: http://codereview.chromium.org/6639010

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@77522 0039d316-1c4b-4281-b951-d872f2087c98

owners.py

@@ -16,17 +16,14 @@ BASIC_EMAIL_REGEXP = r'^[\w\-\+\%\.]+\@[\w\-\+\%\.]+$'
 
 
 class SyntaxErrorInOwnersFile(Exception):
-  def __init__(self, path, line, msg):
-    super(SyntaxErrorInOwnersFile, self).__init__((path, line, msg))
+  def __init__(self, path, lineno, msg):
+    super(SyntaxErrorInOwnersFile, self).__init__((path, lineno, msg))
     self.path = path
-    self.line = line
+    self.lineno = lineno
     self.msg = msg
 
   def __str__(self):
-    if self.msg:
-      return "%s:%d syntax error: %s" % (self.path, self.line, self.msg)
-    else:
-      return "%s:%d syntax error" % (self.path, self.line)
+    return "%s:%d syntax error: %s" % (self.path, self.lineno, self.msg)
 
 
 class Database(object):
@@ -141,11 +138,16 @@ class Database(object):
       if line == 'set noparent':
         self.stop_looking.add(dirpath)
         continue
+      if line.startswith('set '):
+        raise SyntaxErrorInOwnersFile(owners_path, lineno,
+            'unknown option: "%s"' % line[4:].strip())
       if self.email_regexp.match(line) or line == EVERYONE:
         self.owned_by.setdefault(line, set()).add(dirpath)
         self.owners_for.setdefault(dirpath, set()).add(line)
         continue
-      raise SyntaxErrorInOwnersFile(owners_path, lineno, line)
+      raise SyntaxErrorInOwnersFile(owners_path, lineno,
+            ('line is not a comment, a "set" directive, '
+             'or an email address: "%s"' % line))
 
   def _covering_set_of_owners_for(self, files):
     # TODO(dpranke): implement the greedy algorithm for covering sets, and

tests/owners_unittest.py

@@ -72,6 +72,8 @@ class OwnersDatabaseTest(unittest.TestCase):
 
   def test_covered_by__owners_propagates_down(self):
     self.assert_covered_by(['chrome/gpu/OWNERS'], [ben])
+
+  def test_covered_by__no_file_in_dir(self):
     self.assert_covered_by(['/chrome/renderer/gpu/gpu_channel_host.h'], [peter])
 
   def assert_not_covered_by(self, files, reviewers, unreviewed_files):
@@ -114,8 +116,11 @@ class OwnersDatabaseTest(unittest.TestCase):
     db = self.db()
     self.files['/foo/OWNERS'] = owners_file_contents
     self.files['/foo/DEPS'] = ''
-    self.assertRaises(owners.SyntaxErrorInOwnersFile, db.reviewers_for,
-      ['/foo/DEPS'])
+    try:
+      db.reviewers_for(['/foo/DEPS'])
+      self.fail()  # pragma: no cover
+    except owners.SyntaxErrorInOwnersFile, e:
+      self.assertTrue(str(e).startswith('/foo/OWNERS:1'))
 
   def test_syntax_error__unknown_token(self):
     self.assert_syntax_error('{}\n')