Bug 165249 - [Win] test-webkitpy is failing.
Summary: [Win] test-webkitpy is failing.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-01 03:50 PST by Per Arne Vollan
Modified: 2016-12-02 19:29 PST (History)
8 users (show)

See Also:


Attachments
Patch (1.28 KB, patch)
2016-12-01 03:57 PST, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (1.33 KB, patch)
2016-12-02 04:45 PST, Per Arne Vollan
dbates: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2016-12-01 03:50:54 PST
When running test-webkitpy, I get an exception:

[1444/1505] webkitpy.tool.commands.rebaseline_unittest.TestRebaselineTest.test_rebaseline_updates_expectations_file_noop erred:
  Traceback (most recent call last):
    File "/cygdrive/c/Projects/WebKit2/OpenSource/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py", line 101, in test_rebaseline_updates_expectations_file_noop
      self._zero_out_test_expectations()
    File "/cygdrive/c/Projects/WebKit2/OpenSource/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py", line 70, in _zero_out_test_expectations
      port = self.tool.port_factory.get(port_name)
    File "/cygdrive/c/Projects/WebKit2/OpenSource/Tools/Scripts/webkitpy/port/factory.py", line 126, in get
      return cls(self._host, port_name, options=options, **kwargs)
    File "/cygdrive/c/Projects/WebKit2/OpenSource/Tools/Scripts/webkitpy/port/ios.py", line 106, in __init__
      self._current_device = Simulator().current_device()
    File "/cygdrive/c/Projects/WebKit2/OpenSource/Tools/Scripts/webkitpy/xcode/simulator.py", line 303, in __init__
      self.refresh()
    File "/cygdrive/c/Projects/WebKit2/OpenSource/Tools/Scripts/webkitpy/xcode/simulator.py", line 403, in refresh
      device_types_header = next(lines)
  TypeError: tuple object is not an iterator
Comment 1 Per Arne Vollan 2016-12-01 03:57:21 PST
Created attachment 295833 [details]
Patch
Comment 2 Jonathan Bedard 2016-12-01 08:28:40 PST
Comment on attachment 295833 [details]
Patch

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

> Tools/Scripts/webkitpy/xcode/simulator.py:404
> +            return

I think we want another assert in here to test that we are not on a mac.  Otherwise we could end up suppressing a legitimate error on Macs.  Perhaps self._host.platform.is_mac() would do the job?
Comment 3 Per Arne Vollan 2016-12-02 04:45:54 PST
Created attachment 295941 [details]
Patch
Comment 4 Per Arne Vollan 2016-12-02 04:47:40 PST
(In reply to comment #2)
> Comment on attachment 295833 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295833&action=review
> 
> > Tools/Scripts/webkitpy/xcode/simulator.py:404
> > +            return
> 
> I think we want another assert in here to test that we are not on a mac. 
> Otherwise we could end up suppressing a legitimate error on Macs.  Perhaps
> self._host.platform.is_mac() would do the job?

Thanks for reviewing :) Updated patch.
Comment 5 Alex Christensen 2016-12-02 09:29:50 PST
Comment on attachment 295941 [details]
Patch

what about linux?  Why are we even running this Xcode-specific code on Windows?
Comment 6 Jonathan Bedard 2016-12-02 10:55:17 PST
(In reply to comment #5)
> Comment on attachment 295941 [details]
> Patch
> 
> what about linux?  Why are we even running this Xcode-specific code on
> Windows?

I'm not aware of a method of disabling certain unit tests on different platforms in Python (since that is essentially what it going on here).  This change may be better suited for the unit test itself.

It should also be noted that this is a pretty unique circumstance.  Most of the time when we run command line tools in webkitpy we mock them up when testing, meaning that testing platform specific code is platform agnostic.  In this case, the test in question is intended to test the command, so the test can't really be platform agnostic.
Comment 7 Jonathan Bedard 2016-12-02 15:15:59 PST
I think <http://trac.webkit.org/changeset/209266> should have fixed this.

https://bugs.webkit.org/show_bug.cgi?id=164568 should be fixed shortly, the more verbose GTK log revealed a better fix to this problem.
Comment 8 Daniel Bates 2016-12-02 16:32:40 PST
Comment on attachment 295941 [details]
Patch

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

I like the approach of this patch. We already rolled out <https://trac.webkit.org/changeset/209136> in <https://trac.webkit.org/changeset/209266>.

> Tools/Scripts/webkitpy/xcode/simulator.py:404
> +            assert not self._host.platform.is_mac()

This assert is not needed because on a real platform xcode_simctl_list() returns a generator. And an empty generator != None. (For correctness we should fix <http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/system/platforminfo.py?rev=209145#L132> to return an empty generator for a non-Mac port. That being said, this should never happen in practice).