Bug 140787 - test-webkitpy: webkitpy.tool.commands.earlywarningsystem_unittest.EarlyWarningSystemTest.test_ewses fails on EFL, GTK, Win ports
Summary: test-webkitpy: webkitpy.tool.commands.earlywarningsystem_unittest.EarlyWarnin...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords:
Depends on: 140476 140637
Blocks:
  Show dependency treegraph
 
Reported: 2015-01-22 14:07 PST by David Kilzer (:ddkilzer)
Modified: 2015-01-23 19:51 PST (History)
10 users (show)

See Also:


Attachments
Patch v1 (speculative fix) (1.60 KB, patch)
2015-01-22 19:19 PST, David Kilzer (:ddkilzer)
dbates: review+
ddkilzer: commit-queue?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2015-01-22 14:07:31 PST
The webkitpy.tool.commands.earlywarningsystem_unittest.EarlyWarningSystemTest.test_ewses test still fails on EFL, GTK and Win ports:

[1231/1425] webkitpy.tool.commands.earlywarningsystem_unittest.EarlyWarningSystemTest.test_ewses
[1232/1425] webkitpy.tool.commands.earlywarningsystem_unittest.EarlyWarningSystemTest.test_ewses erred:
  Traceback (most recent call last):
    File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py", line 100, in test_ewses
      self._test_ews(ews_class())
    File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py", line 93, in _test_ews
      self.assert_queue_outputs(ews, expected_logs=self._default_expected_logs(ews), options=options)
    File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/tool/commands/queuestest.py", line 100, in assert_queue_outputs
      self.assert_outputs(queue.begin_work_queue, "begin_work_queue", [], expected_stdout, expected_stderr, expected_exceptions, expected_logs)
    File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/tool/commands/queuestest.py", line 66, in assert_outputs
      expected_logs=logs)
    File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/common/system/outputcapture.py", line 93, in assert_outputs
      return_value = function(*args, **kwargs)
    File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py", line 56, in begin_work_queue
      AbstractReviewQueue.begin_work_queue(self)
    File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/tool/commands/queues.py", line 435, in begin_work_queue
      PatchProcessingQueue.begin_work_queue(self)
    File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/tool/commands/queues.py", line 281, in begin_work_queue
      self._port = self._tool.port_factory.get(self._new_port_name_from_old(self.port_name, self._tool.platform))
    File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/port/factory.py", line 126, in get
      port_name = cls.determine_full_port_name(self._host, options, port_name)
    File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/port/ios.py", line 57, in determine_full_port_name
      sdk_command_output = subprocess.check_output(['/usr/bin/xcrun', '--sdk', 'iphoneos', '--show-sdk-version'], stderr=None).rstrip()
    File "/usr/lib/python2.7/subprocess.py", line 537, in check_output
      process = Popen(stdout=PIPE, *popenargs, **kwargs)
    File "/usr/lib/python2.7/subprocess.py", line 679, in __init__
      errread, errwrite)
    File "/usr/lib/python2.7/subprocess.py", line 1249, in _execute_child
      raise child_exception
  OSError: [Errno 2] No such file or directory

The fix is to make sure the "xcrun" command never runs on a port that doesn't have an iOS SDK installed.  (The check I added earlier for running on a Mac host was apparently insufficient.)
Comment 2 David Kilzer (:ddkilzer) 2015-01-22 19:19:10 PST
Created attachment 245194 [details]
Patch v1 (speculative fix)
Comment 3 Daniel Bates 2015-01-22 22:00:35 PST
Comment on attachment 245194 [details]
Patch v1 (speculative fix)

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

> Tools/Scripts/webkitpy/port/ios.py:59
> -            if host.platform.is_mac():
> +            if os.path.isfile('/usr/bin/xcrun'):
>                  sdk_command_output = subprocess.check_output(['/usr/bin/xcrun', '--sdk', 'iphoneos', '--show-sdk-version'], stderr=None).rstrip()
>                  if sdk_command_output:
>                      sdk_version = sdk_command_output

OK. You may want to consider adding a FIXME comment that indicates that we check for the presence of /usr/bin/xcrun as a workaround for the unit test webkitpy.tool.commands.earlywarningsystem_unittest.EarlyWarningSystemTest.test_ewses.

Maybe a better way to fix this issue is to extract the logic to query the iphoneos SDK version in IOSPort.determine_full_port_name() into a method in class PlatformInfo called iphoneSDKVersion() (or you could create a more generic function xcodeSDKVersion(sdk_name) or sdkVersion(sdk_name) that takes a string that represents that name of the SDK to lookup and returns the version of the SDK if it exists and an error value (None?) if the SDK doesn't exist). Similar to the implementation of PlatformInfo.total_bytes_memory(), PlatformInfo.iphoneSDKVersion() would call /usr/bin/xcrun --sdk iphoneos on Mac. Otherwise it would return some error value (None?). We would add an analogous method to class MockPlatformInfo that either returns a hardcoded version or None (or the value it returns could be specified by an argument passed to the constructor of MockPlatformInfo with a reasonable default value for this argument). Then we would implement IOSPort.determine_full_port_name() in terms of host.platform.iphoneSDKVersion().
Comment 4 David Kilzer (:ddkilzer) 2015-01-23 17:06:26 PST
Committed r179046: <http://trac.webkit.org/changeset/179046>
Comment 5 David Kilzer (:ddkilzer) 2015-01-23 19:51:17 PST
(In reply to comment #4)
> Committed r179046: <http://trac.webkit.org/changeset/179046>

BTW, I added a FIXME comment as Dan suggested above.