Fix a bug where optimize-baselines would incorrectly fail to optimize
authortony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Jul 2012 00:26:25 +0000 (00:26 +0000)
committertony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Jul 2012 00:26:25 +0000 (00:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=91551

Reviewed by Adam Barth.

In some cases, optimize-baseline would correctly optimize the results, but because
we weren't filtering the virtual ports out of _results_by_port_name, we thought
that we had failed to optimize.

* Scripts/webkitpy/common/checkout/baselineoptimizer.py:
(BaselineOptimizer._find_optimal_result_placement): No virtual filtering here.
(BaselineOptimizer._filtered_results_by_port_name): New function that filters out virtual directories.
(BaselineOptimizer.optimize): Filter out virtual ports.
* Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py:
(TestBaselineOptimizer._move_baselines): Add a stub so we don't actual move results.
(BaselineOptimizerTest._assertOptimization): Call optimize and verify that the right files were moved.
(BaselineOptimizerTest._assertOptimizationFailed): Add a method for when optimization should fail.
(BaselineOptimizerTest.test_common_directory_includes_root): Update since this test should fail.
(BaselineOptimizerTest.test_virtual_ports_filtered): New test case that demonstrates the bug.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@122902 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py
Tools/Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py

index 6c35d42..191414a 100644 (file)
@@ -1,3 +1,25 @@
+2012-07-17  Tony Chang  <tony@chromium.org>
+
+        Fix a bug where optimize-baselines would incorrectly fail to optimize
+        https://bugs.webkit.org/show_bug.cgi?id=91551
+
+        Reviewed by Adam Barth.
+
+        In some cases, optimize-baseline would correctly optimize the results, but because
+        we weren't filtering the virtual ports out of _results_by_port_name, we thought
+        that we had failed to optimize.
+
+        * Scripts/webkitpy/common/checkout/baselineoptimizer.py:
+        (BaselineOptimizer._find_optimal_result_placement): No virtual filtering here.
+        (BaselineOptimizer._filtered_results_by_port_name): New function that filters out virtual directories.
+        (BaselineOptimizer.optimize): Filter out virtual ports.
+        * Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py:
+        (TestBaselineOptimizer._move_baselines): Add a stub so we don't actual move results.
+        (BaselineOptimizerTest._assertOptimization): Call optimize and verify that the right files were moved.
+        (BaselineOptimizerTest._assertOptimizationFailed): Add a method for when optimization should fail.
+        (BaselineOptimizerTest.test_common_directory_includes_root): Update since this test should fail.
+        (BaselineOptimizerTest.test_virtual_ports_filtered): New test case that demonstrates the bug.
+
 2012-07-17  Xianzhu Wang  <wangxianzhu@chromium.org>
 
         [Chromium] Add --encode-binary command line option for DRT
index d244045..f976716 100644 (file)
@@ -136,17 +136,14 @@ class BaselineOptimizer(object):
                 break  # Frowns. We do not appear to be converging.
             unsatisfied_port_names_by_result = new_unsatisfied_port_names_by_result
 
-        self._filter_virtual_ports(new_results_by_directory)
         return results_by_directory, new_results_by_directory
 
-    def _filter_virtual_ports(self, new_results_by_directory):
-        for port in _VIRTUAL_PORTS:
-            virtual_directory = _VIRTUAL_PORTS[port][0]
-            if virtual_directory in new_results_by_directory:
-                real_directory = _VIRTUAL_PORTS[port][1]
-                if real_directory not in new_results_by_directory:
-                    new_results_by_directory[real_directory] = new_results_by_directory[virtual_directory]
-                del new_results_by_directory[virtual_directory]
+    def _filtered_results_by_port_name(self, results_by_directory):
+        results_by_port_name = self._results_by_port_name(results_by_directory)
+        for port_name in _VIRTUAL_PORTS.keys():
+            if port_name in results_by_port_name:
+                del results_by_port_name[port_name]
+        return results_by_port_name
 
     def _move_baselines(self, baseline_name, results_by_directory, new_results_by_directory):
         data_for_result = {}
@@ -178,7 +175,7 @@ class BaselineOptimizer(object):
 
     def optimize(self, baseline_name):
         results_by_directory, new_results_by_directory = self._find_optimal_result_placement(baseline_name)
-        if self._results_by_port_name(results_by_directory) != self._results_by_port_name(new_results_by_directory):
+        if self._filtered_results_by_port_name(results_by_directory) != self._filtered_results_by_port_name(new_results_by_directory):
             return False
         self._move_baselines(baseline_name, results_by_directory, new_results_by_directory)
         return True
index 9ba6ff1..0325991 100644 (file)
@@ -45,12 +45,19 @@ class TestBaselineOptimizer(BaselineOptimizer):
     def _read_results_by_directory(self, baseline_name):
         return self._mock_results_by_directory
 
+    def _move_baselines(self, baseline_name, results_by_directory, new_results_by_directory):
+        self.new_results_by_directory = new_results_by_directory
+
 
 class BaselineOptimizerTest(unittest.TestCase):
     def _assertOptimization(self, results_by_directory, expected_new_results_by_directory):
         baseline_optimizer = TestBaselineOptimizer(results_by_directory)
-        _, new_results_by_directory = baseline_optimizer._find_optimal_result_placement('mock-baseline.png')
-        self.assertEqual(new_results_by_directory, expected_new_results_by_directory)
+        self.assertTrue(baseline_optimizer.optimize('mock-baseline.png'))
+        self.assertEqual(baseline_optimizer.new_results_by_directory, expected_new_results_by_directory)
+
+    def _assertOptimizationFailed(self, results_by_directory):
+        baseline_optimizer = TestBaselineOptimizer(results_by_directory)
+        self.assertFalse(baseline_optimizer.optimize('mock-baseline.png'))
 
     def test_move_baselines(self):
         host = MockHost()
@@ -135,18 +142,13 @@ class BaselineOptimizerTest(unittest.TestCase):
         })
 
     def test_common_directory_includes_root(self):
