New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 779021 link

Starred by 1 user
Status: Fixed
Owner:
█ raphael....@intel.com
OoO until Feb 4th
Closed: Oct 2017
Cc:
qyears...@chromium.org
robertma@chromium.org
foolip@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

[WPT import] Renamed tests in NeverFixTests break the import process

Project Member Reported by raphael....@intel.com, Oct 27 2017 
See for example https://luci-milo.appspot.com/buildbot/chromium.infra.cron/wpt-importer/3948

This is the problematic bit (i.e. the tests were moved to a different directory upstream, but have not had their contents changed):

diff --git a/third_party/WebKit/LayoutTests/NeverFixTests b/third_party/WebKit/LayoutTests/NeverFixTests
index f3b679cfa3ff..cebc55492f12 100644
--- a/third_party/WebKit/LayoutTests/NeverFixTests
+++ b/third_party/WebKit/LayoutTests/NeverFixTests
@@ -985,16 +985,16 @@ external/wpt/vibration/pattern-array-manual.html [ WontFix ]
 external/wpt/vibration/pattern-array-with-0-manual.html [ WontFix ]
 external/wpt/vibration/simple-array-manual.html [ WontFix ]
 external/wpt/vibration/simple-scalar-manual.html [ WontFix ]
-external/wpt/viewport/viewport-attribute-event-handlers-manual.html [ WontFix ]
-external/wpt/viewport/viewport-dimensions-custom-scrollbars-manual.html [ WontFix ]
-external/wpt/viewport/viewport-dimensions-scrollbars-manual.html [ WontFix ]
-external/wpt/viewport/viewport-offset-manual.html [ WontFix ]
-external/wpt/viewport/viewport-page-manual.html [ WontFix ]
-external/wpt/viewport/viewport-resize-event-manual.html [ WontFix ]
-external/wpt/viewport/viewport-scale-iframe-manual.html [ WontFix ]
-external/wpt/viewport/viewport-scale-manual.html [ WontFix ]
-external/wpt/viewport/viewport-scroll-event-manual.html [ WontFix ]
-external/wpt/viewport/viewport-url-bar-changes-height-manual.html [ WontFix ]
+external/wpt/visual-viewport/viewport-attribute-event-handlers-manual.html [ Skip WontFix ]
+external/wpt/visual-viewport/viewport-dimensions-custom-scrollbars-manual.html [ Skip WontFix ]
+external/wpt/visual-viewport/viewport-dimensions-scrollbars-manual.html [ Skip WontFix ]
+external/wpt/visual-viewport/viewport-offset-manual.html [ Skip WontFix ]
+external/wpt/visual-viewport/viewport-page-manual.html [ Skip WontFix ]
+external/wpt/visual-viewport/viewport-resize-event-manual.html [ Skip WontFix ]
+external/wpt/visual-viewport/viewport-scale-iframe-manual.html [ Skip WontFix ]
+external/wpt/visual-viewport/viewport-scale-manual.html [ Skip WontFix ]
+external/wpt/visual-viewport/viewport-scroll-event-manual.html [ Skip WontFix ]
+external/wpt/visual-viewport/viewport-url-bar-changes-height-manual.html [ Skip WontFix ]
 external/wpt/web-share/share-cancel-manual.html [ WontFix ]
 external/wpt/web-share/share-extra-argument-manual.html [ WontFix ]
 external/wpt/web-share/share-extra-field-manual.html [ WontFix ]

And these are the lint errors we're getting:

