Bug 87758 – [chromium] run_webkit_tests --lint-test-files complains about skia test_expectations

• Component: Tools / Tests
• Status: RESOLVED INVALID

[Rafael Weinstein]

override file doesn't exist -- and it don't like that.

[Elliot Poger]

Assigning to myself, since I added skia_test_expectations...

"run_webkit_tests --lint-test-files" is not in my list of things that I test before committing webkit changes.  Is there a list somewhere of tests, like this one, that I should run before committing?

Here's my current list:
 perl ./Tools/Scripts/run-chromium-webkit-unit-tests --release
 Tools/Scripts/new-run-webkit-tests --chromium --lint
 python ./Tools/Scripts/test-webkitpy

[Dirk Pranke]

I wouldn't worry about running the --lint-test-files step normally; it is run if you actually modify the test_expectations.txt file as part of the style checkers, and otherwise test-webkitpy normally catches any problems that would arise.

[Dirk Pranke]

I'm also not real sure why we're still getting this error; have we not rolled the chromium DEPS in Source/WebKit/chromium to a version that pulls the skia file?

[Elliot Poger]

Here's the command I am running to attempt to replicate this bug:
python Tools/Scripts/new-run-webkit-tests --lint-test-files

I updated my webkit checkout this morning, and when I run that command (with Source/WebKit/chromium/skia/skia_test_expectations.txt in place) I get the following output:



LayoutTests/platform/mac/test_expectations.txt:161 fast/inline/continuation-outlines-with-layers.html is also in a Skipped file.
LayoutTests/platform/mac/test_expectations.txt:170 tables/mozilla_expected_failures/collapsing_borders/bug41262-5.html is also in a Skipped file.
LayoutTests/platform/mac/test_expectations.txt:219 ietestcenter/css3/valuesandunits/units-000.htm is also in a Skipped file.


LayoutTests/platform/qt/test_expectations.txt:20 inspector/styles/override-screen-size.html is also in a Skipped file.
LayoutTests/platform/qt/test_expectations.txt:62 ietestcenter/css3/valuesandunits/units-000.htm is also in a Skipped file.
LayoutTests/platform/qt/test_expectations.txt:74 fast/borders/border-antialiasing.html is also in a Skipped file.
LayoutTests/platform/qt/test_expectations.txt:81 editing/inserting/typing-space-to-trigger-smart-link.html is also in a Skipped file.

Lint failed.

[Elliot Poger]

If I delete Source/WebKit/chromium/skia/skia_test_expectations.txt and then run the same command again, I get the same output but with the following line added at the top:

skia test_expectations overrides file '/usr/local/google/home/epoger/src/webkit/green/WebKit/Source/WebKit/chromium/skia/skia_test_expectations.txt' does not exist


That's not actually an error, it's just the warning logged by line 331 of http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py?rev=117789 :

323	    def _expectations_file_contents(self, filetype, filepath):
324	        if self._filesystem.exists(filepath):
325	            _log.debug(
326	                "reading %s test_expectations overrides from file '%s'" %
327	                (filetype, filepath))
328	            return (self._filesystem.read_text_file(filepath) or '')
329	        else:
330	            _log.warning(
331	                "%s test_expectations overrides file '%s' does not exist" %
332	                (filetype, filepath))
333	            return ''

[Elliot Poger]

For the reasons given above, I think this is already "working as intended".

Once the other --lint-test-files failures are resolved, NRWT should succeed, even though this warning is output.  (And I think it's a good warning to have, when people are running NRWT typically...)

[Ojan Vafai]

(In reply to comment #6)
> For the reasons given above, I think this is already "working as intended".
> 
> Once the other --lint-test-files failures are resolved, NRWT should succeed, even though this warning is output.  (And I think it's a good warning to have, when people are running NRWT typically...)

Just to make sure I understand, if we update chromium DEPS in webkit, then this error goes away, yes? If so, then I'm OK with leaving it as is.

[Dirk Pranke]

(In reply to comment #7)
> (In reply to comment #6)
> > For the reasons given above, I think this is already "working as intended".
> > 
> > Once the other --lint-test-files failures are resolved, NRWT should succeed, even though this warning is output.  (And I think it's a good warning to have, when people are running NRWT typically...)
> 
> Just to make sure I understand, if we update chromium DEPS in webkit, then this error goes away, yes? If so, then I'm OK with leaving it as is.

Yes, and it's fine at tip of tree now.