Changeset 283037 in webkit

Timestamp:

Sep 24, 2021, 5:58:41 AM (4 years ago)

Author:

Sam Sneddon

Message:

LayoutTestFinder should return tests in order
​https://bugs.webkit.org/show_bug.cgi?id=230684

Reviewed by Jonathan Bedard.

It has always been intended that LayoutTestFinder return tests in a deterministic order;
however, a bug in find_files means that directories found by glob aren't sorted, and hence
running run-webkit-tests with no arguments ends up running the top-level directories in a
non-deterministic order.

• Scripts/webkitpy/common/find_files.py:

(_normalized_find):
(_normalized_find.sort_by_directory_key):

• Scripts/webkitpy/common/find_files_unittest.py:

(TestWinNormalize.assert_filesystem_normalizes):
(TestWinNormalize.test_win):
(TestFindFiles):
(TestFindFiles.test_directory_sort_key):
(TestFindFiles.test_directory_sort_key_with_paths):

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/common/find_files.py (modified) (1 diff)
• Scripts/webkitpy/common/find_files_unittest.py (modified) (4 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2021-09-24  Sam Sneddon  <gsnedders@apple.com>
+
+        LayoutTestFinder should return tests in order
+        https://bugs.webkit.org/show_bug.cgi?id=230684
+
+        Reviewed by Jonathan Bedard.
+
+        It has always been intended that LayoutTestFinder return tests in a deterministic order;
+        however, a bug in find_files means that directories found by glob aren't sorted, and hence
+        running run-webkit-tests with no arguments ends up running the top-level directories in a
+        non-deterministic order.
+
+        * Scripts/webkitpy/common/find_files.py:
+        (_normalized_find):
+        (_normalized_find.sort_by_directory_key):
+        * Scripts/webkitpy/common/find_files_unittest.py:
+        (TestWinNormalize.assert_filesystem_normalizes):
+        (TestWinNormalize.test_win):
+        (TestFindFiles):
+        (TestFindFiles.test_directory_sort_key):
+        (TestFindFiles.test_directory_sort_key_with_paths):
+
 2021-09-24  Youenn Fablet  <youenn@apple.com>
 

trunk/Tools/Scripts/webkitpy/common/find_files.py

@@ -74 +74 @@
     """
 
-    paths_to_walk = itertools.chain(*(filesystem.glob(path) for path in paths))
+    def sort_by_directory_key(files_list):
+        if not directory_sort_key:
+            return files_list[:]
 
-    def sort_by_directory_key(files_list):
-        if directory_sort_key:
-            files_list.sort(key=directory_sort_key)
-        return files_list
+        return sorted(files_list, key=directory_sort_key)
+
+    paths_to_walk = itertools.chain(*(sort_by_directory_key(filesystem.glob(path)) for path in paths))
 
     all_files = itertools.chain(*(sort_by_directory_key(filesystem.files_under(path, skipped_directories, file_filter)) for path in paths_to_walk))

trunk/Tools/Scripts/webkitpy/common/find_files_unittest.py

@@ -27 +27 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+import itertools
+import random
 import sys
 import unittest
@@ -33 +35 @@
 
 from webkitpy.common.system.filesystem import FileSystem
+from webkitpy.common.system.filesystem_mock import MockFileSystem
 
 
@@ -45 +48 @@
 class TestWinNormalize(unittest.TestCase):
     def assert_filesystem_normalizes(self, filesystem):
-        self.assertEqual(find_files._normalize(filesystem, "c:\\foo",
-            ['fast/html', 'fast/canvas/*', 'compositing/foo.html']),
-            ['c:\\foo\\fast\\html', 'c:\\foo\\fast\\canvas\\*', 'c:\\foo\\compositing\\foo.html'])
+        self.assertEqual(
+            find_files._normalize(
+                filesystem,
+                "c:\\foo",
+                ['fast/html', 'fast/canvas/*', 'compositing/foo.html'],
+            ),
+            [
+                'c:\\foo\\fast\\html',
+                'c:\\foo\\fast\\canvas\\*',
+                'c:\\foo\\compositing\\foo.html',
+            ],
+        )
 
     def test_mocked_win(self):
@@ -61 +73 @@
             return
         self.assert_filesystem_normalizes(FileSystem())
+
+
+class TestFindFiles(unittest.TestCase):
+    def test_directory_sort_key(self):
+        filenames = [chr(o) for o in range(ord("a"), ord("z") + 1)]
+        fs = MockFileSystem(
+            files={c: "" for c in random.sample(filenames, len(filenames))}
+        )
+        self.assertEqual(
+            list(find_files.find(fs, "", directory_sort_key=lambda x: x)),
+            sorted(filenames),
+        )
+
+    def test_directory_sort_key_with_paths(self):
+        filenames = ["/".join(i) for i in itertools.product("abcde", "12345")]
+        fs = MockFileSystem(
+            files={c: "" for c in random.sample(filenames, len(filenames))}
+        )
+
+        test_subset = random.sample("abcde", 3)
+        self.assertEqual(
+            list(
+                find_files.find(
+                    fs,
+                    "",
+                    paths=[i + "/*" for i in test_subset],
+                    directory_sort_key=lambda x: x,
+                )
+            ),
+            ["/".join(i) for i in itertools.product(test_subset, "12345")],
+        )