Simplify and add comments to copy-existing-baselines-internal test.

third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py

Index: third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py b/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py
index abe3f60e7802c8096790e1b5dd92547b3b9090b6..e9bb1165d275142ebe096f0c68a4c43aa0aecc05 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py
@@ -109,147 +109,137 @@ class TestCopyExistingBaselinesInternal(BaseTestCase):
     def setUp(self):
         super(TestCopyExistingBaselinesInternal, self).setUp()
 
-    def test_copying_overwritten_baseline(self):
-        self.tool.executive = MockExecutive2()
+    def options(self, **kwargs):
+        options_dict = {
+            'results_directory': None,
+            'suffixes': 'txt',
+            'verbose': False,
+        }
+        options_dict.update(kwargs)
+        return optparse.Values(options_dict)
 
-        port = self.tool.port_factory.get('test-mac-mac10.10')
+    def test_copy_baseline_mac(self):
+        port = self.tool.port_factory.get('test-mac-mac10.11')
         self._write(
             port._filesystem.join(
                 port.layout_tests_dir(),
-                'platform/test-mac-mac10.10/failures/expected/image-expected.txt'),
+                'platform/test-mac-mac10.11/failures/expected/image-expected.txt'),
             'original mac10.11 result')
+        self.assertFalse(self.tool.filesystem.exists(
+            self.tool.filesystem.join(
+                port.layout_tests_dir(),
+                'platform/test-mac-mac10.10/failures/expected/image-expected.txt')))
 
         oc = OutputCapture()
