Regenerate MANIFEST.json when WPT tests are run

third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py

Index: third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py b/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py
index f9953679301cce7cc30078d4602a3f9b895dd2f8..e5d789fe5e63504e95052f5a450bf8e6f6ff2c8b 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py
@@ -244,37 +244,41 @@ class PortTest(unittest.TestCase):
         self.assertEqual(tests, [])
 
     @staticmethod
-    def _add_manifest_to_mock_file_system(filesystem):
-        filesystem.write_text_file(LAYOUT_TEST_DIR + '/external/wpt/MANIFEST.json', json.dumps({
-            'items': {
-                'testharness': {
-                    'dom/ranges/Range-attributes.html': [
-                        ['/dom/ranges/Range-attributes.html', {}]
-                    ],
-                    'dom/ranges/Range-attributes-slow.html': [
-                        ['/dom/ranges/Range-attributes.html', {'timeout': 'long'}]
-                    ],
-                    'console/console-is-a-namespace.any.js': [
-                        ['/console/console-is-a-namespace.any.html', {}],
-                        ['/console/console-is-a-namespace.any.worker.html', {}],
-                    ],
-                },
-                'manual': {},
-                'reftest': {
-                    'html/dom/elements/global-attributes/dir_auto-EN-L.html': [
-                        [
-                            '/html/dom/elements/global-attributes/dir_auto-EN-L.html',
+    def _add_manifest_to_mock_file_system(filesystem, contents=None):
+        if not contents:
+            contents = {
+                'items': {
+                    'testharness': {
+                        'dom/ranges/Range-attributes.html': [
+                            ['/dom/ranges/Range-attributes.html', {}]
+                        ],
+                        'dom/ranges/Range-attributes-slow.html': [
+                            ['/dom/ranges/Range-attributes.html', {'timeout': 'long'}]
+                        ],
+                        'console/console-is-a-namespace.any.js': [
+                            ['/console/console-is-a-namespace.any.html', {}],
+                            ['/console/console-is-a-namespace.any.worker.html', {}],
+                        ],
+                    },
+                    'manual': {},
+                    'reftest': {
+                        'html/dom/elements/global-attributes/dir_auto-EN-L.html': [
                             [
+                                '/html/dom/elements/global-attributes/dir_auto-EN-L.html',
                                 [
-                                    '/html/dom/elements/global-attributes/dir_auto-EN-L-ref.html',
-                                    '=='
-                                ]
-                            ],
-                            {'timeout': 'long'}
-                        ]
-                    ],
-                },
-            }}))
+                                    [
+                                        '/html/dom/elements/global-attributes/dir_auto-EN-L-ref.html',
+                                        '=='
+                                    ]
+                                ],
+                                {'timeout': 'long'}
+                            ]
+                        ],
+                    },
+                }
+            }
+
+        filesystem.write_text_file(LAYOUT_TEST_DIR + '/external/wpt/MANIFEST.json', json.dumps(contents))
         filesystem.write_text_file(LAYOUT_TEST_DIR + '/external/wpt/dom/ranges/Range-attributes.html', '')
         filesystem.write_text_file(LAYOUT_TEST_DIR + '/external/wpt/console/console-is-a-namespace.any.js', '')
         filesystem.write_text_file(LAYOUT_TEST_DIR + '/external/wpt/common/blank.html', 'foo')
@@ -285,8 +289,55 @@ class PortTest(unittest.TestCase):
         self.assertNotIn('external/wpt/common/blank.html', port.tests([]))
 
     def test_find_one_if_in_manifest(self):
+        mock_generate_manifest_calls = []
+
+        def mock_generate_manifest():
+            mock_generate_manifest_calls.append('called')
+            # Contents are not important, just update so that they're different
+            # so the file hashes to a different value. This is meant to mock
+            # the result of running the update manifest command.

[qyearsley, 2017/01/26 23:08:51]

Since the contents are not important, it would be nice to make it
shorter/simpler, I think.

+            PortTest._add_manifest_to_mock_file_system(port.host.filesystem, {
+                'items': {
+                    'testharness': {
+                        'dom/ranges/Range-attributes.html': [
+                            ['/dom/ranges/Range-attributes.html', {}]
+                        ],
+                        'dom/ranges/Range-attributes-slow.html': [
+                            ['/dom/ranges/Range-attributes.html', {'timeout': 'long'}]
+                        ],
+                        'console/console-is-a-namespace.any.js': [
+                            ['/console/console-is-a-namespace.any.html', {}],
+                            ['/console/console-is-a-namespace.any.worker.html', {}],
+                        ],
+                        'rutabaga.html': [
+                            ['/rutabaga.html', {}],
+                        ],
+                    },
+                    'manual': {},
+                    'reftest': {
+                        'html/dom/elements/global-attributes/dir_auto-EN-L.html': [
+                            [
+                                '/html/dom/elements/global-attributes/dir_auto-EN-L.html',
+                                [
+                                    [
+                                        '/html/dom/elements/global-attributes/dir_auto-EN-L-ref.html',
+                                        '=='
+                                    ]
+                                ],
+                                {'timeout': 'long'}
+                            ]
+                        ],
+                    },
+                }
+            })
+
         port = self.make_port(with_tests=True)
         PortTest._add_manifest_to_mock_file_system(port.host.filesystem)
+        port._generate_manifest = mock_generate_manifest  # pylint: disable=W0212

[jeffcarp, 2017/01/26 22:26:39]

After rebasing I couldn't get the tests to pass without taking this different
approach - let me know if this works for you.

[qyearsley, 2017/01/26 23:08:51]

I think it's OK but might be nicer if it were simpler. If you were to change the
test to be simpler and test fewer things, then maybe a mock generate manifest
function wouldn't be necessary?

e.g. if we don't replace _generate_manifest, could we just assert that a
manifest command was called, without caring about whether it changes the
MANIFEST.json on the filesystem?

Also, # pylint: disable=W0212 can be changed to pylint: disable=protected-access

+
+        capture = OutputCapture()

[qyearsley, 2017/01/26 23:08:51]

Is it possible to use LoggingTestCase.assertLog (after making this test case
class a subclass of webkitpy.common.log_testing.LoggingTestCase), instead of
using OutputCapture?

+        capture.capture_output()
+
         self.assertIn('external/wpt/dom/ranges/Range-attributes.html', port.tests([]))
         self.assertNotIn('external/wpt/console/console-is-a-namespace.any.js', port.tests([]))
         self.assertEqual(port.tests(['external']), ['external/wpt/dom/ranges/Range-attributes.html'])
@@ -297,6 +348,10 @@ class PortTest(unittest.TestCase):
         self.assertEqual(port.tests(['external/wpt/dom/ranges/Range-attributes.html']),
                          ['external/wpt/dom/ranges/Range-attributes.html'])
 
+        _, _, logs = capture.restore_output()
+        self.assertEqual(len(mock_generate_manifest_calls), 1)
+        self.assertIn('MANIFEST.json has been updated to reflect changes in external/wpt!', logs)
+
     def test_is_test_file(self):
         port = self.make_port(with_tests=True)
         is_test_file = functools.partial(Port.is_test_file, port, port.host.filesystem)