third_party/WebKit/LayoutTests/NeverFixTests:988 Only WONTFIX expectations are allowed in NeverFixTests external/wpt/visual-viewport/viewport-attribute-event-handlers-manual.html
third_party/WebKit/LayoutTests/NeverFixTests:989 Only WONTFIX expectations are allowed in NeverFixTests external/wpt/visual-viewport/viewport-dimensions-custom-scrollbars-manual.html
third_party/WebKit/LayoutTests/NeverFixTests:990 Only WONTFIX expectations are allowed in NeverFixTests external/wpt/visual-viewport/viewport-dimensions-scrollbars-manual.html
third_party/WebKit/LayoutTests/NeverFixTests:991 Only WONTFIX expectations are allowed in NeverFixTests external/wpt/visual-viewport/viewport-offset-manual.html
third_party/WebKit/LayoutTests/NeverFixTests:992 Only WONTFIX expectations are allowed in NeverFixTests external/wpt/visual-viewport/viewport-page-manual.html
third_party/WebKit/LayoutTests/NeverFixTests:993 Only WONTFIX expectations are allowed in NeverFixTests external/wpt/visual-viewport/viewport-resize-event-manual.html
third_party/WebKit/LayoutTests/NeverFixTests:994 Only WONTFIX expectations are allowed in NeverFixTests external/wpt/visual-viewport/viewport-scale-iframe-manual.html
third_party/WebKit/LayoutTests/NeverFixTests:995 Only WONTFIX expectations are allowed in NeverFixTests external/wpt/visual-viewport/viewport-scale-manual.html
third_party/WebKit/LayoutTests/NeverFixTests:996 Only WONTFIX expectations are allowed in NeverFixTests external/wpt/visual-viewport/viewport-scroll-event-manual.html
third_party/WebKit/LayoutTests/NeverFixTests:997 Only WONTFIX expectations are allowed in NeverFixTests external/wpt/visual-viewport/viewport-url-bar-changes-height-manual.html

The logic for this is in //third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py's tokenize_line():

        if 'WONTFIX' in expectations and 'SKIP' not in expectations:
            expectations.append('SKIP')

which gets called by test_importer. When NeverFixTests is updated, the expectations get sorted and we end up with [ Skip WontFix ], which later fails linting because of this other bit in tokenize_line():

        if 'NeverFixTests' in filename and expectations != ['WONTFIX', 'SKIP']:
            warnings.append('Only WONTFIX expectations are allowed in NeverFixTests')

Since we don't have any other occurrences of [ Skip WontFix ] in any of the *Tests files in LayoutTests, I'm considering removing the part that appends 'Skip' altogether (that code's from 2013).
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 27 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28d5025d5c4721dd041692241c07ea546423140a

commit 28d5025d5c4721dd041692241c07ea546423140a
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Fri Oct 27 12:10:38 2017

Simplify [ Skip WontFix ] to [ WontFix ] when updating the *Tests files

In the WPT import process, we can run into upstream changes that cause us to
update NeverFixTests. For instance, in the bug referenced below, a directory
was renamed upstream, so wpt-import had to update the corresponding entries
in NeverFixTests.

test_expectations.py' tokenize_line() appends the 'Skip' expectation to any
lines containing a 'WontFix' one, which makes sense when parsing the *Tests
files to run tests. However, the WPT import process also uses the result of
tokenize_line() to update the same *Tests files, which led it to turn an
entry such as

  external/wpt/viewport/viewport-attribute-event-handlers-manual.html [ WontFix ]

into

  external/wpt/visual-viewport/viewport-attribute-event-handlers-manual.html [ Skip WontFix ]

which (fortunately) later fails git-cl presubmit, as linting also calls
tokenize_line(), which complains when we have "Skip WontFix" instead of
"WontFix Skip" in the expectations.

Since we do not want to add "Skip" to the *Tests files anyway, we can just
filter it out when updating the *Tests files.

Bug:  779021 
Change-Id: Ibd7d8795012729828363a25b4b1145b075257d24
Reviewed-on: https://chromium-review.googlesource.com/741589
Commit-Queue: Raphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com>
Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512154}
[modify] https://crrev.com/28d5025d5c4721dd041692241c07ea546423140a/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
[modify] https://crrev.com/28d5025d5c4721dd041692241c07ea546423140a/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py

Comment 2 by raphael....@intel.com, Oct 27 2017

Status: Fixed (was: Started)
►

Sign in to add a comment