-        try:
-            options = optparse.Values({
-                'builder': "MOCK Mac10.11",
-                'suffixes': 'txt',
-                'verbose': True,
-                'test': "failures/expected/image.html",
-                'results_directory': None
-            })
-            oc.capture_output()
-            self.command.execute(options, [], self.tool)
-        finally:
-            out, _, _ = oc.restore_output()
-
-        self.assertMultiLineEqual(
+        oc.capture_output()
+        self.command.execute(self.options(builder='MOCK Mac10.11', test='failures/expected/image.html'), [], self.tool)
+        out, _, _ = oc.restore_output()
+
+        # The Mac 10.11 baseline is copied over to the Mac 10.10 directory,
+        # because Mac10.10 is the "immediate predecessor" in the fallback tree.
+        # That means that normally for Mac10.10 if there's no Mac10.10-specific
+        # baseline, then we fall back to the Mac10.11 baseline.
+        # The idea is, if in the next step wae download new baselines for Mac10.11

[wkorman, 2016/10/06 21:22:29]

wae -> we

[qyearsley, 2016/10/06 22:00:29]

Done

+        # but not Mac10.10, then mac10.10 will still have the correct baseline.
+        self.assertEqual(
+            self._read(self.tool.filesystem.join(
+                port.layout_tests_dir(),
+                'platform/test-mac-mac10.11/failures/expected/image-expected.txt')),
+            'original mac10.11 result')
+        self.assertEqual(
             self._read(self.tool.filesystem.join(
                 port.layout_tests_dir(),
                 'platform/test-mac-mac10.10/failures/expected/image-expected.txt')),
             'original mac10.11 result')
-        self.assertMultiLineEqual(out, '{"add": [], "remove-lines": [], "delete": []}\n')
-
-    def test_copying_overwritten_baseline_to_multiple_locations(self):
-        self.tool.executive = MockExecutive2()
+        self.assertEqual(out, '{"add": [], "remove-lines": [], "delete": []}\n')
 
+    def test_copy_baseline_win7_to_linux_trusty(self):
         port = self.tool.port_factory.get('test-win-win7')
         self._write(
-            port._filesystem.join(port.layout_tests_dir(), 'platform/test-win-win7/failures/expected/image-expected.txt'),
+            self.tool.filesystem.join(
+                port.layout_tests_dir(),
+                'platform/test-win-win7/failures/expected/image-expected.txt'),
             'original win7 result')
+        self.assertFalse(self.tool.filesystem.exists(
+            self.tool.filesystem.join(
+                port.layout_tests_dir(),
+                'platform/test-linux-trusty/failures/expected/image-expected.txt')))
 
         oc = OutputCapture()
-        try:
-            options = optparse.Values({
-                'builder': "MOCK Win7",
-                'suffixes': "txt",
-                'verbose': True,
-                'test': "failures/expected/image.html",
-                'results_directory': None
-            })
-            oc.capture_output()
-            self.command.execute(options, [], self.tool)
-        finally:
-            out, _, _ = oc.restore_output()
+        oc.capture_output()
+        self.command.execute(self.options(builder='MOCK Win7', test='failures/expected/image.html'), [], self.tool)
+        out, _, _ = oc.restore_output()
 
-        self.assertMultiLineEqual(
+        # The Mac Win7 baseline is copied over to the Linux Trusty directory,
+        # because Linux Trusty is the baseline fallback "immediate predecessor" of Win7.
+        self.assertEqual(
+            self._read(self.tool.filesystem.join(
+                port.layout_tests_dir(),
+                'platform/test-win-win7/failures/expected/image-expected.txt')),
+            'original win7 result')
+        self.assertEqual(
             self._read(self.tool.filesystem.join(
                 port.layout_tests_dir(),
                 'platform/test-linux-trusty/failures/expected/image-expected.txt')),
             'original win7 result')
-        self.assertFalse(self.tool.filesystem.exists(self.tool.filesystem.join(
-            port.layout_tests_dir(), 'platform/test-linux-precise/userscripts/another-test-expected.txt')))
-        self.assertFalse(self.tool.filesystem.exists(self.tool.filesystem.join(
-            port.layout_tests_dir(), 'platform/test-mac-mac10.10/userscripts/another-test-expected.txt')))
-        self.assertMultiLineEqual(out, '{"add": [], "remove-lines": [], "delete": []}\n')
 
-    def test_no_copy_existing_baseline(self):
-        self.tool.executive = MockExecutive2()
+        self.assertEqual(out, '{"add": [], "remove-lines": [], "delete": []}\n')
 
+    def test_no_copy_existing_baseline(self):
         port = self.tool.port_factory.get('test-win-win7')
-        self._write(port._filesystem.join(port.layout_tests_dir(),
-                                          'platform/test-win-win7/failures/expected/image-expected.txt'), 'original win7 result')
+        self._write(
+            self.tool.filesystem.join(
+                port.layout_tests_dir(),
+                'platform/test-win-win7/failures/expected/image-expected.txt'),
+            'original win7 result')
+        self._write(
+            self.tool.filesystem.join(
+                port.layout_tests_dir(),
+                'platform/test-linux-trusty/failures/expected/image-expected.txt'),
+            'original linux trusty result')
 
         oc = OutputCapture()
-        try:
-            options = optparse.Values({
-                'builder': "MOCK Win7",
-                'suffixes': "txt",
-                'verbose': True,
-                'test': "failures/expected/image.html",
-                'results_directory': None
-            })
-            oc.capture_output()
-            self.command.execute(options, [], self.tool)
-        finally:

[wkorman, 2016/10/06 21:22:29]

Just to make sure I understand the change, are we removing try/finally because
they shouldn't fail in general and if they do we'd prefer the test fail and
inform us re: what happened?

[qyearsley, 2016/10/06 22:00:29]

Right -- as far as I understand, if any sort of exception is raised here we want
to fail the test, which is what I think should happen.

I think that the try/finally blocks might be here just because that was used
elsewhere in unit tests that use OutputCapture to assert things about the output
(or hide/ignore the output), but here it's unnecessary.

Actually, in this case, I think that asserting things about the output may be
beside the point of the test (and after http://crrev.com/2397573002, this
shouldn't be outputting SCM changes anyway), and hiding the output is
unnecessary because that's done by the test runner.

So, in the latest patchset, I've removed use of OutputCapture from this test
case; does that seem OK?

[wkorman, 2016/10/06 22:29:28]

Yes, thanks.

-            out, _, _ = oc.restore_output()
+        oc.capture_output()
+        self.command.execute(self.options(builder='MOCK Win7', test='failures/expected/image.html'), [], self.tool)
+        out, _, _ = oc.restore_output()
 
-        self.assertMultiLineEqual(
+        # Since a baseline existed already for Linux Trusty, the Win7 baseline is not copied over.
+        self.assertEqual(
             self._read(self.tool.filesystem.join(
                 port.layout_tests_dir(),
-                'platform/test-linux-trusty/failures/expected/image-expected.txt')),
+                'platform/test-win-win7/failures/expected/image-expected.txt')),
             'original win7 result')
-        self.assertMultiLineEqual(
+        self.assertEqual(
             self._read(self.tool.filesystem.join(
                 port.layout_tests_dir(),
-                'platform/test-win-win7/failures/expected/image-expected.txt')),
-            'original win7 result')
-        self.assertFalse(self.tool.filesystem.exists(self.tool.filesystem.join(
-            port.layout_tests_dir(), 'platform/test-mac-mac10.10/userscripts/another-test-expected.txt')))
-        self.assertMultiLineEqual(out, '{"add": [], "remove-lines": [], "delete": []}\n')
+                'platform/test-linux-trusty/failures/expected/image-expected.txt')),
+            'original linux trusty result')
+
+        self.assertEqual(out, '{"add": [], "remove-lines": [], "delete": []}\n')
 
     def test_no_copy_skipped_test(self):
