Bug 136546 - [Win] webkitpy test suite frequently fails to complete
Summary: [Win] webkitpy test suite frequently fails to complete
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-04 14:35 PDT by Brent Fulgham
Modified: 2014-09-04 16:47 PDT (History)
4 users (show)

See Also:


Attachments
Patch (4.94 KB, patch)
2014-09-04 14:41 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (5.33 KB, patch)
2014-09-04 16:37 PDT, Brent Fulgham
dbates: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2014-09-04 14:35:21 PDT
The Windows port frequently fails to run all tests due to the following two errors:

UnicodeEncodeError raised: 'decimal' codec can't encode characters in position 0-3: invalid decimal Unicode string
Traceback (most recent call last):
  File "/home/buildbot/slave/windows-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 80, in main
    run_details = run(port, options, args, stderr)
  File "/home/buildbot/slave/windows-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 414, in run
    run_details = manager.run(args)
  File "/home/buildbot/slave/windows-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 200, in run
    int(self._options.child_processes), retrying=False)
  File "/home/buildbot/slave/windows-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 257, in _run_tests
    return self._runner.run_tests(self._expectations, test_inputs, tests_to_skip, num_workers, needs_http, needs_websockets, retrying)
  File "/home/buildbot/slave/windows-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 120, in run_tests
    self.start_servers_with_lock(2 * min(num_workers, len(locked_shards)))
  File "/home/buildbot/slave/windows-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 202, in start_servers_with_lock
    self._port.acquire_http_lock()
  File "/home/buildbot/slave/windows-release-tests/build/OpenSource/Tools/Scripts/webkitpy/port/base.py", line 915, in acquire_http_lock
    self._http_lock.wait_for_httpd_lock()
  File "/home/buildbot/slave/windows-release-tests/build/OpenSource/Tools/Scripts/webkitpy/port/http_lock.py", line 134, in wait_for_httpd_lock
    while self._current_lock_pid() != os.getpid():
  File "/home/buildbot/slave/windows-release-tests/build/OpenSource/Tools/Scripts/webkitpy/port/http_lock.py", line 95, in _current_lock_pid
    if not (current_pid and self._executive.check_running_pid(int(current_pid))):
UnicodeEncodeError: 'decimal' codec can't encode characters in position 0-3: invalid decimal Unicode string

The other error is:
worker/2: UnicodeDecodeError(''ascii' codec can't decode byte 0xe0 in position 1889: ordinal not in range(128)') raised:
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/common/message_pool.py", line 255, in run
      worker.handle(message.name, message.src, *message.args)
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 283, in handle
      self._run_test(test_input, test_list_name)
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 308, in _run_test
      result = self._run_test_with_or_without_timeout(test_input, test_timeout_sec, stop_when_done)
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 351, in _run_test_with_or_without_timeout
      return self._run_test_in_this_thread(test_input, stop_when_done)
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 432, in _run_test_in_this_thread
      return self._run_single_test(self._driver, test_input, stop_when_done)
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 436, in _run_single_test
      self._name, driver, test_input, stop_when_done)
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py", line 46, in run_single_test
      return runner.run()
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py", line 102, in run
      return self._run_reftest()
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py", line 313, in _run_reftest
      test_result_writer.write_test_result(self._filesystem, self._port, self._results_directory, self._test_name, test_output, reference_output, test_result.failures)
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py", line 48, in write_test_result
      failure.write_failure(writer, driver_output, expected_driver_output, port)
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py", line 156, in write_failure
      writer.write_crash_log(crashed_driver_output.crash_log)
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py", line 131, in write_crash_log
      self._write_text_file(filename, crash_log)
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py", line 100, in _write_text_file
      self._filesystem.write_text_file(path, contents)
    File "/home/buildbot/slave/windows-64bit-release-tests/build/OpenSource/Tools/Scripts/webkitpy/common/system/filesystem.py", line 227, in write_text_file
      f.write(contents)
    File "/usr/lib/python2.7/codecs.py", line 688, in write
      return self.writer.write(data)
    File "/usr/lib/python2.7/codecs.py", line 351, in write
      data, consumed = self.encode(object, self.errors)
Comment 1 Brent Fulgham 2014-09-04 14:38:21 PDT
The first problem is due to a non-number value being stored in the PID file. This patch adds logging to identify what this content includes so that we can fix any underlying issue.

The second problem appears to be a nuance in the Windows port of Python when the 'codec.open' call is made using the encoding string "utf8" rather than "utf-8". Other ports do not seem to suffer from this problem; perhaps they default to utf-8 when the codec is an unexpected string.

This patch adds error handling for the case of non-numeric (or mixed numeric) pids, and switches the encoding string type from "utf8" to "utf-8", which matches other uses of the codec function in our code base.
Comment 2 Brent Fulgham 2014-09-04 14:41:28 PDT
Created attachment 237648 [details]
Patch
Comment 3 Daniel Bates 2014-09-04 14:56:51 PDT
Comment on attachment 237648 [details]
Patch

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

> Tools/ChangeLog:9
> +        (FileSystem.open_text_file_for_reading): Switch from "utf8" to "utf-8"

Is this necessary? Do you feel it improves the readability of the code? From my understanding of <https://docs.python.org/2/library/codecs.html#standard-encodings> "utf8" is an alias for "utf-8".
Comment 4 Brent Fulgham 2014-09-04 16:12:36 PDT
(In reply to comment #3)
> (From update of attachment 237648 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=237648&action=review
> 
> > Tools/ChangeLog:9
> > +        (FileSystem.open_text_file_for_reading): Switch from "utf8" to "utf-8"
> 
> Is this necessary? Do you feel it improves the readability of the code? From my understanding of <https://docs.python.org/2/library/codecs.html#standard-encodings> "utf8" is an alias for "utf-8".

You are right; that wasn't the problem. The issue is that we are in some cases (e.g., the crash log) getting the contents of the file as a byte string, and passing that value to this function, which assumes it will be given a unicode value that can encode itself as utf-8.

Since we pass a byte string, the moment the codecs class asks it to convert itself to a utf-8 encoding, it (being an ASCII-only byte representation) refuses and throws an exception.

I need to find where this log file is getting read, and make sure it is converted to unicode as early as possible so that this doesn't happen.
Comment 5 Brent Fulgham 2014-09-04 16:37:53 PDT
Created attachment 237658 [details]
Patch
Comment 6 Daniel Bates 2014-09-04 16:43:36 PDT
Comment on attachment 237658 [details]
Patch

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

> Tools/ChangeLog:12
> +        We were mixing ascii/unicode strings under Windows, which was

Nit: ascii => ASCII
unicode => Unicode

> Tools/Scripts/webkitpy/common/system/crashlogs.py:92
> +                    log_file = self._host.filesystem.read_binary_file(path).decode('ascii', 'ignore')

I assume it's always acceptable to decode the file as ASCII regardless of the platform.
Comment 7 Daniel Bates 2014-09-04 16:47:44 PDT
Comment on attachment 237658 [details]
Patch

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

>> Tools/Scripts/webkitpy/common/system/crashlogs.py:92
>> +                    log_file = self._host.filesystem.read_binary_file(path).decode('ascii', 'ignore')
> 
> I assume it's always acceptable to decode the file as ASCII regardless of the platform.

Disregard this comment. This function is specific to Windows.
Comment 8 Brent Fulgham 2014-09-04 16:47:50 PDT
Committed r173290: <http://trac.webkit.org/changeset/173290>