Commit 57c4e4a5 authored by Raphael Kubo da Costa's avatar Raphael Kubo da Costa Committed by Commit Bot

[Owners Extractor] Do not assert when LayoutTests/ itself is passed.

Commit 87454345 ("[Owners Extractor] Fix
issues found in the actual run") added an assertion to the code to verify
that we are not passing a directory above LayoutTests/external to
find_and_extract_owners().

The assertion gets triggered with valid paths though; for example, changes
to LayoutTests/TestExpectations means //third_party/WebKit/LayoutTests gets
passed to the function, which aborts instead of just skipping that path.
This, in turn, broke our automatic web-platform-tests imports, as a recent
change there meant our scripts had to delete a test from TestExpectations.

For example:
https://luci-milo.appspot.com/buildbot/chromium.infra.cron/wpt-importer/415

leads to
Traceback (most recent call last):
  File "/mnt/data/b/rr/tmps9aWJe/w/src/third_party/WebKit/Tools/Scripts/wpt-import", line 18, in <module>
    host.exit(importer.main())
  File "/mnt/data/b/rr/tmps9aWJe/w/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py", line 122, in main
    self._upload_cl()
  File "/mnt/data/b/rr/tmps9aWJe/w/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py", line 385, in _upload_cl
    directory_owners = self.get_directory_owners()
  File "/mnt/data/b/rr/tmps9aWJe/w/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py", line 409, in get_directory_owners
    return extractor.list_owners(changed_files)
  File "/mnt/data/b/rr/tmps9aWJe/w/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/directory_owners_extractor.py", line 42, in list_owners
    owners_file, owners = self.find_and_extract_owners(self.filesystem.dirname(relpath))
  File "/mnt/data/b/rr/tmps9aWJe/w/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/directory_owners_extractor.py", line 69, in find_and_extract_owners
    assert directory.startswith(external_root)
AssertionError

This change just turns the assertion into a check to unbreak the imports, a
better one that increases test coverage should probably be landed later.

TBR=qyearsley@chromium.org,robertma@chromium.org

Bug: 713987
Change-Id: I192f37bf10dad98bc32cf2ff15cefa09e440c4ee
Reviewed-on: https://chromium-review.googlesource.com/599908
Commit-Queue: Raphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com>
Reviewed-by: default avatarRaphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#491704}
parent 4daa3864
......@@ -66,6 +66,10 @@ class DirectoryOwnersExtractor(object):
assert not self.filesystem.isabs(start_directory)
directory = self.finder.path_from_chromium_base(start_directory)
external_root = self.finder.path_from_layout_tests('external')
# Changes to LayoutTests/TestExpectations itself should be skipped and
# not raise an assertion.
if directory == self.finder.layout_tests_dir():
return None, None
assert directory.startswith(external_root)
while directory != external_root:
owners_file = self.filesystem.join(directory, 'OWNERS')
......
......@@ -83,6 +83,8 @@ class DirectoryOwnersExtractorTest(unittest.TestCase):
def test_find_and_extract_owners_out_of_tree(self):
with self.assertRaises(AssertionError):
self.extractor.find_and_extract_owners('third_party/WebKit/LayoutTests/other')
self.assertEqual(self.extractor.find_and_extract_owners('third_party/WebKit/LayoutTests'),
(None, None))
def test_extract_owners(self):
self.filesystem.files = {
......
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