Regenerate MANIFEST.json when WPT tests are run

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

 # Copyright (C) 2010 Google Inc. All rights reserved.
 # Copyright (C) 2010 Gabor Rapcsanyi (rgabor@inf.u-szeged.hu), University of Szeged
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
@@ skipping 160 matching lines @@
             dir_name = '/tmp/layout-test-results' + '_' + str(x)
             self._make_fake_test_result(port.host, dir_name)
         manager = get_manager()
         manager._limit_archived_results_count()
         deleted_dir_count = 0
         for x in range(1, 31):
             dir_name = '/tmp/layout-test-results' + '_' + str(x)
             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']])