Regenerate MANIFEST.json when WPT tests are run

third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py

Index: third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py b/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py
index 7803e5b233037e7f6f652ae0d10d1e82ec362da9..5dfffbfa6f0c77eb87d21fe29b32af03881505a9 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py
@@ -178,3 +178,32 @@ class ManagerTest(unittest.TestCase):
             if not port.host.filesystem.exists(dir_name):
                 deleted_dir_count = deleted_dir_count + 1
         self.assertEqual(deleted_dir_count, 5)
+

[qyearsley, 2017/02/28 18:54:45]

As a convention, when explicitly testing protected methods in unit tests, I've
been adding a comment above the test methods like:

    # Tests for protected methods - pylint: disable=protected-access

To avoid having to repeat the pylint disable comment below. Does this sound like
an OK idea? I think that ideally one should test public methods first and not
necessarily test protected/private methods, but often having tests for private
methods is better than no tests, and in this case I think explicitly saying this
with a pylint disable comment is fine.

[jeffcarp, 2017/02/28 21:57:39]

That sounds good. I'm testing the private method instead of testing
Manager.run() because the latter required a lot more boilerplate and setup.

+    def test_ensure_manifest_copies_new_manifest(self):
+        host = MockHost()
+        port = host.port_factory.get('test-mac-mac10.10')

[qyearsley, 2017/02/28 18:54:45]

If the test is not asserting anything that's port-specific, you can also do
`port = host.port_factory.get()`, which for MockHost I think should return a
TestPort of some kind by default.

[jeffcarp, 2017/02/28 21:57:39]

Done

+
+        self.assertFalse(port.host.filesystem.exists('/mock-checkout/third_party/WebKit/LayoutTests/external/wpt/MANIFEST.json'))
+        manager = Manager(port, options=optparse.Values({'max_locked_shards': 1}), printer=FakePrinter())
+        manager._ensure_manifest()  # pylint: disable=protected-access
+        self.assertTrue(port.host.filesystem.exists('/mock-checkout/third_party/WebKit/LayoutTests/external/wpt/MANIFEST.json'))
+        self.assertEqual(port.host.executive.calls, [

[foolip, 2017/02/28 04:49:17]

I don't understand this assert, leaving it to qyearsley@ to understand :)

[qyearsley, 2017/02/28 18:54:45]

This is asserting that one subprocess was invoked, which is the manifest
generator script; this seems like what we want :-)

+            ['python',
+             '/mock-checkout/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/manifest',
+             '--work', '--tests-root', '/mock-checkout/third_party/WebKit/LayoutTests/external/wpt']])

[qyearsley, 2017/02/28 18:54:45]

Nit: Optionally, you could also format this as something like:

self.assertEqual(
    port.host.executive.calls,
    [
        [
            'python',
            webkit_base + '/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/manifest',
            '--work',
            '--tests-root',
            webkit_base + '/LayoutTests/external/wpt',
        ]
    ])

[jeffcarp, 2017/02/28 21:57:39]

Done

+
+    def test_ensure_manifest_updates_manifest_if_it_exists(self):
+        host = MockHost()
+        port = host.port_factory.get('test-mac-mac10.10')
+        manifest_path = '/mock-checkout/third_party/WebKit/LayoutTests/external/wpt/MANIFEST.json'
+
+        host.filesystem.write_binary_file(manifest_path, '{}')
+        self.assertTrue(port.host.filesystem.exists(manifest_path))
+
+        manager = Manager(port, options=optparse.Values({'max_locked_shards': 1}), printer=FakePrinter())
+        manager._ensure_manifest()  # pylint: disable=protected-access
+        self.assertTrue(port.host.filesystem.exists(manifest_path))
+        self.assertEqual(port.host.executive.calls, [
+            ['python',
+             '/mock-checkout/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/manifest',
+             '--work', '--tests-root', '/mock-checkout/third_party/WebKit/LayoutTests/external/wpt']])