-        # Note: The resulting directories are "wrong" in the sense that
-        # enacting this plan would change semantics. However, this test case
-        # demonstrates that we don't throw an exception in this case. :)
-        self._assertOptimization({
+        # This test case checks that we don't throw an exception when we fail
+        # to optimize.
+        self._assertOptimizationFailed({
             'LayoutTests/platform/gtk': 'e8608763f6241ddacdd5c1ef1973ba27177d0846',
             'LayoutTests/platform/qt': 'bcbd457d545986b7abf1221655d722363079ac87',
             'LayoutTests/platform/chromium-win': '3764ac11e1f9fbadd87a90a2e40278319190a0d3',
             'LayoutTests/platform/mac': 'e8608763f6241ddacdd5c1ef1973ba27177d0846',
-        }, {
-            'LayoutTests/platform/qt': 'bcbd457d545986b7abf1221655d722363079ac87',
-            'LayoutTests/platform/chromium-win': '3764ac11e1f9fbadd87a90a2e40278319190a0d3',
-            'LayoutTests': 'e8608763f6241ddacdd5c1ef1973ba27177d0846',
         })
 
         self._assertOptimization({
@@ -181,3 +183,20 @@ class BaselineOptimizerTest(unittest.TestCase):
             'LayoutTests/platform/win-xp': '5b1253ef4d5094530d5f1bc6cdb95c90b446bec7',
             'LayoutTests/platform/chromium-linux': 'f52fcdde9e4be8bd5142171cd859230bd4471036'
         })
+
+    def test_virtual_ports_filtered(self):
+        self._assertOptimization({
+            'LayoutTests/platform/chromium-mac': '1',
+            'LayoutTests/platform/chromium-mac-snowleopard': '1',
+            'LayoutTests/platform/chromium-win': '2',
+            'LayoutTests/platform/gtk': '3',
+            'LayoutTests/platform/efl': '3',
+            'LayoutTests/platform/qt': '4',
+            'LayoutTests/platform/mac': '5',
+        }, {
+            'LayoutTests/platform/chromium-mac': '1',
+            'LayoutTests/platform/chromium-win': '2',
+            'LayoutTests': '3',
+            'LayoutTests/platform/qt': '4',
+            'LayoutTests/platform/mac': '5',
+        })