Commit 0a0af3cd authored by Robert Ma's avatar Robert Ma Committed by Commit Bot

[WPT Import] Handle Unicode commit subjects correctly

Use a Unicode template string for formatting commit lists, because the
commit subjects can contain non-ASCII characters (and they are Unicode
strings because the underlying host.executive automatically decodes the
subprocess outputs by default).

There are definitely other places that might break when seeing non-ASCII
characters, but nothing can be found around here.

By the way, MockExecutive is changed to match Executive more closely:
run_command now decodes the output and returns a Unicode string by
default. This change isn't required to fix the bug, and doesn't catch
other bugs for now, but hopefully it might be useful in the future.

Bug: 827502
Change-Id: Ib46bfe4cc18d23c2298dfd44bb6ab907dd83e987
Reviewed-on: https://chromium-review.googlesource.com/998676Reviewed-by: default avatarQuinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548608}
parent 74762d7a
...@@ -114,14 +114,14 @@ class MockExecutive(object): ...@@ -114,14 +114,14 @@ class MockExecutive(object):
def run_command(self, def run_command(self,
args, args,
cwd=None, cwd=None,
env=None,
input=None, # pylint: disable=redefined-builtin input=None, # pylint: disable=redefined-builtin
timeout_seconds=False, timeout_seconds=None,
error_handler=None, error_handler=None,
return_exit_code=False, return_exit_code=False,
return_stderr=True, return_stderr=True,
decode_output=False, decode_output=True,
env=None, debug_logging=True):
debug_logging=False):
self._append_call(args, cwd=cwd, input=input, env=env) self._append_call(args, cwd=cwd, input=input, env=env)
assert isinstance(args, list) or isinstance(args, tuple) assert isinstance(args, list) or isinstance(args, tuple)
...@@ -150,10 +150,13 @@ class MockExecutive(object): ...@@ -150,10 +150,13 @@ class MockExecutive(object):
script_error = ScriptError(script_args=args, exit_code=self._exit_code, output=self._output) script_error = ScriptError(script_args=args, exit_code=self._exit_code, output=self._output)
error_handler(script_error) error_handler(script_error)
output = self._output
if return_stderr: if return_stderr:
return self._output + self._stderr output += self._stderr
if decode_output and type(output) is not unicode:
output = output.decode('utf-8')
return self._output return output
def cpu_count(self): def cpu_count(self):
return 2 return 2
......
...@@ -215,7 +215,8 @@ class ImportNotifier(object): ...@@ -215,7 +215,8 @@ class ImportNotifier(object):
directory, self.finder.path_from_layout_tests('external', 'wpt')) directory, self.finder.path_from_layout_tests('external', 'wpt'))
commit_list = '' commit_list = ''
for sha, subject in imported_commits: for sha, subject in imported_commits:
line = '{}: {}'.format(subject, GITHUB_COMMIT_PREFIX + sha) # subject is a Unicode string and can contain non-ASCII characters.
line = u'{}: {}'.format(subject, GITHUB_COMMIT_PREFIX + sha)
if self.local_wpt.is_commit_affecting_directory(sha, path_from_wpt): if self.local_wpt.is_commit_affecting_directory(sha, path_from_wpt):
line += ' [affecting this directory]' line += ' [affecting this directory]'
commit_list += line + '\n' commit_list += line + '\n'
......
# -*- coding: utf-8 -*-
# Copyright 2017 The Chromium Authors. All rights reserved. # Copyright 2017 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be # Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file. # found in the LICENSE file.
...@@ -130,7 +131,9 @@ class ImportNotifierTest(unittest.TestCase): ...@@ -130,7 +131,9 @@ class ImportNotifierTest(unittest.TestCase):
self.assertEqual(self.notifier.new_failures_by_directory, {}) self.assertEqual(self.notifier.new_failures_by_directory, {})
def test_format_commit_list(self): def test_format_commit_list(self):
imported_commits = [('SHA1', 'Subject 1'), ('SHA2', 'Subject 2')] imported_commits = [('SHA1', 'Subject 1'),
# Use non-ASCII chars to really test Unicode handling.
('SHA2', u'ABC~‾¥≈¤・・•∙·☼★星🌟星★☼·∙•・・¤≈¥‾~XYZ')]
def _is_commit_affecting_directory(commit, directory): def _is_commit_affecting_directory(commit, directory):
self.assertIn(commit, ('SHA1', 'SHA2')) self.assertIn(commit, ('SHA1', 'SHA2'))
...@@ -140,8 +143,8 @@ class ImportNotifierTest(unittest.TestCase): ...@@ -140,8 +143,8 @@ class ImportNotifierTest(unittest.TestCase):
self.local_wpt.is_commit_affecting_directory = _is_commit_affecting_directory self.local_wpt.is_commit_affecting_directory = _is_commit_affecting_directory
self.assertEqual( self.assertEqual(
self.notifier.format_commit_list(imported_commits, '/mock-checkout/third_party/WebKit/LayoutTests/external/wpt/foo'), self.notifier.format_commit_list(imported_commits, '/mock-checkout/third_party/WebKit/LayoutTests/external/wpt/foo'),
'Subject 1: https://github.com/w3c/web-platform-tests/commit/SHA1 [affecting this directory]\n' u'Subject 1: https://github.com/w3c/web-platform-tests/commit/SHA1 [affecting this directory]\n'
'Subject 2: https://github.com/w3c/web-platform-tests/commit/SHA2\n' u'ABC~‾¥≈¤・・•∙·☼★星🌟星★☼·∙•・・¤≈¥‾~XYZ: https://github.com/w3c/web-platform-tests/commit/SHA2\n'
) )
def test_find_owned_directory_non_virtual(self): def test_find_owned_directory_non_virtual(self):
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment