Commit ee6ab987 authored by Igor Makarov's avatar Igor Makarov Committed by Commit Bot

Reformat python code in third_party/blink/tools/blinkpy/w3c for PEP8

- added .style.yapf file
- reformat *.py files

Bug: 1051750
Change-Id: I6ddf9f7e91c6a01bab7eed59d5d4f9b990636e65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2142231Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758449}
parent 4d7703b6
...@@ -8,7 +8,6 @@ from blinkpy.w3c.common import is_file_exportable ...@@ -8,7 +8,6 @@ from blinkpy.w3c.common import is_file_exportable
class ChromiumCommit(object): class ChromiumCommit(object):
def __init__(self, host, sha=None, position=None): def __init__(self, host, sha=None, position=None):
"""Initializes a ChomiumCommit object, given a sha or commit position. """Initializes a ChomiumCommit object, given a sha or commit position.
...@@ -52,20 +51,21 @@ class ChromiumCommit(object): ...@@ -52,20 +51,21 @@ class ChromiumCommit(object):
It is inclusive of this commit and of the latest commit. It is inclusive of this commit and of the latest commit.
""" """
return len(self.host.executive.run_command([ return len(
'git', 'rev-list', '{}..origin/master'.format(self.sha) self.host.executive.run_command(
], cwd=self.absolute_chromium_dir).splitlines()) ['git', 'rev-list', '{}..origin/master'.format(self.sha)],
cwd=self.absolute_chromium_dir).splitlines())
def position_to_sha(self, commit_position): def position_to_sha(self, commit_position):
return self.host.executive.run_command([ return self.host.executive.run_command(
'git', 'crrev-parse', commit_position ['git', 'crrev-parse', commit_position],
], cwd=self.absolute_chromium_dir).strip() cwd=self.absolute_chromium_dir).strip()
def sha_to_position(self, sha): def sha_to_position(self, sha):
try: try:
return self.host.executive.run_command([ return self.host.executive.run_command(
'git', 'footers', '--position', sha ['git', 'footers', '--position', sha],
], cwd=self.absolute_chromium_dir).strip() cwd=self.absolute_chromium_dir).strip()
except ScriptError as e: except ScriptError as e:
# Commits from Gerrit CLs that have not yet been committed in # Commits from Gerrit CLs that have not yet been committed in
# Chromium do not have a commit position. # Chromium do not have a commit position.
...@@ -75,38 +75,40 @@ class ChromiumCommit(object): ...@@ -75,38 +75,40 @@ class ChromiumCommit(object):
raise raise
def subject(self): def subject(self):
return self.host.executive.run_command([ return self.host.executive.run_command(
'git', 'show', '--format=%s', '--no-patch', self.sha ['git', 'show', '--format=%s', '--no-patch', self.sha],
], cwd=self.absolute_chromium_dir).strip() cwd=self.absolute_chromium_dir).strip()
def body(self): def body(self):
return self.host.executive.run_command([ return self.host.executive.run_command(
'git', 'show', '--format=%b', '--no-patch', self.sha ['git', 'show', '--format=%b', '--no-patch', self.sha],
], cwd=self.absolute_chromium_dir) cwd=self.absolute_chromium_dir)
def author(self): def author(self):
return self.host.executive.run_command([ return self.host.executive.run_command(
'git', 'show', '--format=%aN <%aE>', '--no-patch', self.sha ['git', 'show', '--format=%aN <%aE>', '--no-patch', self.sha],
], cwd=self.absolute_chromium_dir).strip() cwd=self.absolute_chromium_dir).strip()
def message(self): def message(self):
"""Returns a string with a commit's subject and body.""" """Returns a string with a commit's subject and body."""
return self.host.executive.run_command([ return self.host.executive.run_command(
'git', 'show', '--format=%B', '--no-patch', self.sha ['git', 'show', '--format=%B', '--no-patch', self.sha],
], cwd=self.absolute_chromium_dir) cwd=self.absolute_chromium_dir)
def change_id(self): def change_id(self):
"""Returns the Change-Id footer if it is present.""" """Returns the Change-Id footer if it is present."""
return self.host.executive.run_command([ return self.host.executive.run_command(
'git', 'footers', '--key', 'Change-Id', self.sha ['git', 'footers', '--key', 'Change-Id', self.sha],
], cwd=self.absolute_chromium_dir).strip() cwd=self.absolute_chromium_dir).strip()
def filtered_changed_files(self): def filtered_changed_files(self):
"""Returns a list of modified exportable files.""" """Returns a list of modified exportable files."""
changed_files = self.host.executive.run_command([ changed_files = self.host.executive.run_command(
'git', 'diff-tree', '--name-only', '--no-commit-id', '-r', self.sha, [
'--', self.absolute_chromium_wpt_dir 'git', 'diff-tree', '--name-only', '--no-commit-id', '-r',
], cwd=self.absolute_chromium_dir).splitlines() self.sha, '--', self.absolute_chromium_wpt_dir
],
cwd=self.absolute_chromium_dir).splitlines()
return [f for f in changed_files if is_file_exportable(f)] return [f for f in changed_files if is_file_exportable(f)]
def format_patch(self): def format_patch(self):
...@@ -116,9 +118,10 @@ class ChromiumCommit(object): ...@@ -116,9 +118,10 @@ class ChromiumCommit(object):
if not filtered_files: if not filtered_files:
return '' return ''
return self.host.executive.run_command([ return self.host.executive.run_command(
'git', 'format-patch', '-1', '--stdout', self.sha, '--' ['git', 'format-patch', '-1', '--stdout', self.sha, '--'] +
] + filtered_files, cwd=self.absolute_chromium_dir) filtered_files,
cwd=self.absolute_chromium_dir)
def url(self): def url(self):
"""Returns a URL to view more information about this commit.""" """Returns a URL to view more information about this commit."""
......
...@@ -6,8 +6,8 @@ import hashlib ...@@ -6,8 +6,8 @@ import hashlib
class MockChromiumCommit(object): class MockChromiumCommit(object):
def __init__(self,
def __init__(self, host, host,
position='refs/heads/master@{#123}', position='refs/heads/master@{#123}',
change_id='Iba5eba11', change_id='Iba5eba11',
author='Fake author', author='Fake author',
......
...@@ -14,41 +14,48 @@ CHROMIUM_WPT_DIR = RELATIVE_WEB_TESTS + 'external/wpt/' ...@@ -14,41 +14,48 @@ CHROMIUM_WPT_DIR = RELATIVE_WEB_TESTS + 'external/wpt/'
class ChromiumCommitTest(unittest.TestCase): class ChromiumCommitTest(unittest.TestCase):
def test_validates_sha(self): def test_validates_sha(self):
with self.assertRaises(AssertionError): with self.assertRaises(AssertionError):
ChromiumCommit(MockHost(), sha='rutabaga') ChromiumCommit(MockHost(), sha='rutabaga')
def test_derives_sha_from_position(self): def test_derives_sha_from_position(self):
host = MockHost() host = MockHost()
host.executive = MockExecutive(output='c881563d734a86f7d9cd57ac509653a61c45c240') host.executive = MockExecutive(
output='c881563d734a86f7d9cd57ac509653a61c45c240')
pos = 'Cr-Commit-Position: refs/heads/master@{#789}' pos = 'Cr-Commit-Position: refs/heads/master@{#789}'
chromium_commit = ChromiumCommit(host, position=pos) chromium_commit = ChromiumCommit(host, position=pos)
self.assertEqual(chromium_commit.position, 'refs/heads/master@{#789}') self.assertEqual(chromium_commit.position, 'refs/heads/master@{#789}')
self.assertEqual(chromium_commit.sha, 'c881563d734a86f7d9cd57ac509653a61c45c240') self.assertEqual(chromium_commit.sha,
'c881563d734a86f7d9cd57ac509653a61c45c240')
def test_derives_position_from_sha(self): def test_derives_position_from_sha(self):
host = MockHost() host = MockHost()
host.executive = mock_git_commands({ host.executive = mock_git_commands({
'footers': 'refs/heads/master@{#789}' 'footers':
'refs/heads/master@{#789}'
}) })
chromium_commit = ChromiumCommit(host, sha='c881563d734a86f7d9cd57ac509653a61c45c240') chromium_commit = ChromiumCommit(
host, sha='c881563d734a86f7d9cd57ac509653a61c45c240')
self.assertEqual(chromium_commit.position, 'refs/heads/master@{#789}') self.assertEqual(chromium_commit.position, 'refs/heads/master@{#789}')
self.assertEqual(chromium_commit.sha, 'c881563d734a86f7d9cd57ac509653a61c45c240') self.assertEqual(chromium_commit.sha,
'c881563d734a86f7d9cd57ac509653a61c45c240')
def test_when_commit_has_no_position(self): def test_when_commit_has_no_position(self):
host = MockHost() host = MockHost()
def run_command(_): def run_command(_):
raise ScriptError('Unable to infer commit position from footers rutabaga') raise ScriptError(
'Unable to infer commit position from footers rutabaga')
host.executive = MockExecutive(run_command_fn=run_command) host.executive = MockExecutive(run_command_fn=run_command)
chromium_commit = ChromiumCommit(host, sha='c881563d734a86f7d9cd57ac509653a61c45c240') chromium_commit = ChromiumCommit(
host, sha='c881563d734a86f7d9cd57ac509653a61c45c240')
self.assertEqual(chromium_commit.position, 'no-commit-position-yet') self.assertEqual(chromium_commit.position, 'no-commit-position-yet')
self.assertEqual(chromium_commit.sha, 'c881563d734a86f7d9cd57ac509653a61c45c240') self.assertEqual(chromium_commit.sha,
'c881563d734a86f7d9cd57ac509653a61c45c240')
def test_filtered_changed_files_blacklist(self): def test_filtered_changed_files_blacklist(self):
host = MockHost() host = MockHost()
...@@ -57,8 +64,10 @@ class ChromiumCommitTest(unittest.TestCase): ...@@ -57,8 +64,10 @@ class ChromiumCommitTest(unittest.TestCase):
qualified_fake_files = [CHROMIUM_WPT_DIR + f for f in fake_files] qualified_fake_files = [CHROMIUM_WPT_DIR + f for f in fake_files]
host.executive = mock_git_commands({ host.executive = mock_git_commands({
'diff-tree': '\n'.join(qualified_fake_files), 'diff-tree':
'crrev-parse': 'c881563d734a86f7d9cd57ac509653a61c45c240', '\n'.join(qualified_fake_files),
'crrev-parse':
'c881563d734a86f7d9cd57ac509653a61c45c240',
}) })
position_footer = 'Cr-Commit-Position: refs/heads/master@{#789}' position_footer = 'Cr-Commit-Position: refs/heads/master@{#789}'
...@@ -67,7 +76,9 @@ class ChromiumCommitTest(unittest.TestCase): ...@@ -67,7 +76,9 @@ class ChromiumCommitTest(unittest.TestCase):
files = chromium_commit.filtered_changed_files() files = chromium_commit.filtered_changed_files()
expected_files = ['file1', 'file3'] expected_files = ['file1', 'file3']
qualified_expected_files = [CHROMIUM_WPT_DIR + f for f in expected_files] qualified_expected_files = [
CHROMIUM_WPT_DIR + f for f in expected_files
]
self.assertEqual(files, qualified_expected_files) self.assertEqual(files, qualified_expected_files)
......
...@@ -6,7 +6,6 @@ from blinkpy.w3c.chromium_commit import ChromiumCommit ...@@ -6,7 +6,6 @@ from blinkpy.w3c.chromium_commit import ChromiumCommit
from blinkpy.w3c.chromium_finder import absolute_chromium_dir from blinkpy.w3c.chromium_finder import absolute_chromium_dir
from blinkpy.w3c.common import CHROMIUM_WPT_DIR from blinkpy.w3c.common import CHROMIUM_WPT_DIR
DEFAULT_COMMIT_HISTORY_WINDOW = 10000 DEFAULT_COMMIT_HISTORY_WINDOW = 10000
SKIPPED_REVISIONS = [ SKIPPED_REVISIONS = [
# The great blink mv: https://crbug.com/843412#c13 # The great blink mv: https://crbug.com/843412#c13
...@@ -15,8 +14,12 @@ SKIPPED_REVISIONS = [ ...@@ -15,8 +14,12 @@ SKIPPED_REVISIONS = [
def exportable_commits_over_last_n_commits( def exportable_commits_over_last_n_commits(
host, local_wpt, wpt_github, number=DEFAULT_COMMIT_HISTORY_WINDOW, host,
require_clean=True, verify_merged_pr=False): local_wpt,
wpt_github,
number=DEFAULT_COMMIT_HISTORY_WINDOW,
require_clean=True,
verify_merged_pr=False):
"""Lists exportable commits after a certain point. """Lists exportable commits after a certain point.
Exportable commits contain changes in the wpt directory and have not been Exportable commits contain changes in the wpt directory and have not been
...@@ -48,11 +51,16 @@ def exportable_commits_over_last_n_commits( ...@@ -48,11 +51,16 @@ def exportable_commits_over_last_n_commits(
cleanly, both in chronological order. cleanly, both in chronological order.
""" """
start_commit = 'HEAD~{}'.format(number + 1) start_commit = 'HEAD~{}'.format(number + 1)
return _exportable_commits_since(start_commit, host, local_wpt, wpt_github, require_clean, verify_merged_pr) return _exportable_commits_since(start_commit, host, local_wpt, wpt_github,
require_clean, verify_merged_pr)
def _exportable_commits_since(chromium_commit_hash, host, local_wpt, wpt_github, def _exportable_commits_since(chromium_commit_hash,
require_clean=True, verify_merged_pr=False): host,
local_wpt,
wpt_github,
require_clean=True,
verify_merged_pr=False):
"""Lists exportable commits after the given commit. """Lists exportable commits after the given commit.
Args: Args:
...@@ -61,33 +69,41 @@ def _exportable_commits_since(chromium_commit_hash, host, local_wpt, wpt_github, ...@@ -61,33 +69,41 @@ def _exportable_commits_since(chromium_commit_hash, host, local_wpt, wpt_github,
Return values and remaining arguments are the same as exportable_commits_over_last_n_commits. Return values and remaining arguments are the same as exportable_commits_over_last_n_commits.
""" """
chromium_repo_root = host.executive.run_command([ chromium_repo_root = host.executive.run_command(
'git', 'rev-parse', '--show-toplevel' ['git', 'rev-parse', '--show-toplevel'],
], cwd=absolute_chromium_dir(host)).strip() cwd=absolute_chromium_dir(host)).strip()
wpt_path = chromium_repo_root + '/' + CHROMIUM_WPT_DIR wpt_path = chromium_repo_root + '/' + CHROMIUM_WPT_DIR
commit_range = '{}..HEAD'.format(chromium_commit_hash) commit_range = '{}..HEAD'.format(chromium_commit_hash)
skipped_revs = ['^' + rev for rev in SKIPPED_REVISIONS] skipped_revs = ['^' + rev for rev in SKIPPED_REVISIONS]
command = ['git', 'rev-list', commit_range] + skipped_revs + ['--reverse', '--', wpt_path] command = (['git', 'rev-list', commit_range] + skipped_revs +
commit_hashes = host.executive.run_command(command, cwd=absolute_chromium_dir(host)).splitlines() ['--reverse', '--', wpt_path])
commit_hashes = host.executive.run_command(
command, cwd=absolute_chromium_dir(host)).splitlines()
chromium_commits = [ChromiumCommit(host, sha=sha) for sha in commit_hashes] chromium_commits = [ChromiumCommit(host, sha=sha) for sha in commit_hashes]
exportable_commits = [] exportable_commits = []
errors = [] errors = []
for commit in chromium_commits: for commit in chromium_commits:
state, error = get_commit_export_state(commit, local_wpt, wpt_github, verify_merged_pr) state, error = get_commit_export_state(commit, local_wpt, wpt_github,
verify_merged_pr)
if require_clean: if require_clean:
success = state == CommitExportState.EXPORTABLE_CLEAN success = state == CommitExportState.EXPORTABLE_CLEAN
else: else:
success = state in (CommitExportState.EXPORTABLE_CLEAN, CommitExportState.EXPORTABLE_DIRTY) success = state in (CommitExportState.EXPORTABLE_CLEAN,
CommitExportState.EXPORTABLE_DIRTY)
if success: if success:
exportable_commits.append(commit) exportable_commits.append(commit)
elif error != '': elif error != '':
errors.append('The following commit did not apply cleanly:\nSubject: %s (%s)\n%s' % errors.append(
(commit.subject(), commit.url(), error)) 'The following commit did not apply cleanly:\nSubject: %s (%s)\n%s' % \
(commit.subject(), commit.url(), error))
return exportable_commits, errors return exportable_commits, errors
def get_commit_export_state(chromium_commit, local_wpt, wpt_github, verify_merged_pr=False): def get_commit_export_state(chromium_commit,
local_wpt,
wpt_github,
verify_merged_pr=False):
"""Determines the exportability state of a Chromium commit. """Determines the exportability state of a Chromium commit.
Args: Args:
...@@ -111,14 +127,17 @@ def get_commit_export_state(chromium_commit, local_wpt, wpt_github, verify_merge ...@@ -111,14 +127,17 @@ def get_commit_export_state(chromium_commit, local_wpt, wpt_github, verify_merge
if not patch: if not patch:
return CommitExportState.NO_PATCH, '' return CommitExportState.NO_PATCH, ''
if _is_commit_exported(chromium_commit, local_wpt, wpt_github, verify_merged_pr): if _is_commit_exported(chromium_commit, local_wpt, wpt_github,
verify_merged_pr):
return CommitExportState.EXPORTED, '' return CommitExportState.EXPORTED, ''
success, error = local_wpt.test_patch(patch) success, error = local_wpt.test_patch(patch)
return (CommitExportState.EXPORTABLE_CLEAN, '') if success else (CommitExportState.EXPORTABLE_DIRTY, error) return ((CommitExportState.EXPORTABLE_CLEAN, '') if success else
(CommitExportState.EXPORTABLE_DIRTY, error))
def _is_commit_exported(chromium_commit, local_wpt, wpt_github, verify_merged_pr): def _is_commit_exported(chromium_commit, local_wpt, wpt_github,
verify_merged_pr):
pull_request = wpt_github.pr_for_chromium_commit(chromium_commit) pull_request = wpt_github.pr_for_chromium_commit(chromium_commit)
if not pull_request or pull_request.state != 'closed': if not pull_request or pull_request.state != 'closed':
return False return False
...@@ -139,8 +158,9 @@ def _is_commit_exported(chromium_commit, local_wpt, wpt_github, verify_merged_pr ...@@ -139,8 +158,9 @@ def _is_commit_exported(chromium_commit, local_wpt, wpt_github, verify_merged_pr
# PR is merged, and we need to verify that local WPT contains the commit. # PR is merged, and we need to verify that local WPT contains the commit.
change_id = chromium_commit.change_id() change_id = chromium_commit.change_id()
found_in_upstream = (local_wpt.seek_change_id(change_id) if change_id found_in_upstream = (local_wpt.seek_change_id(change_id)
else local_wpt.seek_commit_position(chromium_commit.position)) if change_id else local_wpt.seek_commit_position(
chromium_commit.position))
return found_in_upstream return found_in_upstream
......
# Copyright 2016 The Chromium Authors. All rights reserved. # Copyright 2016 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.
"""Utility functions used both when importing and exporting.""" """Utility functions used both when importing and exporting."""
import json import json
...@@ -9,12 +8,12 @@ import logging ...@@ -9,12 +8,12 @@ import logging
from blinkpy.common.path_finder import RELATIVE_WEB_TESTS from blinkpy.common.path_finder import RELATIVE_WEB_TESTS
WPT_GH_ORG = 'web-platform-tests' WPT_GH_ORG = 'web-platform-tests'
WPT_GH_REPO_NAME = 'wpt' WPT_GH_REPO_NAME = 'wpt'
WPT_GH_URL = 'https://github.com/%s/%s/' % (WPT_GH_ORG, WPT_GH_REPO_NAME) WPT_GH_URL = 'https://github.com/%s/%s/' % (WPT_GH_ORG, WPT_GH_REPO_NAME)
WPT_MIRROR_URL = 'https://chromium.googlesource.com/external/github.com/web-platform-tests/wpt.git' WPT_MIRROR_URL = 'https://chromium.googlesource.com/external/github.com/web-platform-tests/wpt.git'
WPT_GH_SSH_URL_TEMPLATE = 'https://{}@github.com/%s/%s.git' % (WPT_GH_ORG, WPT_GH_REPO_NAME) WPT_GH_SSH_URL_TEMPLATE = 'https://{}@github.com/%s/%s.git' % \
(WPT_GH_ORG, WPT_GH_REPO_NAME)
WPT_REVISION_FOOTER = 'WPT-Export-Revision: ' WPT_REVISION_FOOTER = 'WPT-Export-Revision: '
CHANGE_ID_FOOTER = 'Change-Id: ' CHANGE_ID_FOOTER = 'Change-Id: '
EXPORT_PR_LABEL = 'chromium-export' EXPORT_PR_LABEL = 'chromium-export'
...@@ -48,7 +47,8 @@ def read_credentials(host, credentials_json): ...@@ -48,7 +47,8 @@ def read_credentials(host, credentials_json):
if not credentials_json: if not credentials_json:
return env_credentials return env_credentials
if not host.filesystem.exists(credentials_json): if not host.filesystem.exists(credentials_json):
_log.warning('Credentials JSON file not found at %s.', credentials_json) _log.warning('Credentials JSON file not found at %s.',
credentials_json)
return {} return {}
credentials = {} credentials = {}
contents = json.loads(host.filesystem.read_text_file(credentials_json)) contents = json.loads(host.filesystem.read_text_file(credentials_json))
...@@ -75,12 +75,11 @@ def is_basename_skipped(basename): ...@@ -75,12 +75,11 @@ def is_basename_skipped(basename):
""" """
assert '/' not in basename assert '/' not in basename
blacklist = [ blacklist = [
'MANIFEST.json', # MANIFEST.json is automatically regenerated. 'MANIFEST.json', # MANIFEST.json is automatically regenerated.
'OWNERS', # https://crbug.com/584660 https://crbug.com/702283 'OWNERS', # https://crbug.com/584660 https://crbug.com/702283
'reftest.list', # https://crbug.com/582838 'reftest.list', # https://crbug.com/582838
] ]
return (basename in blacklist return (basename in blacklist or is_testharness_baseline(basename)
or is_testharness_baseline(basename)
or basename.startswith('.')) or basename.startswith('.'))
......
...@@ -7,17 +7,12 @@ import unittest ...@@ -7,17 +7,12 @@ import unittest
from blinkpy.common.host_mock import MockHost from blinkpy.common.host_mock import MockHost
from blinkpy.common.path_finder import RELATIVE_WEB_TESTS from blinkpy.common.path_finder import RELATIVE_WEB_TESTS
from blinkpy.w3c.common import ( from blinkpy.w3c.common import (read_credentials, is_testharness_baseline,
read_credentials, is_basename_skipped, is_file_exportable,
is_testharness_baseline, CHROMIUM_WPT_DIR)
is_basename_skipped,
is_file_exportable,
CHROMIUM_WPT_DIR
)
class CommonTest(unittest.TestCase): class CommonTest(unittest.TestCase):
def test_get_credentials_empty(self): def test_get_credentials_empty(self):
host = MockHost() host = MockHost()
host.filesystem.write_text_file('/tmp/credentials.json', '{}') host.filesystem.write_text_file('/tmp/credentials.json', '{}')
...@@ -36,8 +31,7 @@ class CommonTest(unittest.TestCase): ...@@ -36,8 +31,7 @@ class CommonTest(unittest.TestCase):
'UNUSED_VALUE': 'foo', 'UNUSED_VALUE': 'foo',
}) })
self.assertEqual( self.assertEqual(
read_credentials(host, None), read_credentials(host, None), {
{
'GH_USER': 'user-github', 'GH_USER': 'user-github',
'GH_TOKEN': 'pass-github', 'GH_TOKEN': 'pass-github',
'GERRIT_USER': 'user-gerrit', 'GERRIT_USER': 'user-gerrit',
...@@ -55,8 +49,7 @@ class CommonTest(unittest.TestCase): ...@@ -55,8 +49,7 @@ class CommonTest(unittest.TestCase):
'GERRIT_TOKEN': 'pass-gerrit', 'GERRIT_TOKEN': 'pass-gerrit',
})) }))
self.assertEqual( self.assertEqual(
read_credentials(host, '/tmp/credentials.json'), read_credentials(host, '/tmp/credentials.json'), {
{
'GH_USER': 'user-github', 'GH_USER': 'user-github',
'GH_TOKEN': 'pass-github', 'GH_TOKEN': 'pass-github',
'GERRIT_USER': 'user-gerrit', 'GERRIT_USER': 'user-gerrit',
...@@ -78,18 +71,20 @@ class CommonTest(unittest.TestCase): ...@@ -78,18 +71,20 @@ class CommonTest(unittest.TestCase):
'GH_TOKEN': 'pass-github-from-json', 'GH_TOKEN': 'pass-github-from-json',
})) }))
self.assertEqual( self.assertEqual(
read_credentials(host, '/tmp/credentials.json'), read_credentials(host, '/tmp/credentials.json'), {
{
'GH_USER': 'user-github-from-json', 'GH_USER': 'user-github-from-json',
'GH_TOKEN': 'pass-github-from-json', 'GH_TOKEN': 'pass-github-from-json',
}) })
def test_is_testharness_baseline(self): def test_is_testharness_baseline(self):
self.assertTrue(is_testharness_baseline('fake-test-expected.txt')) self.assertTrue(is_testharness_baseline('fake-test-expected.txt'))
self.assertTrue(is_testharness_baseline('external/wpt/fake-test-expected.txt')) self.assertTrue(
self.assertTrue(is_testharness_baseline('/tmp/wpt/fake-test-expected.txt')) is_testharness_baseline('external/wpt/fake-test-expected.txt'))
self.assertTrue(
is_testharness_baseline('/tmp/wpt/fake-test-expected.txt'))
self.assertFalse(is_testharness_baseline('fake-test-expected.html')) self.assertFalse(is_testharness_baseline('fake-test-expected.html'))
self.assertFalse(is_testharness_baseline('external/wpt/fake-test-expected.html')) self.assertFalse(
is_testharness_baseline('external/wpt/fake-test-expected.html'))
def test_is_basename_skipped(self): def test_is_basename_skipped(self):
self.assertTrue(is_basename_skipped('MANIFEST.json')) self.assertTrue(is_basename_skipped('MANIFEST.json'))
...@@ -103,9 +98,13 @@ class CommonTest(unittest.TestCase): ...@@ -103,9 +98,13 @@ class CommonTest(unittest.TestCase):
is_basename_skipped('third_party/fake/OWNERS') is_basename_skipped('third_party/fake/OWNERS')
def test_is_file_exportable(self): def test_is_file_exportable(self):
self.assertTrue(is_file_exportable(CHROMIUM_WPT_DIR + 'html/fake-test.html')) self.assertTrue(
self.assertFalse(is_file_exportable(CHROMIUM_WPT_DIR + 'html/fake-test-expected.txt')) is_file_exportable(CHROMIUM_WPT_DIR + 'html/fake-test.html'))
self.assertFalse(is_file_exportable(CHROMIUM_WPT_DIR + 'MANIFEST.json')) self.assertFalse(
is_file_exportable(CHROMIUM_WPT_DIR +
'html/fake-test-expected.txt'))
self.assertFalse(
is_file_exportable(CHROMIUM_WPT_DIR + 'MANIFEST.json'))
self.assertFalse(is_file_exportable(CHROMIUM_WPT_DIR + 'dom/OWNERS')) self.assertFalse(is_file_exportable(CHROMIUM_WPT_DIR + 'dom/OWNERS'))
def test_is_file_exportable_asserts_path(self): def test_is_file_exportable_asserts_path(self):
...@@ -117,4 +116,5 @@ class CommonTest(unittest.TestCase): ...@@ -117,4 +116,5 @@ class CommonTest(unittest.TestCase):
is_file_exportable('third_party/fake/OWNERS') is_file_exportable('third_party/fake/OWNERS')
# Rejects absolute paths. # Rejects absolute paths.
with self.assertRaises(AssertionError): with self.assertRaises(AssertionError):
is_file_exportable('/mock-checkout/' + RELATIVE_WEB_TESTS + 'external/wpt/OWNERS') is_file_exportable('/mock-checkout/' + RELATIVE_WEB_TESTS +
'external/wpt/OWNERS')
# 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.
"""A limited finder & parser for Chromium OWNERS files. """A limited finder & parser for Chromium OWNERS files.
This module is intended to be used within web_tests/external and is This module is intended to be used within web_tests/external and is
...@@ -16,7 +15,6 @@ from blinkpy.common.memoized import memoized ...@@ -16,7 +15,6 @@ from blinkpy.common.memoized import memoized
from blinkpy.common.path_finder import PathFinder from blinkpy.common.path_finder import PathFinder
from blinkpy.common.system.filesystem import FileSystem from blinkpy.common.system.filesystem import FileSystem
# Format of OWNERS files can be found at //src/third_party/depot_tools/owners.py # Format of OWNERS files can be found at //src/third_party/depot_tools/owners.py
# In our use case (under external/wpt), we only process the first enclosing # In our use case (under external/wpt), we only process the first enclosing
# OWNERS file for any given path (i.e. always assuming "set noparent"), and we # OWNERS file for any given path (i.e. always assuming "set noparent"), and we
...@@ -29,7 +27,6 @@ COMPONENT_REGEXP = r'^# *COMPONENT: *(.+)$' ...@@ -29,7 +27,6 @@ COMPONENT_REGEXP = r'^# *COMPONENT: *(.+)$'
class DirectoryOwnersExtractor(object): class DirectoryOwnersExtractor(object):
def __init__(self, filesystem=None): def __init__(self, filesystem=None):
self.filesystem = filesystem or FileSystem() self.filesystem = filesystem or FileSystem()
self.finder = PathFinder(filesystem) self.finder = PathFinder(filesystem)
...@@ -46,7 +43,8 @@ class DirectoryOwnersExtractor(object): ...@@ -46,7 +43,8 @@ class DirectoryOwnersExtractor(object):
owned directories (paths relative to the root of web tests). owned directories (paths relative to the root of web tests).
""" """
email_map = collections.defaultdict(set) email_map = collections.defaultdict(set)
external_root_owners = self.finder.path_from_web_tests('external', 'OWNERS') external_root_owners = self.finder.path_from_web_tests(
'external', 'OWNERS')
for relpath in changed_files: for relpath in changed_files:
# Try to find the first *non-empty* OWNERS file. # Try to find the first *non-empty* OWNERS file.
absolute_path = self.finder.path_from_chromium_base(relpath) absolute_path = self.finder.path_from_chromium_base(relpath)
...@@ -57,16 +55,21 @@ class DirectoryOwnersExtractor(object): ...@@ -57,16 +55,21 @@ class DirectoryOwnersExtractor(object):
if owners: if owners:
break break
# Found an empty OWNERS file. Try again from the parent directory. # Found an empty OWNERS file. Try again from the parent directory.
absolute_path = self.filesystem.dirname(self.filesystem.dirname(owners_file)) absolute_path = self.filesystem.dirname(
self.filesystem.dirname(owners_file))
owners_file = self.find_owners_file(absolute_path) owners_file = self.find_owners_file(absolute_path)
# Skip web_tests/external/OWNERS. # Skip web_tests/external/OWNERS.
if not owners or owners_file == external_root_owners: if not owners or owners_file == external_root_owners:
continue continue
owned_directory = self.filesystem.dirname(owners_file) owned_directory = self.filesystem.dirname(owners_file)
owned_directory_relpath = self.filesystem.relpath(owned_directory, self.finder.web_tests_dir()) owned_directory_relpath = self.filesystem.relpath(
owned_directory, self.finder.web_tests_dir())
email_map[tuple(owners)].add(owned_directory_relpath) email_map[tuple(owners)].add(owned_directory_relpath)
return {owners: sorted(owned_directories) for owners, owned_directories in email_map.iteritems()} return {
owners: sorted(owned_directories)
for owners, owned_directories in email_map.iteritems()
}
def find_owners_file(self, start_path): def find_owners_file(self, start_path):
"""Finds the first enclosing OWNERS file for a given path. """Finds the first enclosing OWNERS file for a given path.
...@@ -82,8 +85,8 @@ class DirectoryOwnersExtractor(object): ...@@ -82,8 +85,8 @@ class DirectoryOwnersExtractor(object):
The absolute path to the first OWNERS file found; None if not found The absolute path to the first OWNERS file found; None if not found
or if start_path is outside of web_tests/external. or if start_path is outside of web_tests/external.
""" """
abs_start_path = (start_path if self.filesystem.isabs(start_path) abs_start_path = (start_path if self.filesystem.isabs(start_path) else
else self.finder.path_from_chromium_base(start_path)) self.finder.path_from_chromium_base(start_path))
directory = (abs_start_path if self.filesystem.isdir(abs_start_path) directory = (abs_start_path if self.filesystem.isdir(abs_start_path)
else self.filesystem.dirname(abs_start_path)) else self.filesystem.dirname(abs_start_path))
external_root = self.finder.path_from_web_tests('external') external_root = self.finder.path_from_web_tests('external')
...@@ -92,7 +95,8 @@ class DirectoryOwnersExtractor(object): ...@@ -92,7 +95,8 @@ class DirectoryOwnersExtractor(object):
# Stop at web_tests, which is the parent of external_root. # Stop at web_tests, which is the parent of external_root.
while directory != self.finder.web_tests_dir(): while directory != self.finder.web_tests_dir():
owners_file = self.filesystem.join(directory, 'OWNERS') owners_file = self.filesystem.join(directory, 'OWNERS')
if self.filesystem.isfile(self.finder.path_from_chromium_base(owners_file)): if self.filesystem.isfile(
self.finder.path_from_chromium_base(owners_file)):
return owners_file return owners_file
directory = self.filesystem.dirname(directory) directory = self.filesystem.dirname(directory)
return None return None
......
...@@ -8,17 +8,17 @@ from blinkpy.common.path_finder import RELATIVE_WEB_TESTS ...@@ -8,17 +8,17 @@ from blinkpy.common.path_finder import RELATIVE_WEB_TESTS
from blinkpy.common.system.filesystem_mock import MockFileSystem from blinkpy.common.system.filesystem_mock import MockFileSystem
from blinkpy.w3c.directory_owners_extractor import DirectoryOwnersExtractor from blinkpy.w3c.directory_owners_extractor import DirectoryOwnersExtractor
MOCK_WEB_TESTS = '/mock-checkout/' + RELATIVE_WEB_TESTS MOCK_WEB_TESTS = '/mock-checkout/' + RELATIVE_WEB_TESTS
ABS_WPT_BASE = MOCK_WEB_TESTS + 'external/wpt' ABS_WPT_BASE = MOCK_WEB_TESTS + 'external/wpt'
REL_WPT_BASE = RELATIVE_WEB_TESTS + 'external/wpt' REL_WPT_BASE = RELATIVE_WEB_TESTS + 'external/wpt'
class DirectoryOwnersExtractorTest(unittest.TestCase):
class DirectoryOwnersExtractorTest(unittest.TestCase):
def setUp(self): def setUp(self):
# We always have an OWNERS file at web_tests/external. # We always have an OWNERS file at web_tests/external.
self.filesystem = MockFileSystem(files={ self.filesystem = MockFileSystem(files={
MOCK_WEB_TESTS + 'external/OWNERS': 'ecosystem-infra@chromium.org' MOCK_WEB_TESTS + 'external/OWNERS':
'ecosystem-infra@chromium.org'
}) })
self.extractor = DirectoryOwnersExtractor(self.filesystem) self.extractor = DirectoryOwnersExtractor(self.filesystem)
...@@ -30,19 +30,24 @@ class DirectoryOwnersExtractorTest(unittest.TestCase): ...@@ -30,19 +30,24 @@ class DirectoryOwnersExtractorTest(unittest.TestCase):
def test_list_owners_combines_same_owners(self): def test_list_owners_combines_same_owners(self):
self._write_files({ self._write_files({
ABS_WPT_BASE + '/foo/x.html': '', ABS_WPT_BASE + '/foo/x.html':
ABS_WPT_BASE + '/foo/OWNERS': 'a@chromium.org\nc@chromium.org\n', '',
ABS_WPT_BASE + '/bar/x/y.html': '', ABS_WPT_BASE + '/foo/OWNERS':
ABS_WPT_BASE + '/bar/OWNERS': 'a@chromium.org\nc@chromium.org\n', 'a@chromium.org\nc@chromium.org\n',
ABS_WPT_BASE + '/bar/x/y.html':
'',
ABS_WPT_BASE + '/bar/OWNERS':
'a@chromium.org\nc@chromium.org\n',
}) })
changed_files = [ changed_files = [
REL_WPT_BASE + '/foo/x.html', REL_WPT_BASE + '/foo/x.html',
REL_WPT_BASE + '/bar/x/y.html', REL_WPT_BASE + '/bar/x/y.html',
] ]
self.assertEqual( self.assertEqual(
self.extractor.list_owners(changed_files), self.extractor.list_owners(changed_files), {
{('a@chromium.org', 'c@chromium.org'): ['external/wpt/bar', 'external/wpt/foo']} ('a@chromium.org', 'c@chromium.org'):
) ['external/wpt/bar', 'external/wpt/foo']
})
def test_list_owners_combines_same_directory(self): def test_list_owners_combines_same_directory(self):
self._write_files({ self._write_files({
...@@ -56,30 +61,34 @@ class DirectoryOwnersExtractorTest(unittest.TestCase): ...@@ -56,30 +61,34 @@ class DirectoryOwnersExtractorTest(unittest.TestCase):
] ]
self.assertEqual( self.assertEqual(
self.extractor.list_owners(changed_files), self.extractor.list_owners(changed_files),
{('foo@chromium.org',): ['external/wpt/baz/x']} {('foo@chromium.org', ): ['external/wpt/baz/x']})
)
def test_list_owners_skips_empty_owners(self): def test_list_owners_skips_empty_owners(self):
self._write_files({ self._write_files({
ABS_WPT_BASE + '/baz/x/y/z.html': '', ABS_WPT_BASE + '/baz/x/y/z.html':
ABS_WPT_BASE + '/baz/x/y/OWNERS': '# Some comments\n', '',
ABS_WPT_BASE + '/baz/x/OWNERS': 'foo@chromium.org\n', ABS_WPT_BASE + '/baz/x/y/OWNERS':
'# Some comments\n',
ABS_WPT_BASE + '/baz/x/OWNERS':
'foo@chromium.org\n',
}) })
changed_files = [ changed_files = [
REL_WPT_BASE + '/baz/x/y/z.html', REL_WPT_BASE + '/baz/x/y/z.html',
] ]
self.assertEqual( self.assertEqual(
self.extractor.list_owners(changed_files), self.extractor.list_owners(changed_files),
{('foo@chromium.org',): ['external/wpt/baz/x']} {('foo@chromium.org', ): ['external/wpt/baz/x']})
)
def test_list_owners_not_found(self): def test_list_owners_not_found(self):
self._write_files({ self._write_files({
# Although web_tests/external/OWNERS exists, it should not be listed. # Although web_tests/external/OWNERS exists, it should not be listed.
ABS_WPT_BASE + '/foo/bar.html': '', ABS_WPT_BASE + '/foo/bar.html':
'',
# Files out of external. # Files out of external.
'/mock-checkout/' + RELATIVE_WEB_TESTS + 'TestExpectations': '', '/mock-checkout/' + RELATIVE_WEB_TESTS + 'TestExpectations':
'/mock-checkout/' + RELATIVE_WEB_TESTS + 'OWNERS': 'foo@chromium.org', '',
'/mock-checkout/' + RELATIVE_WEB_TESTS + 'OWNERS':
'foo@chromium.org',
}) })
changed_files = [ changed_files = [
REL_WPT_BASE + '/foo/bar.html', REL_WPT_BASE + '/foo/bar.html',
...@@ -88,24 +97,27 @@ class DirectoryOwnersExtractorTest(unittest.TestCase): ...@@ -88,24 +97,27 @@ class DirectoryOwnersExtractorTest(unittest.TestCase):
self.assertEqual(self.extractor.list_owners(changed_files), {}) self.assertEqual(self.extractor.list_owners(changed_files), {})
def test_find_owners_file_at_current_dir(self): def test_find_owners_file_at_current_dir(self):
self._write_files({ self._write_files({ABS_WPT_BASE + '/foo/OWNERS': 'a@chromium.org'})
ABS_WPT_BASE + '/foo/OWNERS': 'a@chromium.org' self.assertEqual(
}) self.extractor.find_owners_file(REL_WPT_BASE + '/foo'),
self.assertEqual(self.extractor.find_owners_file(REL_WPT_BASE + '/foo'), ABS_WPT_BASE + '/foo/OWNERS') ABS_WPT_BASE + '/foo/OWNERS')
def test_find_owners_file_at_ancestor(self): def test_find_owners_file_at_ancestor(self):
self._write_files({ self._write_files({
ABS_WPT_BASE + '/x/OWNERS': 'a@chromium.org', ABS_WPT_BASE + '/x/OWNERS': 'a@chromium.org',
ABS_WPT_BASE + '/x/y/z.html': '', ABS_WPT_BASE + '/x/y/z.html': '',
}) })
self.assertEqual(self.extractor.find_owners_file(REL_WPT_BASE + '/x/y'), ABS_WPT_BASE + '/x/OWNERS') self.assertEqual(
self.extractor.find_owners_file(REL_WPT_BASE + '/x/y'),
ABS_WPT_BASE + '/x/OWNERS')
def test_find_owners_file_stops_at_external_root(self): def test_find_owners_file_stops_at_external_root(self):
self._write_files({ self._write_files({
ABS_WPT_BASE + '/x/y/z.html': '', ABS_WPT_BASE + '/x/y/z.html': '',
}) })
self.assertEqual(self.extractor.find_owners_file(REL_WPT_BASE + '/x/y'), self.assertEqual(
MOCK_WEB_TESTS + 'external/OWNERS') self.extractor.find_owners_file(REL_WPT_BASE + '/x/y'),
MOCK_WEB_TESTS + 'external/OWNERS')
def test_find_owners_file_takes_four_kinds_of_paths(self): def test_find_owners_file_takes_four_kinds_of_paths(self):
owners_path = ABS_WPT_BASE + '/foo/OWNERS' owners_path = ABS_WPT_BASE + '/foo/OWNERS'
...@@ -114,21 +126,33 @@ class DirectoryOwnersExtractorTest(unittest.TestCase): ...@@ -114,21 +126,33 @@ class DirectoryOwnersExtractorTest(unittest.TestCase):
ABS_WPT_BASE + '/foo/bar.html': '', ABS_WPT_BASE + '/foo/bar.html': '',
}) })
# Absolute paths of directories. # Absolute paths of directories.
self.assertEqual(self.extractor.find_owners_file(ABS_WPT_BASE + '/foo'), owners_path) self.assertEqual(
self.extractor.find_owners_file(ABS_WPT_BASE + '/foo'),
owners_path)
# Relative paths of directories. # Relative paths of directories.
self.assertEqual(self.extractor.find_owners_file(REL_WPT_BASE + '/foo'), owners_path) self.assertEqual(
self.extractor.find_owners_file(REL_WPT_BASE + '/foo'),
owners_path)
# Absolute paths of files. # Absolute paths of files.
self.assertEqual(self.extractor.find_owners_file(ABS_WPT_BASE + '/foo/bar.html'), owners_path) self.assertEqual(
self.extractor.find_owners_file(ABS_WPT_BASE + '/foo/bar.html'),
owners_path)
# Relative paths of files. # Relative paths of files.
self.assertEqual(self.extractor.find_owners_file(REL_WPT_BASE + '/foo/bar.html'), owners_path) self.assertEqual(
self.extractor.find_owners_file(REL_WPT_BASE + '/foo/bar.html'),
owners_path)
def test_find_owners_file_out_of_external(self): def test_find_owners_file_out_of_external(self):
self._write_files({ self._write_files({
'/mock-checkout/' + RELATIVE_WEB_TESTS + 'OWNERS': 'foo@chromium.org', '/mock-checkout/' + RELATIVE_WEB_TESTS + 'OWNERS':
'/mock-checkout/' + RELATIVE_WEB_TESTS + 'other/some_file': '', 'foo@chromium.org',
'/mock-checkout/' + RELATIVE_WEB_TESTS + 'other/some_file':
'',
}) })
self.assertIsNone(self.extractor.find_owners_file(RELATIVE_WEB_TESTS[:-1])) self.assertIsNone(
self.assertIsNone(self.extractor.find_owners_file(RELATIVE_WEB_TESTS + 'other')) self.extractor.find_owners_file(RELATIVE_WEB_TESTS[:-1]))
self.assertIsNone(
self.extractor.find_owners_file(RELATIVE_WEB_TESTS + 'other'))
self.assertIsNone(self.extractor.find_owners_file('third_party')) self.assertIsNone(self.extractor.find_owners_file('third_party'))
def test_extract_owners(self): def test_extract_owners(self):
...@@ -143,8 +167,9 @@ class DirectoryOwnersExtractorTest(unittest.TestCase): ...@@ -143,8 +167,9 @@ class DirectoryOwnersExtractorTest(unittest.TestCase):
'# TEAM: some-team@chromium.org\n' '# TEAM: some-team@chromium.org\n'
'# COMPONENT: Blink>Layout\n' '# COMPONENT: Blink>Layout\n'
} }
self.assertEqual(self.extractor.extract_owners(ABS_WPT_BASE + '/foo/OWNERS'), self.assertEqual(
['foo@chromium.org', 'bar@chromium.org']) self.extractor.extract_owners(ABS_WPT_BASE + '/foo/OWNERS'),
['foo@chromium.org', 'bar@chromium.org'])
def test_extract_component(self): def test_extract_component(self):
self.filesystem.files = { self.filesystem.files = {
...@@ -152,7 +177,9 @@ class DirectoryOwnersExtractorTest(unittest.TestCase): ...@@ -152,7 +177,9 @@ class DirectoryOwnersExtractorTest(unittest.TestCase):
'# TEAM: some-team@chromium.org\n' '# TEAM: some-team@chromium.org\n'
'# COMPONENT: Blink>Layout\n' '# COMPONENT: Blink>Layout\n'
} }
self.assertEqual(self.extractor.extract_component(ABS_WPT_BASE + '/foo/OWNERS'), 'Blink>Layout') self.assertEqual(
self.extractor.extract_component(ABS_WPT_BASE + '/foo/OWNERS'),
'Blink>Layout')
def test_is_wpt_notify_enabled_true(self): def test_is_wpt_notify_enabled_true(self):
self.filesystem.files = { self.filesystem.files = {
...@@ -160,7 +187,8 @@ class DirectoryOwnersExtractorTest(unittest.TestCase): ...@@ -160,7 +187,8 @@ class DirectoryOwnersExtractorTest(unittest.TestCase):
'# COMPONENT: Blink>Layout\n' '# COMPONENT: Blink>Layout\n'
'# WPT-NOTIFY: true\n' '# WPT-NOTIFY: true\n'
} }
self.assertTrue(self.extractor.is_wpt_notify_enabled(ABS_WPT_BASE + '/foo/OWNERS')) self.assertTrue(
self.extractor.is_wpt_notify_enabled(ABS_WPT_BASE + '/foo/OWNERS'))
def test_is_wpt_notify_enabled_false(self): def test_is_wpt_notify_enabled_false(self):
self.filesystem.files = { self.filesystem.files = {
...@@ -168,7 +196,8 @@ class DirectoryOwnersExtractorTest(unittest.TestCase): ...@@ -168,7 +196,8 @@ class DirectoryOwnersExtractorTest(unittest.TestCase):
'# COMPONENT: Blink>Layout\n' '# COMPONENT: Blink>Layout\n'
'# WPT-NOTIFY: false\n' '# WPT-NOTIFY: false\n'
} }
self.assertFalse(self.extractor.is_wpt_notify_enabled(ABS_WPT_BASE + '/foo/OWNERS')) self.assertFalse(
self.extractor.is_wpt_notify_enabled(ABS_WPT_BASE + '/foo/OWNERS'))
def test_is_wpt_notify_enabled_absence_is_false(self): def test_is_wpt_notify_enabled_absence_is_false(self):
self.filesystem.files = { self.filesystem.files = {
...@@ -176,4 +205,5 @@ class DirectoryOwnersExtractorTest(unittest.TestCase): ...@@ -176,4 +205,5 @@ class DirectoryOwnersExtractorTest(unittest.TestCase):
'# TEAM: some-team@chromium.org\n' '# TEAM: some-team@chromium.org\n'
'# COMPONENT: Blink>Layout\n' '# COMPONENT: Blink>Layout\n'
} }
self.assertFalse(self.extractor.is_wpt_notify_enabled(ABS_WPT_BASE + '/foo/OWNERS')) self.assertFalse(
self.extractor.is_wpt_notify_enabled(ABS_WPT_BASE + '/foo/OWNERS'))
# Copyright 2019 The Chromium Authors. All rights reserved. # Copyright 2019 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.
"""Sends notifications after automatic exports. """Sends notifications after automatic exports.
Automatically comments on a Gerrit CL when its corresponding PR fails the Taskcluster check. In Automatically comments on a Gerrit CL when its corresponding PR fails the Taskcluster check. In
...@@ -22,7 +21,6 @@ _log = logging.getLogger(__name__) ...@@ -22,7 +21,6 @@ _log = logging.getLogger(__name__)
class ExportNotifier(object): class ExportNotifier(object):
def __init__(self, host, wpt_github, gerrit, dry_run=True): def __init__(self, host, wpt_github, gerrit, dry_run=True):
self.host = host self.host = host
self.wpt_github = wpt_github self.wpt_github = wpt_github
...@@ -38,13 +36,13 @@ class ExportNotifier(object): ...@@ -38,13 +36,13 @@ class ExportNotifier(object):
prs = self.wpt_github.recent_failing_chromium_exports() prs = self.wpt_github.recent_failing_chromium_exports()
except GitHubError as e: except GitHubError as e:
_log.info( _log.info(
'Surfacing Taskcluster failures cannot be completed due to the following error:') 'Surfacing Taskcluster failures cannot be completed due to the following error:'
)
_log.error(str(e)) _log.error(str(e))
return True return True
if len(prs) > 100: if len(prs) > 100:
_log.error( _log.error('Too many open failing PRs: %s; abort.', len(prs))
'Too many open failing PRs: %s; abort.', len(prs))
return True return True
_log.info('Found %d failing PRs.', len(prs)) _log.info('Found %d failing PRs.', len(prs))
...@@ -61,15 +59,14 @@ class ExportNotifier(object): ...@@ -61,15 +59,14 @@ class ExportNotifier(object):
gerrit_id = self.wpt_github.extract_metadata( gerrit_id = self.wpt_github.extract_metadata(
'Change-Id: ', pr.body) 'Change-Id: ', pr.body)
if not gerrit_id: if not gerrit_id:
_log.error( _log.error('Can not retrieve Change-Id for %s.', pr.number)
'Can not retrieve Change-Id for %s.', pr.number)
continue continue
gerrit_sha = self.wpt_github.extract_metadata(WPT_REVISION_FOOTER, pr.body) gerrit_sha = self.wpt_github.extract_metadata(
WPT_REVISION_FOOTER, pr.body)
gerrit_dict[gerrit_id] = PRStatusInfo( gerrit_dict[gerrit_id] = PRStatusInfo(
taskcluster_status['node_id'], taskcluster_status['node_id'],
taskcluster_status['target_url'], taskcluster_status['target_url'], gerrit_sha)
gerrit_sha)
self.process_failing_prs(gerrit_dict) self.process_failing_prs(gerrit_dict)
return False return False
...@@ -109,19 +106,19 @@ class ExportNotifier(object): ...@@ -109,19 +106,19 @@ class ExportNotifier(object):
cl_comment = pr_status_info.to_gerrit_comment() cl_comment = pr_status_info.to_gerrit_comment()
if self.dry_run: if self.dry_run:
_log.info( _log.info('[dry_run] Would have commented on CL %s\n',
'[dry_run] Would have commented on CL %s\n', change_id) change_id)
_log.debug( _log.debug('Comments are:\n %s\n', cl_comment)
'Comments are:\n %s\n', cl_comment)
else: else:
_log.info('Commenting on CL %s\n', change_id) _log.info('Commenting on CL %s\n', change_id)
cl.post_comment(cl_comment) cl.post_comment(cl_comment)
except GerritError as e: except GerritError as e:
_log.error( _log.error('Could not process Gerrit CL %s: %s', change_id,
'Could not process Gerrit CL %s: %s', change_id, str(e)) str(e))
continue continue
def has_latest_taskcluster_status_commented(self, messages, pr_status_info): def has_latest_taskcluster_status_commented(self, messages,
pr_status_info):
"""Determines if the Taskcluster status has already been commented on the messages of a CL. """Determines if the Taskcluster status has already been commented on the messages of a CL.
Args: Args:
...@@ -206,9 +203,10 @@ class PRStatusInfo(object): ...@@ -206,9 +203,10 @@ class PRStatusInfo(object):
@staticmethod @staticmethod
def from_gerrit_comment(comment): def from_gerrit_comment(comment):
tags = [PRStatusInfo.NODE_ID_TAG, tags = [
PRStatusInfo.LINK_TAG, PRStatusInfo.NODE_ID_TAG, PRStatusInfo.LINK_TAG,
PRStatusInfo.CL_SHA_TAG] PRStatusInfo.CL_SHA_TAG
]
values = ['', '', ''] values = ['', '', '']
for line in comment.splitlines(): for line in comment.splitlines():
...@@ -223,12 +221,13 @@ class PRStatusInfo(object): ...@@ -223,12 +221,13 @@ class PRStatusInfo(object):
return PRStatusInfo(*values) return PRStatusInfo(*values)
def to_gerrit_comment(self, patchset=None): def to_gerrit_comment(self, patchset=None):
status_line = ('The exported PR for the current patch failed Taskcluster check(s) ' status_line = (
'on GitHub, which could indict cross-broswer failures on the ' 'The exported PR for the current patch failed Taskcluster check(s) '
'exportable changes. Please contact ecosystem-infra@ team for ' 'on GitHub, which could indict cross-broswer failures on the '
'more information.') 'exportable changes. Please contact ecosystem-infra@ team for '
node_id_line = ('\n\n{}{}').format( 'more information.')
PRStatusInfo.NODE_ID_TAG, self.node_id) node_id_line = ('\n\n{}{}').format(PRStatusInfo.NODE_ID_TAG,
self.node_id)
link_line = ('\n{}{}').format(PRStatusInfo.LINK_TAG, self.link) link_line = ('\n{}{}').format(PRStatusInfo.LINK_TAG, self.link)
sha_line = ('\n{}{}').format(PRStatusInfo.CL_SHA_TAG, self.gerrit_sha) sha_line = ('\n{}{}').format(PRStatusInfo.CL_SHA_TAG, self.gerrit_sha)
......
...@@ -47,7 +47,8 @@ class GerritAPI(object): ...@@ -47,7 +47,8 @@ class GerritAPI(object):
The path has to be prefixed with '/a/': The path has to be prefixed with '/a/':
https://gerrit-review.googlesource.com/Documentation/rest-api.html#authentication https://gerrit-review.googlesource.com/Documentation/rest-api.html#authentication
""" """
assert path.startswith('/a/'), 'POST requests need to use authenticated routes.' assert path.startswith('/a/'), \
'POST requests need to use authenticated routes.'
url = URL_BASE + path url = URL_BASE + path
assert self.user and self.token, 'Gerrit user and token required for authenticated routes.' assert self.user and self.token, 'Gerrit user and token required for authenticated routes.'
...@@ -56,7 +57,8 @@ class GerritAPI(object): ...@@ -56,7 +57,8 @@ class GerritAPI(object):
'Authorization': 'Basic {}'.format(b64auth), 'Authorization': 'Basic {}'.format(b64auth),
'Content-Type': 'application/json', 'Content-Type': 'application/json',
} }
return self.host.web.request('POST', url, data=json.dumps(data), headers=headers) return self.host.web.request(
'POST', url, data=json.dumps(data), headers=headers)
def query_cl_comments_and_revisions(self, change_id): def query_cl_comments_and_revisions(self, change_id):
"""Queries a CL with comments and revisions information.""" """Queries a CL with comments and revisions information."""
...@@ -64,7 +66,8 @@ class GerritAPI(object): ...@@ -64,7 +66,8 @@ class GerritAPI(object):
def query_cl(self, change_id, query_options=QUERY_OPTIONS): def query_cl(self, change_id, query_options=QUERY_OPTIONS):
"""Queries a commit information from Gerrit.""" """Queries a commit information from Gerrit."""
path = '/changes/chromium%2Fsrc~master~{}?{}'.format(change_id, query_options) path = '/changes/chromium%2Fsrc~master~{}?{}'.format(
change_id, query_options)
try: try:
cl_data = self.get(path) cl_data = self.get(path)
except NetworkTimeout: except NetworkTimeout:
...@@ -149,13 +152,13 @@ class GerritCL(object): ...@@ -149,13 +152,13 @@ class GerritCL(object):
def post_comment(self, message): def post_comment(self, message):
"""Posts a comment to the CL.""" """Posts a comment to the CL."""
path = '/a/changes/{change_id}/revisions/current/review'.format( path = '/a/changes/{change_id}/revisions/current/review'.format(
change_id=self.change_id, change_id=self.change_id, )
)
try: try:
return self.api.post(path, {'message': message}) return self.api.post(path, {'message': message})
except HTTPError as e: except HTTPError as e:
raise GerritError('Failed to post a comment to issue {} (code {}).'.format( raise GerritError(
self.change_id, e.code)) 'Failed to post a comment to issue {} (code {}).'.format(
self.change_id, e.code))
def is_exportable(self): def is_exportable(self):
# TODO(robertma): Consolidate with the related part in chromium_exportable_commits.py. # TODO(robertma): Consolidate with the related part in chromium_exportable_commits.py.
......
...@@ -9,7 +9,6 @@ from blinkpy.w3c.gerrit import GerritCL, GerritError, QUERY_OPTIONS ...@@ -9,7 +9,6 @@ from blinkpy.w3c.gerrit import GerritCL, GerritError, QUERY_OPTIONS
class MockGerritAPI(object): class MockGerritAPI(object):
def __init__(self, raise_error=False): def __init__(self, raise_error=False):
self.exportable_open_cls = [] self.exportable_open_cls = []
self.request_posted = [] self.request_posted = []
...@@ -38,7 +37,6 @@ class MockGerritAPI(object): ...@@ -38,7 +37,6 @@ class MockGerritAPI(object):
class MockGerritCL(GerritCL): class MockGerritCL(GerritCL):
def __init__(self, data, api=None, chromium_commit=None): def __init__(self, data, api=None, chromium_commit=None):
api = api or MockGerritAPI() api = api or MockGerritAPI()
self.chromium_commit = chromium_commit self.chromium_commit = chromium_commit
......
...@@ -26,51 +26,69 @@ class GerritAPITest(unittest.TestCase): ...@@ -26,51 +26,69 @@ class GerritAPITest(unittest.TestCase):
class GerritCLTest(unittest.TestCase): class GerritCLTest(unittest.TestCase):
def test_url(self): def test_url(self):
data = { data = {
'change_id': 'Ib58c7125d85d2fd71af711ea8bbd2dc927ed02cb', 'change_id': 'Ib58c7125d85d2fd71af711ea8bbd2dc927ed02cb',
'_number': 638250, '_number': 638250,
} }
gerrit_cl = GerritCL(data, MockGerritAPI()) gerrit_cl = GerritCL(data, MockGerritAPI())
self.assertEqual(gerrit_cl.url, 'https://chromium-review.googlesource.com/638250') self.assertEqual(gerrit_cl.url,
'https://chromium-review.googlesource.com/638250')
def test_fetch_current_revision_commit(self): def test_fetch_current_revision_commit(self):
host = MockHost() host = MockHost()
host.executive = mock_git_commands({ host.executive = mock_git_commands(
'fetch': '', {
'rev-parse': '4de71d0ce799af441c1f106c5432c7fa7256be45', 'fetch': '',
'footers': 'no-commit-position-yet' 'rev-parse': '4de71d0ce799af441c1f106c5432c7fa7256be45',
}, strict=True) 'footers': 'no-commit-position-yet'
},
strict=True)
data = { data = {
'change_id': 'Ib58c7125d85d2fd71af711ea8bbd2dc927ed02cb', 'change_id': 'Ib58c7125d85d2fd71af711ea8bbd2dc927ed02cb',
'subject': 'fake subject', 'subject': 'fake subject',
'_number': 638250, '_number': 638250,
'current_revision': '1', 'current_revision': '1',
'revisions': {'1': { 'revisions': {
'fetch': {'http': { '1': {
'url': 'https://chromium.googlesource.com/chromium/src', 'fetch': {
'ref': 'refs/changes/50/638250/1' 'http': {
}} 'url':
}}, 'https://chromium.googlesource.com/chromium/src',
'owner': {'email': 'test@chromium.org'}, 'ref':
'refs/changes/50/638250/1'
}
}
}
},
'owner': {
'email': 'test@chromium.org'
},
} }
gerrit_cl = GerritCL(data, MockGerritAPI()) gerrit_cl = GerritCL(data, MockGerritAPI())
commit = gerrit_cl.fetch_current_revision_commit(host) commit = gerrit_cl.fetch_current_revision_commit(host)
self.assertEqual(commit.sha, '4de71d0ce799af441c1f106c5432c7fa7256be45') self.assertEqual(commit.sha,
self.assertEqual(host.executive.calls, [ '4de71d0ce799af441c1f106c5432c7fa7256be45')
['git', 'fetch', 'https://chromium.googlesource.com/chromium/src', 'refs/changes/50/638250/1'], self.assertEqual(host.executive.calls,
['git', 'rev-parse', 'FETCH_HEAD'], [[
['git', 'footers', '--position', '4de71d0ce799af441c1f106c5432c7fa7256be45'] 'git', 'fetch',
]) 'https://chromium.googlesource.com/chromium/src',
'refs/changes/50/638250/1'
], ['git', 'rev-parse', 'FETCH_HEAD'],
[
'git', 'footers', '--position',
'4de71d0ce799af441c1f106c5432c7fa7256be45'
]])
def test_empty_cl_is_not_exportable(self): def test_empty_cl_is_not_exportable(self):
data = { data = {
'change_id': 'Ib58c7125d85d2fd71af711ea8bbd2dc927ed02cb', 'change_id': 'Ib58c7125d85d2fd71af711ea8bbd2dc927ed02cb',
'subject': 'fake subject', 'subject': 'fake subject',
'_number': 638250, '_number': 638250,
'owner': {'email': 'test@chromium.org'}, 'owner': {
'email': 'test@chromium.org'
},
} }
gerrit_cl = GerritCL(data, MockGerritAPI()) gerrit_cl = GerritCL(data, MockGerritAPI())
# It's important that this does not throw! # It's important that this does not throw!
...@@ -82,13 +100,17 @@ class GerritCLTest(unittest.TestCase): ...@@ -82,13 +100,17 @@ class GerritCLTest(unittest.TestCase):
'subject': 'fake subject', 'subject': 'fake subject',
'_number': 638250, '_number': 638250,
'current_revision': '1', 'current_revision': '1',
'revisions': {'1': { 'revisions': {
'commit_with_footers': 'fake subject', '1': {
'files': { 'commit_with_footers': 'fake subject',
RELATIVE_WEB_TESTS + 'external/wpt/foo/bar.html': '', 'files': {
RELATIVE_WEB_TESTS + 'external/wpt/foo/bar.html': '',
}
} }
}}, },
'owner': {'email': 'test@chromium.org'}, 'owner': {
'email': 'test@chromium.org'
},
} }
gerrit_cl = GerritCL(data, MockGerritAPI()) gerrit_cl = GerritCL(data, MockGerritAPI())
self.assertTrue(gerrit_cl.is_exportable()) self.assertTrue(gerrit_cl.is_exportable())
...@@ -99,13 +121,17 @@ class GerritCLTest(unittest.TestCase): ...@@ -99,13 +121,17 @@ class GerritCLTest(unittest.TestCase):
'subject': 'fake subject', 'subject': 'fake subject',
'_number': 638250, '_number': 638250,
'current_revision': '1', 'current_revision': '1',
'revisions': {'1': { 'revisions': {
'commit_with_footers': 'fake subject', '1': {
'files': { 'commit_with_footers': 'fake subject',
RELATIVE_WEB_TESTS + 'foo/bar.html': '', 'files': {
RELATIVE_WEB_TESTS + 'foo/bar.html': '',
}
} }
}}, },
'owner': {'email': 'test@chromium.org'}, 'owner': {
'email': 'test@chromium.org'
},
} }
gerrit_cl = GerritCL(data, MockGerritAPI()) gerrit_cl = GerritCL(data, MockGerritAPI())
self.assertFalse(gerrit_cl.is_exportable()) self.assertFalse(gerrit_cl.is_exportable())
...@@ -116,13 +142,17 @@ class GerritCLTest(unittest.TestCase): ...@@ -116,13 +142,17 @@ class GerritCLTest(unittest.TestCase):
'subject': 'fake subject', 'subject': 'fake subject',
'_number': 638250, '_number': 638250,
'current_revision': '1', 'current_revision': '1',
'revisions': {'1': { 'revisions': {
'commit_with_footers': 'fake subject\nNo-Export: true', '1': {
'files': { 'commit_with_footers': 'fake subject\nNo-Export: true',
RELATIVE_WEB_TESTS + 'external/wpt/foo/bar.html': '', 'files': {
RELATIVE_WEB_TESTS + 'external/wpt/foo/bar.html': '',
}
} }
}}, },
'owner': {'email': 'test@chromium.org'}, 'owner': {
'email': 'test@chromium.org'
},
} }
gerrit_cl = GerritCL(data, MockGerritAPI()) gerrit_cl = GerritCL(data, MockGerritAPI())
self.assertFalse(gerrit_cl.is_exportable()) self.assertFalse(gerrit_cl.is_exportable())
...@@ -133,13 +163,17 @@ class GerritCLTest(unittest.TestCase): ...@@ -133,13 +163,17 @@ class GerritCLTest(unittest.TestCase):
'subject': 'fake subject', 'subject': 'fake subject',
'_number': 638250, '_number': 638250,
'current_revision': '1', 'current_revision': '1',
'revisions': {'1': { 'revisions': {
'commit_with_footers': 'fake subject\nNOEXPORT=true', '1': {
'files': { 'commit_with_footers': 'fake subject\nNOEXPORT=true',
RELATIVE_WEB_TESTS + 'external/wpt/foo/bar.html': '', 'files': {
RELATIVE_WEB_TESTS + 'external/wpt/foo/bar.html': '',
}
} }
}}, },
'owner': {'email': 'test@chromium.org'}, 'owner': {
'email': 'test@chromium.org'
},
} }
gerrit_cl = GerritCL(data, MockGerritAPI()) gerrit_cl = GerritCL(data, MockGerritAPI())
self.assertFalse(gerrit_cl.is_exportable()) self.assertFalse(gerrit_cl.is_exportable())
...@@ -150,13 +184,17 @@ class GerritCLTest(unittest.TestCase): ...@@ -150,13 +184,17 @@ class GerritCLTest(unittest.TestCase):
'subject': 'Import something', 'subject': 'Import something',
'_number': 638250, '_number': 638250,
'current_revision': '1', 'current_revision': '1',
'revisions': {'1': { 'revisions': {
'commit_with_footers': 'fake subject', '1': {
'files': { 'commit_with_footers': 'fake subject',
RELATIVE_WEB_TESTS + 'external/wpt/foo/bar.html': '', 'files': {
RELATIVE_WEB_TESTS + 'external/wpt/foo/bar.html': '',
}
} }
}}, },
'owner': {'email': 'test@chromium.org'}, 'owner': {
'email': 'test@chromium.org'
},
} }
gerrit_cl = GerritCL(data, MockGerritAPI()) gerrit_cl = GerritCL(data, MockGerritAPI())
self.assertTrue(gerrit_cl.is_exportable()) self.assertTrue(gerrit_cl.is_exportable())
# Copyright 2016 The Chromium Authors. All rights reserved. # Copyright 2016 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.
"""A utility class for interacting with a local checkout of the Web Platform Tests.""" """A utility class for interacting with a local checkout of the Web Platform Tests."""
import logging import logging
...@@ -19,7 +18,6 @@ _log = logging.getLogger(__name__) ...@@ -19,7 +18,6 @@ _log = logging.getLogger(__name__)
class LocalWPT(object): class LocalWPT(object):
def __init__(self, host, gh_token=None, path='/tmp/wpt'): def __init__(self, host, gh_token=None, path='/tmp/wpt'):
"""Initializes a LocalWPT instance. """Initializes a LocalWPT instance.
...@@ -47,9 +45,12 @@ class LocalWPT(object): ...@@ -47,9 +45,12 @@ class LocalWPT(object):
else: else:
remote_url = WPT_MIRROR_URL remote_url = WPT_MIRROR_URL
_log.info('No credentials given, using wpt mirror URL.') _log.info('No credentials given, using wpt mirror URL.')
_log.info('It is possible for the mirror to be delayed; see https://crbug.com/698272.') _log.info(
'It is possible for the mirror to be delayed; see https://crbug.com/698272.'
)
# Do not use self.run here because self.path doesn't exist yet. # Do not use self.run here because self.path doesn't exist yet.
self.host.executive.run_command(['git', 'clone', remote_url, self.path]) self.host.executive.run_command(
['git', 'clone', remote_url, self.path])
_log.info('Setting git user name & email in %s', self.path) _log.info('Setting git user name & email in %s', self.path)
self.run(['git', 'config', 'user.name', DEFAULT_WPT_COMMITTER_NAME]) self.run(['git', 'config', 'user.name', DEFAULT_WPT_COMMITTER_NAME])
...@@ -58,7 +59,8 @@ class LocalWPT(object): ...@@ -58,7 +59,8 @@ class LocalWPT(object):
def run(self, command, **kwargs): def run(self, command, **kwargs):
"""Runs a command in the local WPT directory.""" """Runs a command in the local WPT directory."""
# TODO(robertma): Migrate to blinkpy.common.checkout.Git. (crbug.com/676399) # TODO(robertma): Migrate to blinkpy.common.checkout.Git. (crbug.com/676399)
return self.host.executive.run_command(command, cwd=self.path, **kwargs) return self.host.executive.run_command(
command, cwd=self.path, **kwargs)
def clean(self): def clean(self):
"""Resets git to a clean state, on origin/master with no changed files.""" """Resets git to a clean state, on origin/master with no changed files."""
...@@ -66,7 +68,12 @@ class LocalWPT(object): ...@@ -66,7 +68,12 @@ class LocalWPT(object):
self.run(['git', 'clean', '-fdx']) self.run(['git', 'clean', '-fdx'])
self.run(['git', 'checkout', 'origin/master']) self.run(['git', 'checkout', 'origin/master'])
def create_branch_with_patch(self, branch_name, message, patch, author, force_push=False): def create_branch_with_patch(self,
branch_name,
message,
patch,
author,
force_push=False):
"""Commits the given patch and pushes to the upstream repo. """Commits the given patch and pushes to the upstream repo.
Args: Args:
...@@ -157,9 +164,9 @@ class LocalWPT(object): ...@@ -157,9 +164,9 @@ class LocalWPT(object):
This doesn't include the given commit, and this assumes that the given This doesn't include the given commit, and this assumes that the given
commit is on the the master branch. commit is on the the master branch.
""" """
return len(self.run([ return len(
'git', 'rev-list', '{}..origin/master'.format(commit) self.run(['git', 'rev-list',
]).splitlines()) '{}..origin/master'.format(commit)]).splitlines())
def _most_recent_log_matching(self, grep_str): def _most_recent_log_matching(self, grep_str):
"""Finds the most recent commit whose message contains the given pattern. """Finds the most recent commit whose message contains the given pattern.
...@@ -184,7 +191,8 @@ class LocalWPT(object): ...@@ -184,7 +191,8 @@ class LocalWPT(object):
A list of (SHA, commit subject) pairs ordered reverse-chronologically. A list of (SHA, commit subject) pairs ordered reverse-chronologically.
""" """
revision_range = revision_start + '..' + revision_end revision_range = revision_start + '..' + revision_end
output = self.run(['git', 'rev-list', '--pretty=oneline', revision_range]) output = self.run(
['git', 'rev-list', '--pretty=oneline', revision_range])
commits = [] commits = []
for line in output.splitlines(): for line in output.splitlines():
# Split at the first space. # Split at the first space.
...@@ -193,7 +201,10 @@ class LocalWPT(object): ...@@ -193,7 +201,10 @@ class LocalWPT(object):
def is_commit_affecting_directory(self, commit, directory): def is_commit_affecting_directory(self, commit, directory):
"""Checks if a commit affects a directory.""" """Checks if a commit affects a directory."""
exit_code = self.run(['git', 'diff-tree', '--quiet', '--no-commit-id', commit, '--', directory], exit_code = self.run([
'git', 'diff-tree', '--quiet', '--no-commit-id', commit, '--',
directory
],
return_exit_code=True) return_exit_code=True)
return exit_code == 1 return exit_code == 1
...@@ -216,4 +227,5 @@ class LocalWPT(object): ...@@ -216,4 +227,5 @@ class LocalWPT(object):
Returns: Returns:
A string of the matched commit log, empty if not found. A string of the matched commit log, empty if not found.
""" """
return self._most_recent_log_matching('^Cr-Commit-Position: %s' % commit_position) return self._most_recent_log_matching(
'^Cr-Commit-Position: %s' % commit_position)
# 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.
"""A mock LocalWPT class with canned responses.""" """A mock LocalWPT class with canned responses."""
class MockLocalWPT(object): class MockLocalWPT(object):
def __init__(self,
def __init__(self, test_patch=None, apply_patch=None, change_ids=None, commit_positions=None): test_patch=None,
apply_patch=None,
change_ids=None,
commit_positions=None):
"""Initializes the mock with pre-populated responses. """Initializes the mock with pre-populated responses.
Args: Args:
......
...@@ -13,12 +13,9 @@ from blinkpy.w3c.local_wpt import LocalWPT ...@@ -13,12 +13,9 @@ from blinkpy.w3c.local_wpt import LocalWPT
class LocalWPTTest(unittest.TestCase): class LocalWPTTest(unittest.TestCase):
def test_fetch_when_wpt_dir_exists(self): def test_fetch_when_wpt_dir_exists(self):
host = MockHost() host = MockHost()
host.filesystem = MockFileSystem(files={ host.filesystem = MockFileSystem(files={'/tmp/wpt': ''})
'/tmp/wpt': ''
})
local_wpt = LocalWPT(host, 'token') local_wpt = LocalWPT(host, 'token')
local_wpt.fetch() local_wpt.fetch()
...@@ -34,7 +31,11 @@ class LocalWPTTest(unittest.TestCase): ...@@ -34,7 +31,11 @@ class LocalWPTTest(unittest.TestCase):
local_wpt.fetch() local_wpt.fetch()
self.assertEqual(host.executive.calls, [ self.assertEqual(host.executive.calls, [
['git', 'clone', 'https://token@github.com/web-platform-tests/wpt.git', '/tmp/wpt'], [
'git', 'clone',
'https://token@github.com/web-platform-tests/wpt.git',
'/tmp/wpt'
],
['git', 'config', 'user.name', DEFAULT_WPT_COMMITTER_NAME], ['git', 'config', 'user.name', DEFAULT_WPT_COMMITTER_NAME],
['git', 'config', 'user.email', DEFAULT_WPT_COMMITTER_EMAIL], ['git', 'config', 'user.email', DEFAULT_WPT_COMMITTER_EMAIL],
]) ])
...@@ -55,20 +56,26 @@ class LocalWPTTest(unittest.TestCase): ...@@ -55,20 +56,26 @@ class LocalWPTTest(unittest.TestCase):
local_wpt = LocalWPT(host, 'token') local_wpt = LocalWPT(host, 'token')
local_wpt.fetch() local_wpt.fetch()
local_wpt.create_branch_with_patch('chromium-export-decafbad', 'message', 'patch', 'author <author@author.com>') local_wpt.create_branch_with_patch('chromium-export-decafbad',
self.assertEqual(host.executive.calls, [ 'message', 'patch',
['git', 'clone', 'https://token@github.com/web-platform-tests/wpt.git', '/tmp/wpt'], 'author <author@author.com>')
['git', 'config', 'user.name', DEFAULT_WPT_COMMITTER_NAME], self.assertEqual(
['git', 'config', 'user.email', DEFAULT_WPT_COMMITTER_EMAIL], host.executive.calls,
['git', 'reset', '--hard', 'HEAD'], [[
['git', 'clean', '-fdx'], 'git', 'clone',
['git', 'checkout', 'origin/master'], 'https://token@github.com/web-platform-tests/wpt.git',
['git', 'branch', '-D', 'chromium-export-decafbad'], '/tmp/wpt'
['git', 'checkout', '-b', 'chromium-export-decafbad'], ], ['git', 'config', 'user.name', DEFAULT_WPT_COMMITTER_NAME],
['git', 'apply', '-'], ['git', 'config', 'user.email', DEFAULT_WPT_COMMITTER_EMAIL],
['git', 'add', '.'], ['git', 'reset', '--hard', 'HEAD'], ['git', 'clean', '-fdx'],
['git', 'commit', '--author', 'author <author@author.com>', '-am', 'message'], ['git', 'checkout', 'origin/master'],
['git', 'push', 'origin', 'chromium-export-decafbad']]) ['git', 'branch', '-D', 'chromium-export-decafbad'],
['git', 'checkout', '-b', 'chromium-export-decafbad'],
['git', 'apply', '-'], ['git', 'add', '.'],
[
'git', 'commit', '--author', 'author <author@author.com>',
'-am', 'message'
], ['git', 'push', 'origin', 'chromium-export-decafbad']])
def test_test_patch_success(self): def test_test_patch_success(self):
host = MockHost() host = MockHost()
...@@ -79,7 +86,8 @@ class LocalWPTTest(unittest.TestCase): ...@@ -79,7 +86,8 @@ class LocalWPTTest(unittest.TestCase):
'reset': '', 'reset': '',
'clean': '', 'clean': '',
'checkout': '', 'checkout': '',
}, strict=True) },
strict=True)
local_wpt = LocalWPT(host, 'token') local_wpt = LocalWPT(host, 'token')
self.assertEqual(local_wpt.test_patch('dummy patch'), (True, '')) self.assertEqual(local_wpt.test_patch('dummy patch'), (True, ''))
...@@ -93,7 +101,8 @@ class LocalWPTTest(unittest.TestCase): ...@@ -93,7 +101,8 @@ class LocalWPTTest(unittest.TestCase):
'reset': '', 'reset': '',
'clean': '', 'clean': '',
'checkout': '', 'checkout': '',
}, strict=True) },
strict=True)
local_wpt = LocalWPT(host, 'token') local_wpt = LocalWPT(host, 'token')
self.assertEqual(local_wpt.test_patch('dummy patch'), (False, '')) self.assertEqual(local_wpt.test_patch('dummy patch'), (False, ''))
...@@ -108,18 +117,24 @@ class LocalWPTTest(unittest.TestCase): ...@@ -108,18 +117,24 @@ class LocalWPTTest(unittest.TestCase):
host.executive = MockExecutive(run_command_fn=_run_fn) host.executive = MockExecutive(run_command_fn=_run_fn)
local_wpt = LocalWPT(host, 'token') local_wpt = LocalWPT(host, 'token')
self.assertEqual(local_wpt.test_patch('dummy patch'), (False, 'MOCK failed applying patch')) self.assertEqual(
local_wpt.test_patch('dummy patch'),
(False, 'MOCK failed applying patch'))
def test_commits_in_range(self): def test_commits_in_range(self):
host = MockHost() host = MockHost()
host.executive = mock_git_commands({ host.executive = mock_git_commands({
'rev-list': '34ab6c3f5aee8bf05207b674edbcb6affb179545 Fix presubmit errors\n' 'rev-list':
'8c596b820634a623dfd7a2b0f36007ce2f7a0c9f test\n' '34ab6c3f5aee8bf05207b674edbcb6affb179545 Fix presubmit errors\n'
}, strict=True) '8c596b820634a623dfd7a2b0f36007ce2f7a0c9f test\n'
},
strict=True)
local_wpt = LocalWPT(host, 'token') local_wpt = LocalWPT(host, 'token')
self.assertTrue(local_wpt.commits_in_range('HEAD~2', 'HEAD')) self.assertTrue(local_wpt.commits_in_range('HEAD~2', 'HEAD'))
self.assertEqual(host.executive.calls, [['git', 'rev-list', '--pretty=oneline', 'HEAD~2..HEAD']]) self.assertEqual(
host.executive.calls,
[['git', 'rev-list', '--pretty=oneline', 'HEAD~2..HEAD']])
def test_is_commit_affecting_directory(self): def test_is_commit_affecting_directory(self):
host = MockHost() host = MockHost()
...@@ -128,19 +143,28 @@ class LocalWPTTest(unittest.TestCase): ...@@ -128,19 +143,28 @@ class LocalWPTTest(unittest.TestCase):
host.executive = mock_git_commands({'diff-tree': 1}, strict=True) host.executive = mock_git_commands({'diff-tree': 1}, strict=True)
local_wpt = LocalWPT(host, 'token') local_wpt = LocalWPT(host, 'token')
self.assertTrue(local_wpt.is_commit_affecting_directory('HEAD', 'css/')) self.assertTrue(
self.assertEqual(host.executive.calls, [['git', 'diff-tree', '--quiet', '--no-commit-id', 'HEAD', '--', 'css/']]) local_wpt.is_commit_affecting_directory('HEAD', 'css/'))
self.assertEqual(host.executive.calls, [[
'git', 'diff-tree', '--quiet', '--no-commit-id', 'HEAD', '--',
'css/'
]])
def test_seek_change_id(self): def test_seek_change_id(self):
host = MockHost() host = MockHost()
local_wpt = LocalWPT(host, 'token') local_wpt = LocalWPT(host, 'token')
local_wpt.seek_change_id('Ifake-change-id') local_wpt.seek_change_id('Ifake-change-id')
self.assertEqual(host.executive.calls, [['git', 'log', '-1', '--grep', '^Change-Id: Ifake-change-id']]) self.assertEqual(
host.executive.calls,
[['git', 'log', '-1', '--grep', '^Change-Id: Ifake-change-id']])
def test_seek_commit_position(self): def test_seek_commit_position(self):
host = MockHost() host = MockHost()
local_wpt = LocalWPT(host, 'token') local_wpt = LocalWPT(host, 'token')
local_wpt.seek_commit_position('refs/heads/master@{12345}') local_wpt.seek_commit_position('refs/heads/master@{12345}')
self.assertEqual(host.executive.calls, [['git', 'log', '-1', '--grep', '^Cr-Commit-Position: refs/heads/master@{12345}']]) self.assertEqual(host.executive.calls, [[
'git', 'log', '-1', '--grep',
'^Cr-Commit-Position: refs/heads/master@{12345}'
]])
...@@ -19,7 +19,8 @@ class MonorailIssue(object): ...@@ -19,7 +19,8 @@ class MonorailIssue(object):
_ALLOWED_FIELDS = _STRING_LIST_FIELDS | _STRING_FIELDS _ALLOWED_FIELDS = _STRING_LIST_FIELDS | _STRING_FIELDS
# Again, the list is non-exhaustive. # Again, the list is non-exhaustive.
_VALID_STATUSES = frozenset(['Unconfirmed', 'Untriaged', 'Available', 'Assigned', 'Started']) _VALID_STATUSES = frozenset(
['Unconfirmed', 'Untriaged', 'Available', 'Assigned', 'Started'])
def __init__(self, project_id, **kwargs): def __init__(self, project_id, **kwargs):
self.project_id = project_id self.project_id = project_id
...@@ -40,17 +41,20 @@ class MonorailIssue(object): ...@@ -40,17 +41,20 @@ class MonorailIssue(object):
self._body[field] = list(self._body[field]) self._body[field] = list(self._body[field])
# We expect a KeyError to be raised if 'status' is missing. # We expect a KeyError to be raised if 'status' is missing.
self._body['status'] = self._body['status'].capitalize() self._body['status'] = self._body['status'].capitalize()
assert self._body['status'] in self._VALID_STATUSES, 'Unknown status %s.' % self._body['status'] assert self._body['status'] in self._VALID_STATUSES, \
'Unknown status %s.' % self._body['status']
assert self._body['summary'], 'summary cannot be empty.' assert self._body['summary'], 'summary cannot be empty.'
def __unicode__(self): def __unicode__(self):
result = (u'Monorail issue in project {}\n' result = (u'Monorail issue in project {}\n'
'Summary: {}\n' 'Summary: {}\n'
'Status: {}\n').format(self.project_id, self.body['summary'], self.body['status']) 'Status: {}\n').format(self.project_id, self.body['summary'],
self.body['status'])
if 'cc' in self.body: if 'cc' in self.body:
result += u'CC: {}\n'.format(', '.join(self.body['cc'])) result += u'CC: {}\n'.format(', '.join(self.body['cc']))
if 'components' in self.body: if 'components' in self.body:
result += u'Components: {}\n'.format(', '.join(self.body['components'])) result += u'Components: {}\n'.format(', '.join(
self.body['components']))
if 'labels' in self.body: if 'labels' in self.body:
result += u'Labels: {}\n'.format(', '.join(self.body['labels'])) result += u'Labels: {}\n'.format(', '.join(self.body['labels']))
if 'description' in self.body: if 'description' in self.body:
...@@ -77,8 +81,7 @@ class MonorailIssue(object): ...@@ -77,8 +81,7 @@ class MonorailIssue(object):
description=description, description=description,
cc=cc or [], cc=cc or [],
components=components or [], components=components or [],
status='Untriaged' status='Untriaged')
)
@staticmethod @staticmethod
def crbug_link(issue_id): def crbug_link(issue_id):
...@@ -116,17 +119,22 @@ class MonorailAPI(object): ...@@ -116,17 +119,22 @@ class MonorailAPI(object):
# TODO(robertma): Deprecate the JSON key support once BuildBot is gone. # TODO(robertma): Deprecate the JSON key support once BuildBot is gone.
if service_account_key_json: if service_account_key_json:
credentials = self._oauth2_client.GoogleCredentials.from_stream(service_account_key_json) credentials = self._oauth2_client.GoogleCredentials.from_stream(
service_account_key_json)
elif access_token: elif access_token:
credentials = self._oauth2_client.AccessTokenCredentials( credentials = self._oauth2_client.AccessTokenCredentials(
access_token=access_token, access_token=access_token, user_agent='blinkpy/1.0')
user_agent='blinkpy/1.0')
else: else:
credentials = self._oauth2_client.GoogleCredentials.get_application_default() credentials = self._oauth2_client.GoogleCredentials.get_application_default(
)
# cache_discovery needs to be disabled because of https://github.com/google/google-api-python-client/issues/299 # cache_discovery needs to be disabled because of https://github.com/google/google-api-python-client/issues/299
self.api = self._api_discovery.build( self.api = self._api_discovery.build(
'monorail', 'v1', discoveryServiceUrl=self._DISCOVERY_URL, credentials=credentials, cache_discovery=False) 'monorail',
'v1',
discoveryServiceUrl=self._DISCOVERY_URL,
credentials=credentials,
cache_discovery=False)
@staticmethod @staticmethod
def _fix_cc_in_body(body): def _fix_cc_in_body(body):
...@@ -139,4 +147,5 @@ class MonorailAPI(object): ...@@ -139,4 +147,5 @@ class MonorailAPI(object):
def insert_issue(self, issue): def insert_issue(self, issue):
body = self._fix_cc_in_body(issue.body) body = self._fix_cc_in_body(issue.body)
return self.api.issues().insert(projectId=issue.project_id, body=body).execute() return self.api.issues().insert(
projectId=issue.project_id, body=body).execute()
...@@ -9,31 +9,42 @@ from blinkpy.w3c.monorail import MonorailAPI, MonorailIssue ...@@ -9,31 +9,42 @@ from blinkpy.w3c.monorail import MonorailAPI, MonorailIssue
class MonorailIssueTest(unittest.TestCase): class MonorailIssueTest(unittest.TestCase):
def test_init_succeeds(self): def test_init_succeeds(self):
# Minimum example. # Minimum example.
MonorailIssue('chromium', summary='test', status='Untriaged') MonorailIssue('chromium', summary='test', status='Untriaged')
# All fields. # All fields.
MonorailIssue('chromium', summary='test', status='Untriaged', description='body', MonorailIssue(
cc=['foo@chromium.org'], labels=['Flaky'], components=['Infra']) 'chromium',
summary='test',
status='Untriaged',
description='body',
cc=['foo@chromium.org'],
labels=['Flaky'],
components=['Infra'])
def test_init_fills_project_id(self): def test_init_fills_project_id(self):
issue = MonorailIssue('chromium', summary='test', status='Untriaged') issue = MonorailIssue('chromium', summary='test', status='Untriaged')
self.assertEqual(issue.body['projectId'], 'chromium') self.assertEqual(issue.body['projectId'], 'chromium')
def test_unicode(self): def test_unicode(self):
issue = MonorailIssue('chromium', summary=u'test', status='Untriaged', issue = MonorailIssue(
description=u'ABC~‾¥≈¤・・•∙·☼★星🌟星★☼·∙•・・¤≈¥‾~XYZ', 'chromium',
cc=['foo@chromium.org', 'bar@chromium.org'], labels=['Flaky'], components=['Infra']) summary=u'test',
status='Untriaged',
description=u'ABC~‾¥≈¤・・•∙·☼★星🌟星★☼·∙•・・¤≈¥‾~XYZ',
cc=['foo@chromium.org', 'bar@chromium.org'],
labels=['Flaky'],
components=['Infra'])
self.assertEqual(type(unicode(issue)), unicode) self.assertEqual(type(unicode(issue)), unicode)
self.assertEqual(unicode(issue), self.assertEqual(
(u'Monorail issue in project chromium\n' unicode(issue),
u'Summary: test\n' (u'Monorail issue in project chromium\n'
u'Status: Untriaged\n' u'Summary: test\n'
u'CC: foo@chromium.org, bar@chromium.org\n' u'Status: Untriaged\n'
u'Components: Infra\n' u'CC: foo@chromium.org, bar@chromium.org\n'
u'Labels: Flaky\n' u'Components: Infra\n'
u'Description:\nABC~‾¥≈¤・・•∙·☼★星🌟星★☼·∙•・・¤≈¥‾~XYZ\n')) u'Labels: Flaky\n'
u'Description:\nABC~‾¥≈¤・・•∙·☼★星🌟星★☼·∙•・・¤≈¥‾~XYZ\n'))
def test_init_unknown_fields(self): def test_init_unknown_fields(self):
with self.assertRaises(AssertionError): with self.assertRaises(AssertionError):
...@@ -53,15 +64,27 @@ class MonorailIssueTest(unittest.TestCase): ...@@ -53,15 +64,27 @@ class MonorailIssueTest(unittest.TestCase):
def test_init_string_passed_for_list_fields(self): def test_init_string_passed_for_list_fields(self):
with self.assertRaises(AssertionError): with self.assertRaises(AssertionError):
MonorailIssue('chromium', summary='test', status='Untriaged', cc='foo@chromium.org') MonorailIssue(
'chromium',
summary='test',
status='Untriaged',
cc='foo@chromium.org')
with self.assertRaises(AssertionError): with self.assertRaises(AssertionError):
MonorailIssue('chromium', summary='test', status='Untriaged', components='Infra') MonorailIssue(
'chromium',
summary='test',
status='Untriaged',
components='Infra')
with self.assertRaises(AssertionError): with self.assertRaises(AssertionError):
MonorailIssue('chromium', summary='test', status='Untriaged', labels='Flaky') MonorailIssue(
'chromium', summary='test', status='Untriaged', labels='Flaky')
def test_new_chromium_issue(self): def test_new_chromium_issue(self):
issue = MonorailIssue.new_chromium_issue( issue = MonorailIssue.new_chromium_issue(
'test', description='body', cc=['foo@chromium.org'], components=['Infra']) 'test',
description='body',
cc=['foo@chromium.org'],
components=['Infra'])
self.assertEqual(issue.project_id, 'chromium') self.assertEqual(issue.project_id, 'chromium')
self.assertEqual(issue.body['summary'], 'test') self.assertEqual(issue.body['summary'], 'test')
self.assertEqual(issue.body['description'], 'body') self.assertEqual(issue.body['description'], 'body')
...@@ -69,18 +92,23 @@ class MonorailIssueTest(unittest.TestCase): ...@@ -69,18 +92,23 @@ class MonorailIssueTest(unittest.TestCase):
self.assertEqual(issue.body['components'], ['Infra']) self.assertEqual(issue.body['components'], ['Infra'])
def test_crbug_link(self): def test_crbug_link(self):
self.assertEqual(MonorailIssue.crbug_link(12345), 'https://crbug.com/12345') self.assertEqual(
MonorailIssue.crbug_link(12345), 'https://crbug.com/12345')
class MonorailAPITest(unittest.TestCase): class MonorailAPITest(unittest.TestCase):
def test_fix_cc_field_in_body(self): def test_fix_cc_field_in_body(self):
original_body = { original_body = {
'summary': 'test bug', 'summary': 'test bug',
'cc': ['foo@chromium.org', 'bar@chromium.org'] 'cc': ['foo@chromium.org', 'bar@chromium.org']
} }
# pylint: disable=protected-access # pylint: disable=protected-access
self.assertEqual(MonorailAPI._fix_cc_in_body(original_body), { self.assertEqual(
'summary': 'test bug', MonorailAPI._fix_cc_in_body(original_body), {
'cc': [{'name': 'foo@chromium.org'}, {'name': 'bar@chromium.org'}] 'summary': 'test bug',
}) 'cc': [{
'name': 'foo@chromium.org'
}, {
'name': 'bar@chromium.org'
}]
})
...@@ -6,9 +6,7 @@ import logging ...@@ -6,9 +6,7 @@ import logging
from blinkpy.common.system.log_utils import configure_logging from blinkpy.common.system.log_utils import configure_logging
from blinkpy.w3c.wpt_github import WPTGitHub from blinkpy.w3c.wpt_github import WPTGitHub
from blinkpy.w3c.gerrit import GerritAPI, GerritError from blinkpy.w3c.gerrit import GerritAPI, GerritError
from blinkpy.w3c.common import ( from blinkpy.w3c.common import (read_credentials)
read_credentials
)
_log = logging.getLogger(__name__) _log = logging.getLogger(__name__)
...@@ -44,8 +42,8 @@ class PrCleanupTool(object): ...@@ -44,8 +42,8 @@ class PrCleanupTool(object):
'script may fail with a network error when making ' 'script may fail with a network error when making '
'an API request to Gerrit.') 'an API request to Gerrit.')
self.wpt_github = self.wpt_github or WPTGitHub( self.wpt_github = self.wpt_github or WPTGitHub(self.host, gh_user,
self.host, gh_user, gh_token) gh_token)
self.gerrit = self.gerrit or GerritAPI(self.host, gr_user, gr_token) self.gerrit = self.gerrit or GerritAPI(self.host, gr_user, gr_token)
pull_requests = self.retrieve_all_prs() pull_requests = self.retrieve_all_prs()
for pull_request in pull_requests: for pull_request in pull_requests:
...@@ -60,8 +58,8 @@ class PrCleanupTool(object): ...@@ -60,8 +58,8 @@ class PrCleanupTool(object):
try: try:
cl = self.gerrit.query_cl(change_id) cl = self.gerrit.query_cl(change_id)
except GerritError as e: except GerritError as e:
_log.error( _log.error('Could not query change_id %s: %s', change_id,
'Could not query change_id %s: %s', change_id, str(e)) str(e))
continue continue
cl_status = cl.status cl_status = cl.status
...@@ -80,12 +78,14 @@ class PrCleanupTool(object): ...@@ -80,12 +78,14 @@ class PrCleanupTool(object):
parser = argparse.ArgumentParser() parser = argparse.ArgumentParser()
parser.description = __doc__ parser.description = __doc__
parser.add_argument( parser.add_argument(
'-v', '--verbose', action='store_true', '-v',
'--verbose',
action='store_true',
help='log extra details that may be helpful when debugging') help='log extra details that may be helpful when debugging')
parser.add_argument( parser.add_argument(
'--credentials-json', '--credentials-json',
help='A JSON file with GitHub credentials, ' help='A JSON file with GitHub credentials, '
'generally not necessary on developer machines') 'generally not necessary on developer machines')
return parser.parse_args(argv) return parser.parse_args(argv)
def retrieve_all_prs(self): def retrieve_all_prs(self):
...@@ -104,5 +104,6 @@ class PrCleanupTool(object): ...@@ -104,5 +104,6 @@ class PrCleanupTool(object):
_log.info(comment) _log.info(comment)
_log.info('https://github.com/web-platform-tests/wpt/pull/%s', _log.info('https://github.com/web-platform-tests/wpt/pull/%s',
pull_request.number) pull_request.number)
_log.info(self.wpt_github.extract_metadata( _log.info(
'Reviewed-on: ', pull_request.body)) self.wpt_github.extract_metadata('Reviewed-on: ',
pull_request.body))
...@@ -11,7 +11,6 @@ from blinkpy.w3c.wpt_github_mock import MockWPTGitHub ...@@ -11,7 +11,6 @@ from blinkpy.w3c.wpt_github_mock import MockWPTGitHub
class PrCleanupToolTest(LoggingTestCase): class PrCleanupToolTest(LoggingTestCase):
def setUp(self): def setUp(self):
super(PrCleanupToolTest, self).setUp() super(PrCleanupToolTest, self).setUp()
host = MockHost() host = MockHost()
...@@ -28,8 +27,12 @@ class PrCleanupToolTest(LoggingTestCase): ...@@ -28,8 +27,12 @@ class PrCleanupToolTest(LoggingTestCase):
def test_main_successful_close_abandoned_cl(self): def test_main_successful_close_abandoned_cl(self):
pr_cleanup = PrCleanupTool(self.host) pr_cleanup = PrCleanupTool(self.host)
pr_cleanup.wpt_github = MockWPTGitHub(pull_requests=[ pr_cleanup.wpt_github = MockWPTGitHub(pull_requests=[
PullRequest(title='title1', number=1234, PullRequest(
body='Change-Id: 88', state='open', labels=[]), title='title1',
number=1234,
body='Change-Id: 88',
state='open',
labels=[]),
]) ])
pr_cleanup.gerrit = MockGerritAPI() pr_cleanup.gerrit = MockGerritAPI()
pr_cleanup.gerrit.cl = MockGerritCL( pr_cleanup.gerrit.cl = MockGerritCL(
...@@ -41,9 +44,13 @@ class PrCleanupToolTest(LoggingTestCase): ...@@ -41,9 +44,13 @@ class PrCleanupToolTest(LoggingTestCase):
'current_revision': '1', 'current_revision': '1',
'has_review_started': True, 'has_review_started': True,
'revisions': { 'revisions': {
'1': {'commit_with_footers': 'a commit with footers'} '1': {
'commit_with_footers': 'a commit with footers'
}
},
'owner': {
'email': 'test@chromium.org'
}, },
'owner': {'email': 'test@chromium.org'},
}, },
api=pr_cleanup.gerrit) api=pr_cleanup.gerrit)
pr_cleanup.main(['--credentials-json', '/tmp/credentials.json']) pr_cleanup.main(['--credentials-json', '/tmp/credentials.json'])
...@@ -51,13 +58,18 @@ class PrCleanupToolTest(LoggingTestCase): ...@@ -51,13 +58,18 @@ class PrCleanupToolTest(LoggingTestCase):
self.assertEqual(pr_cleanup.wpt_github.calls, [ self.assertEqual(pr_cleanup.wpt_github.calls, [
'all_pull_requests', 'all_pull_requests',
'add_comment "Close this PR because the Chromium CL has been abandoned."', 'add_comment "Close this PR because the Chromium CL has been abandoned."',
'update_pr', 'get_pr_branch', 'delete_remote_branch']) 'update_pr', 'get_pr_branch', 'delete_remote_branch'
])
def test_main_successful_close_no_exportable_changes(self): def test_main_successful_close_no_exportable_changes(self):
pr_cleanup = PrCleanupTool(self.host) pr_cleanup = PrCleanupTool(self.host)
pr_cleanup.wpt_github = MockWPTGitHub(pull_requests=[ pr_cleanup.wpt_github = MockWPTGitHub(pull_requests=[
PullRequest(title='title1', number=1234, PullRequest(
body='Change-Id: 99', state='open', labels=[]), title='title1',
number=1234,
body='Change-Id: 99',
state='open',
labels=[]),
]) ])
pr_cleanup.gerrit = MockGerritAPI() pr_cleanup.gerrit = MockGerritAPI()
pr_cleanup.gerrit.cl = MockGerritCL( pr_cleanup.gerrit.cl = MockGerritCL(
...@@ -69,12 +81,16 @@ class PrCleanupToolTest(LoggingTestCase): ...@@ -69,12 +81,16 @@ class PrCleanupToolTest(LoggingTestCase):
'current_revision': '1', 'current_revision': '1',
'has_review_started': True, 'has_review_started': True,
'revisions': { 'revisions': {
'1': {'commit_with_footers': 'a commit with footers', '1': {
'files': { 'commit_with_footers': 'a commit with footers',
RELATIVE_WEB_TESTS + 'foo/bar.html': '', 'files': {
}} RELATIVE_WEB_TESTS + 'foo/bar.html': '',
}
}
},
'owner': {
'email': 'test@chromium.org'
}, },
'owner': {'email': 'test@chromium.org'},
}, },
api=pr_cleanup.gerrit) api=pr_cleanup.gerrit)
pr_cleanup.main(['--credentials-json', '/tmp/credentials.json']) pr_cleanup.main(['--credentials-json', '/tmp/credentials.json'])
...@@ -82,14 +98,19 @@ class PrCleanupToolTest(LoggingTestCase): ...@@ -82,14 +98,19 @@ class PrCleanupToolTest(LoggingTestCase):
self.assertEqual(pr_cleanup.wpt_github.calls, [ self.assertEqual(pr_cleanup.wpt_github.calls, [
'all_pull_requests', 'all_pull_requests',
'add_comment "Close this PR because the Chromium' 'add_comment "Close this PR because the Chromium'
' CL does not have exportable changes."', ' CL does not have exportable changes."', 'update_pr',
'update_pr', 'get_pr_branch', 'delete_remote_branch']) 'get_pr_branch', 'delete_remote_branch'
])
def test_query_cl_raise_exception(self): def test_query_cl_raise_exception(self):
pr_cleanup = PrCleanupTool(self.host) pr_cleanup = PrCleanupTool(self.host)
pr_cleanup.wpt_github = MockWPTGitHub(pull_requests=[ pr_cleanup.wpt_github = MockWPTGitHub(pull_requests=[
PullRequest(title='title1', number=1234, PullRequest(
body='Change-Id: 88', state='open', labels=[]), title='title1',
number=1234,
body='Change-Id: 88',
state='open',
labels=[]),
]) ])
pr_cleanup.gerrit = MockGerritAPI(raise_error=True) pr_cleanup.gerrit = MockGerritAPI(raise_error=True)
pr_cleanup.gerrit.cl = MockGerritCL( pr_cleanup.gerrit.cl = MockGerritCL(
...@@ -101,9 +122,13 @@ class PrCleanupToolTest(LoggingTestCase): ...@@ -101,9 +122,13 @@ class PrCleanupToolTest(LoggingTestCase):
'current_revision': '1', 'current_revision': '1',
'has_review_started': True, 'has_review_started': True,
'revisions': { 'revisions': {
'1': {'commit_with_footers': 'a commit with footers'} '1': {
'commit_with_footers': 'a commit with footers'
}
},
'owner': {
'email': 'test@chromium.org'
}, },
'owner': {'email': 'test@chromium.org'},
}, },
api=pr_cleanup.gerrit) api=pr_cleanup.gerrit)
pr_cleanup.main(['--credentials-json', '/tmp/credentials.json']) pr_cleanup.main(['--credentials-json', '/tmp/credentials.json'])
......
...@@ -24,7 +24,6 @@ ...@@ -24,7 +24,6 @@
# TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF # TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
# THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
# SUCH DAMAGE. # SUCH DAMAGE.
"""Logic for converting and copying files from a W3C repo. """Logic for converting and copying files from a W3C repo.
This module is responsible for modifying and copying a subset of the tests from This module is responsible for modifying and copying a subset of the tests from
...@@ -40,12 +39,11 @@ from blinkpy.web_tests.models.typ_types import ResultType ...@@ -40,12 +39,11 @@ from blinkpy.web_tests.models.typ_types import ResultType
_log = logging.getLogger(__name__) _log = logging.getLogger(__name__)
# Directory for imported tests relative to the web tests base directory. # Directory for imported tests relative to the web tests base directory.
DEST_DIR_NAME = 'external' DEST_DIR_NAME = 'external'
class TestCopier(object):
class TestCopier(object):
def __init__(self, host, source_repo_path): def __init__(self, host, source_repo_path):
"""Initializes variables to prepare for copying and converting files. """Initializes variables to prepare for copying and converting files.
...@@ -63,10 +61,10 @@ class TestCopier(object): ...@@ -63,10 +61,10 @@ class TestCopier(object):
self.web_tests_dir = self.path_finder.web_tests_dir() self.web_tests_dir = self.path_finder.web_tests_dir()
self.destination_directory = self.filesystem.normpath( self.destination_directory = self.filesystem.normpath(
self.filesystem.join( self.filesystem.join(
self.web_tests_dir, self.web_tests_dir, DEST_DIR_NAME,
DEST_DIR_NAME,
self.filesystem.basename(self.source_repo_path))) self.filesystem.basename(self.source_repo_path)))
self.import_in_place = (self.source_repo_path == self.destination_directory) self.import_in_place = (
self.source_repo_path == self.destination_directory)
self.dir_above_repo = self.filesystem.dirname(self.source_repo_path) self.dir_above_repo = self.filesystem.dirname(self.source_repo_path)
self.import_list = [] self.import_list = []
...@@ -76,7 +74,8 @@ class TestCopier(object): ...@@ -76,7 +74,8 @@ class TestCopier(object):
self._prefixed_properties = {} self._prefixed_properties = {}
def do_import(self): def do_import(self):
_log.info('Importing %s into %s', self.source_repo_path, self.destination_directory) _log.info('Importing %s into %s', self.source_repo_path,
self.destination_directory)
self.find_importable_tests() self.find_importable_tests()
self.import_tests() self.import_tests()
...@@ -92,7 +91,7 @@ class TestCopier(object): ...@@ -92,7 +91,7 @@ class TestCopier(object):
cur_dir = root.replace(self.dir_above_repo + '/', '') + '/' cur_dir = root.replace(self.dir_above_repo + '/', '') + '/'
_log.debug('Scanning %s...', cur_dir) _log.debug('Scanning %s...', cur_dir)
dirs_to_skip = ('.git',) dirs_to_skip = ('.git', )
if dirs: if dirs:
for name in dirs_to_skip: for name in dirs_to_skip:
...@@ -114,7 +113,8 @@ class TestCopier(object): ...@@ -114,7 +113,8 @@ class TestCopier(object):
for filename in files: for filename in files:
path_full = self.filesystem.join(root, filename) path_full = self.filesystem.join(root, filename)
path_base = path_full.replace(self.source_repo_path + '/', '') path_base = path_full.replace(self.source_repo_path + '/', '')
path_base = self.destination_directory.replace(self.web_tests_dir + '/', '') + '/' + path_base path_base = self.destination_directory.replace(
self.web_tests_dir + '/', '') + '/' + path_base
if path_base in paths_to_skip: if path_base in paths_to_skip:
if self.import_in_place: if self.import_in_place:
_log.debug('Pruning: %s', path_base) _log.debug('Pruning: %s', path_base)
...@@ -126,30 +126,43 @@ class TestCopier(object): ...@@ -126,30 +126,43 @@ class TestCopier(object):
if is_basename_skipped(filename): if is_basename_skipped(filename):
_log.debug('Skipping: %s', path_full) _log.debug('Skipping: %s', path_full)
_log.debug(' Reason: This file may cause Chromium presubmit to fail.') _log.debug(
' Reason: This file may cause Chromium presubmit to fail.'
)
continue continue
copy_list.append({'src': path_full, 'dest': filename}) copy_list.append({'src': path_full, 'dest': filename})
if copy_list: if copy_list:
# Only add this directory to the list if there's something to import # Only add this directory to the list if there's something to import
self.import_list.append({'dirname': root, 'copy_list': copy_list}) self.import_list.append({
'dirname': root,
'copy_list': copy_list
})
def find_paths_to_skip(self): def find_paths_to_skip(self):
paths_to_skip = set() paths_to_skip = set()
port = self.host.port_factory.get() port = self.host.port_factory.get()
w3c_import_expectations_path = self.path_finder.path_from_web_tests('W3CImportExpectations') w3c_import_expectations_path = self.path_finder.path_from_web_tests(
w3c_import_expectations = self.filesystem.read_text_file(w3c_import_expectations_path) 'W3CImportExpectations')
expectations = TestExpectations(port, {w3c_import_expectations_path: w3c_import_expectations}) w3c_import_expectations = self.filesystem.read_text_file(
w3c_import_expectations_path)
expectations = TestExpectations(
port, {w3c_import_expectations_path: w3c_import_expectations})
# get test names that should be skipped # get test names that should be skipped
for line in expectations.get_updated_lines(w3c_import_expectations_path): for line in expectations.get_updated_lines(
w3c_import_expectations_path):
if line.is_glob: if line.is_glob:
_log.warning('W3CImportExpectations:%d Globs are not allowed in this file.' % line.lineno) _log.warning(
'W3CImportExpectations:%d Globs are not allowed in this file.'
% line.lineno)
continue continue
if ResultType.Skip in line.results: if ResultType.Skip in line.results:
if line.tags: if line.tags:
_log.warning('W3CImportExpectations:%d should not have any specifiers' % line.lineno) _log.warning(
'W3CImportExpectations:%d should not have any specifiers'
% line.lineno)
paths_to_skip.add(line.test) paths_to_skip.add(line.test)
return paths_to_skip return paths_to_skip
...@@ -162,8 +175,10 @@ class TestCopier(object): ...@@ -162,8 +175,10 @@ class TestCopier(object):
orig_path = dir_to_copy['dirname'] orig_path = dir_to_copy['dirname']
relative_dir = self.filesystem.relpath(orig_path, self.source_repo_path) relative_dir = self.filesystem.relpath(orig_path,
dest_dir = self.filesystem.join(self.destination_directory, relative_dir) self.source_repo_path)
dest_dir = self.filesystem.join(self.destination_directory,
relative_dir)
if not self.filesystem.exists(dest_dir): if not self.filesystem.exists(dest_dir):
self.filesystem.maybe_make_directory(dest_dir) self.filesystem.maybe_make_directory(dest_dir)
...@@ -177,8 +192,11 @@ class TestCopier(object): ...@@ -177,8 +192,11 @@ class TestCopier(object):
if self._prefixed_properties: if self._prefixed_properties:
_log.info('Properties needing prefixes (by count):') _log.info('Properties needing prefixes (by count):')
for prefixed_property in sorted(self._prefixed_properties, key=lambda p: self._prefixed_properties[p]): for prefixed_property in sorted(
_log.info(' %s: %s', prefixed_property, self._prefixed_properties[prefixed_property]) self._prefixed_properties,
key=lambda p: self._prefixed_properties[p]):
_log.info(' %s: %s', prefixed_property,
self._prefixed_properties[prefixed_property])
def copy_file(self, file_to_copy, dest_dir): def copy_file(self, file_to_copy, dest_dir):
"""Converts and copies a file, if it should be copied. """Converts and copies a file, if it should be copied.
...@@ -200,12 +218,14 @@ class TestCopier(object): ...@@ -200,12 +218,14 @@ class TestCopier(object):
return return
if not self.filesystem.exists(source_path): if not self.filesystem.exists(source_path):
_log.error('%s not found. Possible error in the test.', source_path) _log.error('%s not found. Possible error in the test.',
source_path)
return return
if not self.filesystem.exists(self.filesystem.dirname(dest_path)): if not self.filesystem.exists(self.filesystem.dirname(dest_path)):
if not self.import_in_place: if not self.import_in_place:
self.filesystem.maybe_make_directory(self.filesystem.dirname(dest_path)) self.filesystem.maybe_make_directory(
self.filesystem.dirname(dest_path))
relpath = self.filesystem.relpath(dest_path, self.web_tests_dir) relpath = self.filesystem.relpath(dest_path, self.web_tests_dir)
# FIXME: Maybe doing a file diff is in order here for existing files? # FIXME: Maybe doing a file diff is in order here for existing files?
......
...@@ -32,7 +32,6 @@ from blinkpy.common.system.filesystem_mock import MockFileSystem ...@@ -32,7 +32,6 @@ from blinkpy.common.system.filesystem_mock import MockFileSystem
from blinkpy.common.system.log_testing import LoggingTestCase from blinkpy.common.system.log_testing import LoggingTestCase
from blinkpy.w3c.test_copier import TestCopier from blinkpy.w3c.test_copier import TestCopier
MOCK_WEB_TESTS = '/mock-checkout/' + RELATIVE_WEB_TESTS MOCK_WEB_TESTS = '/mock-checkout/' + RELATIVE_WEB_TESTS
FAKE_SOURCE_REPO_DIR = '/blink' FAKE_SOURCE_REPO_DIR = '/blink'
...@@ -49,7 +48,6 @@ FAKE_FILES = { ...@@ -49,7 +48,6 @@ FAKE_FILES = {
class TestCopierTest(LoggingTestCase): class TestCopierTest(LoggingTestCase):
def test_import_dir_with_no_tests(self): def test_import_dir_with_no_tests(self):
host = MockHost() host = MockHost()
host.executive = MockExecutive(exception=ScriptError('error')) host.executive = MockExecutive(exception=ScriptError('error'))
...@@ -62,34 +60,34 @@ class TestCopierTest(LoggingTestCase): ...@@ -62,34 +60,34 @@ class TestCopierTest(LoggingTestCase):
host.filesystem = MockFileSystem(files=FAKE_FILES) host.filesystem = MockFileSystem(files=FAKE_FILES)
copier = TestCopier(host, FAKE_SOURCE_REPO_DIR) copier = TestCopier(host, FAKE_SOURCE_REPO_DIR)
copier.find_importable_tests() copier.find_importable_tests()
self.assertEqual( self.assertEqual(copier.import_list, [{
copier.import_list, 'copy_list': [{
[ 'dest': 'has_shebang.txt',
{ 'src': '/blink/w3c/dir/has_shebang.txt'
'copy_list': [ }, {
{'dest': 'has_shebang.txt', 'src': '/blink/w3c/dir/has_shebang.txt'}, 'dest': 'README.txt',
{'dest': 'README.txt', 'src': '/blink/w3c/dir/README.txt'} 'src': '/blink/w3c/dir/README.txt'
], }],
'dirname': '/blink/w3c/dir', 'dirname':
} '/blink/w3c/dir',
]) }])
def test_does_not_import_reftestlist_file(self): def test_does_not_import_reftestlist_file(self):
host = MockHost() host = MockHost()
host.filesystem = MockFileSystem(files=FAKE_FILES) host.filesystem = MockFileSystem(files=FAKE_FILES)
copier = TestCopier(host, FAKE_SOURCE_REPO_DIR) copier = TestCopier(host, FAKE_SOURCE_REPO_DIR)
copier.find_importable_tests() copier.find_importable_tests()
self.assertEqual( self.assertEqual(copier.import_list, [{
copier.import_list, 'copy_list': [{
[ 'dest': 'has_shebang.txt',
{ 'src': '/blink/w3c/dir/has_shebang.txt'
'copy_list': [ }, {
{'dest': 'has_shebang.txt', 'src': '/blink/w3c/dir/has_shebang.txt'}, 'dest': 'README.txt',
{'dest': 'README.txt', 'src': '/blink/w3c/dir/README.txt'} 'src': '/blink/w3c/dir/README.txt'
], }],
'dirname': '/blink/w3c/dir', 'dirname':
} '/blink/w3c/dir',
]) }])
def test_files_with_shebang_are_made_executable(self): def test_files_with_shebang_are_made_executable(self):
host = MockHost() host = MockHost()
...@@ -102,21 +100,26 @@ class TestCopierTest(LoggingTestCase): ...@@ -102,21 +100,26 @@ class TestCopierTest(LoggingTestCase):
def test_ref_test_with_ref_is_copied(self): def test_ref_test_with_ref_is_copied(self):
host = MockHost() host = MockHost()
host.filesystem = MockFileSystem(files={ host.filesystem = MockFileSystem(
'/blink/w3c/dir1/my-ref-test.html': '<html><head><link rel="match" href="ref-file.html" />test</head></html>', files={
'/blink/w3c/dir1/ref-file.html': '<html><head>test</head></html>', '/blink/w3c/dir1/my-ref-test.html':
MOCK_WEB_TESTS + 'W3CImportExpectations': '', '<html><head><link rel="match" href="ref-file.html" />test</head></html>',
}) '/blink/w3c/dir1/ref-file.html':
'<html><head>test</head></html>',
MOCK_WEB_TESTS + 'W3CImportExpectations':
'',
})
copier = TestCopier(host, FAKE_SOURCE_REPO_DIR) copier = TestCopier(host, FAKE_SOURCE_REPO_DIR)
copier.find_importable_tests() copier.find_importable_tests()
self.assertEqual( self.assertEqual(copier.import_list, [{
copier.import_list, 'copy_list': [{
[ 'src': '/blink/w3c/dir1/ref-file.html',
{ 'dest': 'ref-file.html'
'copy_list': [ },
{'src': '/blink/w3c/dir1/ref-file.html', 'dest': 'ref-file.html'}, {
{'src': '/blink/w3c/dir1/my-ref-test.html', 'dest': 'my-ref-test.html'} 'src': '/blink/w3c/dir1/my-ref-test.html',
], 'dest': 'my-ref-test.html'
'dirname': '/blink/w3c/dir1', }],
} 'dirname':
]) '/blink/w3c/dir1',
}])
...@@ -10,7 +10,11 @@ class MockWPTGitHub(object): ...@@ -10,7 +10,11 @@ class MockWPTGitHub(object):
# Some unused arguments may be included to match the real class's API. # Some unused arguments may be included to match the real class's API.
# pylint: disable=unused-argument # pylint: disable=unused-argument
def __init__(self, pull_requests, unsuccessful_merge_index=-1, create_pr_fail_index=-1, merged_index=-1): def __init__(self,
pull_requests,
unsuccessful_merge_index=-1,
create_pr_fail_index=-1,
merged_index=-1):
"""Initializes a mock WPTGitHub. """Initializes a mock WPTGitHub.
Args: Args:
...@@ -60,8 +64,8 @@ class MockWPTGitHub(object): ...@@ -60,8 +64,8 @@ class MockWPTGitHub(object):
self.calls.append('create_pr') self.calls.append('create_pr')
if self.create_pr_fail_index != self.create_pr_index: if self.create_pr_fail_index != self.create_pr_index:
self.pull_requests_created.append( self.pull_requests_created.append((remote_branch_name, desc_title,
(remote_branch_name, desc_title, body)) body))
self.create_pr_index += 1 self.create_pr_index += 1
return 5678 return 5678
......
# 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.
"""WPTManifest is responsible for handling MANIFEST.json. """WPTManifest is responsible for handling MANIFEST.json.
The MANIFEST.json file contains metadata about files in web-platform-tests, The MANIFEST.json file contains metadata about files in web-platform-tests,
...@@ -228,29 +227,37 @@ class WPTManifest(object): ...@@ -228,29 +227,37 @@ class WPTManifest(object):
# TODO(crbug.com/853815): perhaps also cache the manifest for wpt_internal. # TODO(crbug.com/853815): perhaps also cache the manifest for wpt_internal.
if 'external' in path: if 'external' in path:
base_manifest_path = fs.join(port.web_tests_dir(), 'external', BASE_MANIFEST_NAME) base_manifest_path = fs.join(port.web_tests_dir(), 'external',
BASE_MANIFEST_NAME)
if fs.exists(base_manifest_path): if fs.exists(base_manifest_path):
fs.copyfile(base_manifest_path, manifest_path) fs.copyfile(base_manifest_path, manifest_path)
else: else:
_log.error('Manifest base not found at "%s".', base_manifest_path) _log.error('Manifest base not found at "%s".',
base_manifest_path)
WPTManifest.generate_manifest(port.host, wpt_path) WPTManifest.generate_manifest(port.host, wpt_path)
if fs.isfile(manifest_path): if fs.isfile(manifest_path):
_log.debug('Manifest generation completed.') _log.debug('Manifest generation completed.')
else: else:
_log.error('Manifest generation failed; creating an empty MANIFEST.json...') _log.error(
'Manifest generation failed; creating an empty MANIFEST.json...'
)
fs.write_text_file(manifest_path, '{}') fs.write_text_file(manifest_path, '{}')
@staticmethod @staticmethod
def generate_manifest(host, dest_path): def generate_manifest(host, dest_path):
"""Generates MANIFEST.json on the specified directory.""" """Generates MANIFEST.json on the specified directory."""
finder = PathFinder(host.filesystem) finder = PathFinder(host.filesystem)
wpt_exec_path = finder.path_from_blink_tools('blinkpy', 'third_party', 'wpt', 'wpt', 'wpt') wpt_exec_path = finder.path_from_blink_tools('blinkpy', 'third_party',
cmd = ['python', wpt_exec_path, 'manifest', '--no-download', '--tests-root', dest_path] 'wpt', 'wpt', 'wpt')
cmd = [
'python', wpt_exec_path, 'manifest', '--no-download',
'--tests-root', dest_path
]
# ScriptError will be raised if the command fails. # ScriptError will be raised if the command fails.
host.executive.run_command( host.executive.run_command(
cmd, cmd,
return_stderr=True # This will also include stderr in the exception message. # This will also include stderr in the exception message.
) return_stderr=True)
...@@ -12,10 +12,8 @@ from blinkpy.web_tests.port.factory_mock import MockPortFactory ...@@ -12,10 +12,8 @@ from blinkpy.web_tests.port.factory_mock import MockPortFactory
from blinkpy.w3c.wpt_output_updater import WPTOutputUpdater from blinkpy.w3c.wpt_output_updater import WPTOutputUpdater
_EXPECTATIONS_TEST_LIST = [ _EXPECTATIONS_TEST_LIST = [
"some/test.html", "some/test.html", "external/wpt/expected_crash.html",
"external/wpt/expected_crash.html", "external/wpt/flake.html", "external/wpt/subdir/unexpected_failure.html",
"external/wpt/flake.html",
"external/wpt/subdir/unexpected_failure.html",
"passed/test.html" "passed/test.html"
] ]
...@@ -29,14 +27,14 @@ external/wpt/subdir/unexpected_failure.html [ Timeout ] ...@@ -29,14 +27,14 @@ external/wpt/subdir/unexpected_failure.html [ Timeout ]
class WPTOutputUpdaterTest(unittest.TestCase): class WPTOutputUpdaterTest(unittest.TestCase):
def setUp(self): def setUp(self):
self.host = MockHost() self.host = MockHost()
self.host.port_factory = MockPortFactory(self.host) self.host.port_factory = MockPortFactory(self.host)
self.port = self.host.port_factory.get() self.port = self.host.port_factory.get()
expectations_dict = OrderedDict() expectations_dict = OrderedDict()
expectations_dict['expectations'] = _EXPECTATIONS_FILE_STRING expectations_dict['expectations'] = _EXPECTATIONS_FILE_STRING
self.exp = TestExpectations(self.port, expectations_dict=expectations_dict) self.exp = TestExpectations(
self.port, expectations_dict=expectations_dict)
def test_update_output_json(self): def test_update_output_json(self):
"""Tests that output JSON is properly updated with expectations.""" """Tests that output JSON is properly updated with expectations."""
...@@ -76,8 +74,11 @@ class WPTOutputUpdaterTest(unittest.TestCase): ...@@ -76,8 +74,11 @@ class WPTOutputUpdaterTest(unittest.TestCase):
output_json = json.loads(output_string) output_json = json.loads(output_string)
# A few simple assertions that the original JSON is formatter properly. # A few simple assertions that the original JSON is formatter properly.
self.assertEqual("PASS", output_json["tests"]["expected_crash.html"]["expected"]) self.assertEqual(
self.assertEqual("FAIL", output_json["tests"]["subdir"]["unexpected_failure.html"]["actual"]) "PASS", output_json["tests"]["expected_crash.html"]["expected"])
self.assertEqual(
"FAIL", output_json["tests"]["subdir"]["unexpected_failure.html"]
["actual"])
# Run the output updater, and confirm the expected statuses are updated. # Run the output updater, and confirm the expected statuses are updated.
new_output_json = output_updater.update_output_json(output_json) new_output_json = output_updater.update_output_json(output_json)
...@@ -105,7 +106,8 @@ class WPTOutputUpdaterTest(unittest.TestCase): ...@@ -105,7 +106,8 @@ class WPTOutputUpdaterTest(unittest.TestCase):
# The unexpected_failure.html test had a different status than expected, # The unexpected_failure.html test had a different status than expected,
# so is_unexpected is true. Since the actual status wasn't a Pass, it's # so is_unexpected is true. Since the actual status wasn't a Pass, it's
# also a regression. # also a regression.
cur_test = new_output_json["tests"]["subdir"]["unexpected_failure.html"] cur_test = new_output_json["tests"]["subdir"][
"unexpected_failure.html"]
self.assertEqual("TIMEOUT", cur_test["expected"]) self.assertEqual("TIMEOUT", cur_test["expected"])
self.assertTrue(cur_test["is_regression"]) self.assertTrue(cur_test["is_regression"])
self.assertTrue(cur_test["is_unexpected"]) self.assertTrue(cur_test["is_unexpected"])
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