-        self.tool.executive = MockExecutive2()
         port = self.tool.port_factory.get('test-win-win7')
-        fs = self.tool.filesystem
         self._write(
-            fs.join(
+            self.tool.filesystem.join(
                 port.layout_tests_dir(),
                 'platform/test-win-win7/failures/expected/image-expected.txt'),
             'original win7 result')
-        expectations_path = fs.join(port.path_to_generic_test_expectations_file())
         self._write(
-            expectations_path,
+            port.path_to_generic_test_expectations_file(),
             ("[ Win ] failures/expected/image.html [ Failure ]\n"
              "[ Linux ] failures/expected/image.html [ Skip ]\n"))
+
         oc = OutputCapture()
-        try:
-            options = optparse.Values({
-                'builder': "MOCK Win7",
-                'suffixes': "txt", 'verbose': True,
-                'test': "failures/expected/image.html",
-                'results_directory': None
-            })
-            oc.capture_output()
-            self.command.execute(options, [], self.tool)
-        finally:
-            oc.restore_output()
+        oc.capture_output()
+        self.command.execute(self.options(builder='MOCK Win7', test='failures/expected/image.html'), [], self.tool)
+        oc.restore_output()
 
+        # The Win7 baseline is not copied over to the Linux Trusty directory
+        # because the test is skipped on linux.
         self.assertFalse(
-            fs.exists(fs.join(
-                port.layout_tests_dir(),
-                'platform/test-mac-mac10.10/failures/expected/image-expected.txt')))
-        self.assertFalse(
-            fs.exists(fs.join(
+            self.tool.filesystem.exists(self.tool.filesystem.join(
                 port.layout_tests_dir(),
                 'platform/test-linux-trusty/failures/expected/image-expected.txt')))
-        self.assertFalse(
-            fs.exists(fs.join(
-                port.layout_tests_dir(),
-                'platform/test-linux-precise/failures/expected/image-expected.txt')))
-        self.assertEqual(
-            self._read(fs.join(port.layout_tests_dir(), 'platform/test-win-win7/failures/expected/image-expected.txt')),
-            'original win7 result')
 
 
 class TestRebaselineTest(BaseTestCase):