Skip to content

Commit

Permalink
Fix test-webkitpy webkitflaskpy
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=261108

Reviewed by Jonathan Bedard.

webkitpy.test.finder.DirectoryTree.subpath used to consider everything except
for absolute paths as subpaths, and as such everything was always found as a
subtree of the first tree. This is clearly incorrect. Instead, require it
actually start with the name of the package described by the tree.

While we're at it, make `starting_directory` required everywhere: the only place
where it was previously None was in finder_unittest, which isn't helpful.

* Tools/Scripts/webkitpy/test/finder.py:
(_DirectoryTree.__init__):
(_DirectoryTree.subpath):
(Finder.add_tree):
* Tools/Scripts/webkitpy/test/finder_unittest.py:
(FinderTest.setUp):
(FinderTest.test_additional_system_paths):
(FinderTest.test_default_names):
(FinderTest.test_paths):
* Tools/Scripts/webkitpy/test/main.py:
(Tester.add_tree):

Canonical link: https://commits.webkit.org/267685@main
  • Loading branch information
gsnedders committed Sep 6, 2023
1 parent 8578dd9 commit 375f0a1
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 12 deletions.
13 changes: 6 additions & 7 deletions Tools/Scripts/webkitpy/test/finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,8 @@ class _DirectoryTree(object):
def __init__(self, filesystem, top_directory, starting_subdirectory):
self.filesystem = filesystem
self.top_directory = filesystem.realpath(top_directory)
self.search_directory = self.top_directory
self.top_package = ''
if starting_subdirectory:
self.top_package = starting_subdirectory.replace(filesystem.sep, '.') + '.'
self.search_directory = filesystem.join(self.top_directory, starting_subdirectory)
self.top_package = starting_subdirectory.replace(filesystem.sep, '.') + '.'
self.search_directory = filesystem.join(self.top_directory, starting_subdirectory)

def find_modules(self, suffixes, sub_directory=None):
if sub_directory:
Expand All @@ -64,7 +61,9 @@ def to_module(self, path):
def subpath(self, path):
"""Returns the relative path from the top of the tree to the path, or None if the path is not under the top of the tree."""
realpath = self.filesystem.realpath(self.filesystem.join(self.top_directory, path))
if realpath.startswith(self.top_directory + self.filesystem.sep):
if realpath == self.search_directory:
return self.top_package
if realpath.startswith(self.search_directory + self.filesystem.sep):
return realpath.replace(self.top_directory + self.filesystem.sep, '')
return None

Expand All @@ -89,7 +88,7 @@ def __init__(self, filesystem):
self.trees = []
self._names_to_skip = []

def add_tree(self, top_directory, starting_subdirectory=None):
def add_tree(self, top_directory, starting_subdirectory):
self.trees.append(_DirectoryTree(self.filesystem, top_directory, starting_subdirectory))

def skip(self, names, reason, bugid):
Expand Down
12 changes: 8 additions & 4 deletions Tools/Scripts/webkitpy/test/finder_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ def setUp(self):
files = {
'/foo/bar/baz.py': '',
'/foo/bar/baz_unittest.py': '',
'/foo/libraries/corelib/baz.py': '',
'/foo/libraries/corelib/baz_unittest.py': '',
'/foo2/bar2/baz2.py': '',
'/foo2/bar2/baz2.pyc': '',
'/foo2/bar2/baz2_integrationtest.py': '',
Expand All @@ -43,7 +45,8 @@ def setUp(self):
self.fs = MockFileSystem(files)
self.finder = Finder(self.fs)
self.finder.add_tree('/foo', 'bar')
self.finder.add_tree('/foo2')
self.finder.add_tree('/foo2', 'bar2')
self.finder.add_tree('/foo/libraries', 'corelib')

# Here we have to jump through a hoop to make sure test-webkitpy doesn't log
# any messages from these tests :(.
Expand All @@ -60,7 +63,7 @@ def tearDown(self):

def test_additional_system_paths(self):
self.assertEqual(self.finder.additional_paths(['/usr']),
['/foo', '/foo2'])
['/foo', '/foo2', '/foo/libraries'])

def test_is_module(self):
self.assertTrue(self.finder.is_module('bar.baz'))
Expand All @@ -84,8 +87,8 @@ def check_names(self, names, expected_names, find_all=True):
self.assertEqual(self.finder.find_names(names, find_all), expected_names)

def test_default_names(self):
self.check_names([], ['bar.baz_unittest', 'bar2.baz2_integrationtest'], find_all=True)
self.check_names([], ['bar.baz_unittest', 'bar2.baz2_integrationtest'], find_all=False)
self.check_names([], ['bar.baz_unittest', 'bar2.baz2_integrationtest', 'corelib.baz_unittest'], find_all=True)
self.check_names([], ['bar.baz_unittest', 'bar2.baz2_integrationtest', 'corelib.baz_unittest'], find_all=False)

# Should return the names given it, even if they don't exist.
self.check_names(['foobar'], ['foobar'], find_all=False)
Expand All @@ -101,6 +104,7 @@ def test_paths(self):
self.fs.chdir('/')
self.check_names(['bar'], ['bar.baz_unittest'])
self.check_names(['/foo/bar/'], ['bar.baz_unittest'])
self.check_names(['corelib'], ['corelib.baz_unittest'])

# This works 'by accident' since it maps onto a package.
self.check_names(['bar/'], ['bar.baz_unittest'])
Expand Down
2 changes: 1 addition & 1 deletion Tools/Scripts/webkitpy/test/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def __init__(self, filesystem=None):
self._options = None
self.upload_style = 'release'

def add_tree(self, top_directory, starting_subdirectory=None):
def add_tree(self, top_directory, starting_subdirectory):
self.finder.add_tree(top_directory, starting_subdirectory)

def skip(self, names, reason, bugid):
Expand Down

0 comments on commit 375f0a1

Please sign in to comment.