Bug 54652 – REGRESSION (r78522): webkitpy.layout_tests.port.chromium_win_unittest.ChromiumWinTest.test_setup_environ_for_server_register_cygwin failing on Apple's Windows port

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Adam Roben (:aroben)]

webkitpy.layout_tests.port.chromium_win_unittest.ChromiumWinTest.test_setup_environ_for_server_register_cygwin is failing on Apple's Windows port. See the URL for an example.

[Adam Roben (:aroben)]

Looks like this started failing in r78522. Here's the first test run where it failed: http://build.webkit.org/builders/Windows%20XP%20Debug%20%28Tests%29/builds/25257

[Adam Roben (:aroben)]

<rdar://problem/9016939>

[Dirk Pranke]

I have a patch for this somewhere. I'll try to get it landed this afternoon.

[Dirk Pranke]

Created attachment 82905 [details]
Patch

[Dirk Pranke]

Note that I'm kinda hijacking this bug. I believe the change in port_testcase fixes this issue, but the fixes to the baseline path are needed somewhere in here as well.

[Adam Roben (:aroben)]

Comment on attachment 82905 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=82905&action=review

> Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py:102
> +    def baseline_path(self):
> +        if self.version() == 'win7':
> +            return self._webkit_baseline_path('chromium-win')
> +        return self._webkit_baseline_path(self.name())

Why does win7 need a special case? Seems worth a comment.

> Tools/Scripts/webkitpy/layout_tests/port/chromium_win_unittest.py:44
> +            self.results_directory = '/'

What effect does this have? Can you explain in the ChangeLog?

[Dirk Pranke]

Created attachment 82908 [details]
Patch

[Dirk Pranke]

Committed r78943: <http://trac.webkit.org/changeset/78943>

[Dirk Pranke]

(In reply to comment #6)
> (From update of attachment 82905 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=82905&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py:102
> > +    def baseline_path(self):
> > +        if self.version() == 'win7':
> > +            return self._webkit_baseline_path('chromium-win')
> > +        return self._webkit_baseline_path(self.name())
> 
> Why does win7 need a special case? Seems worth a comment.
> 

Done (this is confusingly written in this version, but if version is win7, name is 'chromium-win-win7' and we don't want to use the version-specific directory, since win7 is the newest version.

This gets cleaned up in a later patch so I'm not going to sweat it here.

> > Tools/Scripts/webkitpy/layout_tests/port/chromium_win_unittest.py:44
> > +            self.results_directory = '/'
> 
> What effect does this have? Can you explain in the ChangeLog?

Turns out this didn't need to be set, but the code in port_testcase that was using it was doing something unnecessary that triggered it. I fixed that code instead.

[Ojan Vafai]

Comment on attachment 82908 [details]
Patch

Clearing review flag since this has been committed.

[Jessie Berlin]

This test is still failing:

http://build.webkit.org/builders/Windows%207%20Release%20%28Tests%29/builds/9507/steps/webkitpy-test/logs/stdio

[Dirk Pranke]

so it is ... maybe I only fixed it for win32, and not cygwin. Looking now ...

[Dirk Pranke]

Created attachment 83015 [details]
Patch

[Dirk Pranke]

Committed r79036: <http://trac.webkit.org/changeset/79036>