Changeset 129941 in webkit

Timestamp:

Sep 28, 2012, 1:28:00 PM (13 years ago)

Author:

dpranke@chromium.org

Message:

webkit-patch rebaseline-expectations is broken
​https://bugs.webkit.org/show_bug.cgi?id=97619

Reviewed by Ojan Vafai.

The new expectations format is causing two problems for
webkit-patch rebaseline expectations

1) the transformation into and out of the old syntax (which is
still used internally) is somewhat lossy, e.g., we're not
preserving the case of Bug(x) identifiers. Also, we can't
tell if the input was [ WontFix ] or [ Skip WontFix Pass ]
2) the new syntax is more lenient, allowing for multiple ways to
specify the same result, e.g., "[ Skip ]" may or may not be
missing.

As a result, we end up rewriting a whole bunch of lines for
purely syntactic differences.

Both of these things are bugs that need to be fixed, but
we should also not just rewrite lines that shouldn't be
modified.

This patch fixes the last issue (no rewriting).

• Scripts/webkitpy/layout_tests/models/test_expectations.py:

(TestExpectationLine._format_line):
(TestExpectations.remove_rebaselined_tests):

• Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:

(RebaseliningTest.test_remove):

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/layout_tests/models/test_expectations.py (modified) (1 diff)
• Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2012-09-28  Dirk Pranke  <dpranke@chromium.org>
+
+        webkit-patch rebaseline-expectations is broken
+        https://bugs.webkit.org/show_bug.cgi?id=97619
+
+        Reviewed by Ojan Vafai.
+
+        The new expectations format is causing two problems for
+        webkit-patch rebaseline expectations
+
+        1) the transformation into and out of the old syntax (which is
+        still used internally) is somewhat lossy, e.g., we're not
+        preserving the case of Bug(x) identifiers. Also, we can't
+        tell if the input was [ WontFix ] or [ Skip WontFix Pass ]
+        2) the new syntax is more lenient, allowing for multiple ways to
+        specify the same result, e.g., "[ Skip ]" may or may not be
+        missing.
+
+        As a result, we end up rewriting a whole bunch of lines for
+        purely syntactic differences.
+
+        Both of these things are bugs that need to be fixed, but
+        we should also not just rewrite lines that shouldn't be
+        modified.
+
+        This patch fixes the last issue (no rewriting).
+
+        * Scripts/webkitpy/layout_tests/models/test_expectations.py:
+        (TestExpectationLine._format_line):
+        (TestExpectations.remove_rebaselined_tests):
+        * Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
+        (RebaseliningTest.test_remove):
+
 2012-09-28  Dirk Pranke  <dpranke@chromium.org>
 

trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py

@@ -971 +971 @@
                          'rebaseline' in expectation.parsed_modifiers))
 
-        return self.list_to_string(filter(without_rebaseline_modifier, self._expectations))
+        return self.list_to_string(filter(without_rebaseline_modifier, self._expectations), reconstitute_only_these=[])
 
     def _add_expectations(self, expectation_list):

trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py

@@ -514 +514 @@
                           'Bug(x0) failures/expected/image.html [ Crash ]\n')
 
+        # Ensure that we don't modify unrelated lines, even if we could rewrite them.
+        # i.e., the second line doesn't get rewritten to "Bug(y) failures/expected/skip.html"
+        self.assertRemove('Bug(x) failures/expected/text.html [ Failure Rebaseline ]\n'
+                          'Bug(Y) failures/expected/image.html [ Skip   ]\n'
+                          'Bug(z) failures/expected/crash.html\n',
+                          '',
+                          ['failures/expected/text.html'],
+                          'Bug(Y) failures/expected/image.html [ Skip   ]\n'
+                          'Bug(z) failures/expected/crash.html\n',
+                          '')
+
+
 
     def test_no_get_rebaselining_failures(self):