Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2651673017:
W3C test importer: improve log messages printed when skipping. (Closed)
DescriptionW3C test importer: improve log messages printed when skipping.
BUG=684029
Review-Url: https://codereview.chromium.org/2651673017
Cr-Commit-Position: refs/heads/master@{#446942}
Committed: https://chromium.googlesource.com/chromium/src/+/c8c6ddd28bc30527059f4e5a0aebf42ce2f946cd
Patch Set 1 #Patch Set 2 : Add back reason, change debug to info #
Total comments: 1
Patch Set 3 : Rebased #
Messages
Total messages: 23 (17 generated)
qyearsley@chromium.org changed reviewers: + jeffcarp@chromium.org
https://codereview.chromium.org/2651673017/diff/20001/third_party/WebKit/Tool... File third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py (right): https://codereview.chromium.org/2651673017/diff/20001/third_party/WebKit/Tool... third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py:63: TODO(qyearsley): This can be made into a constant. I think that in an earlier change, I started to write this docstring but then left it incomplete.
The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by qyearsley@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for
third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py:
While running git apply --index -p1;
error: patch failed:
third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py:142
error: third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py: patch
does not apply
Patch: third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py
Index: third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py
b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py
index
1ac7f272e4b68536e4131b3755fd7506101b5b49..8eb374cdcc7f07ba88269678e448a497f6206b0b
100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py
@@ -55,7 +55,12 @@ class TestCopier(object):
"""Initializes variables to prepare for copying and converting files.
Args:
- source_repo_path: Path to the local checkout of a WPT
+ host: An instance of Host.
+ source_repo_path: Path to the local checkout of a
+ web-platform-tests or csswg-test repository.
+ dest_dir_name: The name of the directory under the layout tests
+ directory where imported tests should be copied to.
+ TODO(qyearsley): This can be made into a constant.
"""
self.host = host
@@ -96,7 +101,7 @@ class TestCopier(object):
for root, dirs, files in self.filesystem.walk(self.source_repo_path):
cur_dir = root.replace(self.dir_above_repo + '/', '') + '/'
- _log.debug(' scanning ' + cur_dir + '...')
+ _log.debug('Scanning %s...', cur_dir)
total_tests = 0
reftests = 0
jstests = 0
@@ -119,12 +124,10 @@ class TestCopier(object):
path_base = path_base.replace(cur_dir, '')
path_full = self.filesystem.join(root, path_base)
if path_base in dirs:
+ _log.info('Skipping: %s', path_full)
dirs.remove(path_base)
if self.import_in_place:
- _log.debug(' pruning %s', path_base)
self.filesystem.rmtree(path_full)
- else:
- _log.debug(' skipping %s', path_base)
copy_list = []
@@ -134,7 +137,7 @@ class TestCopier(object):
path_base =
self.destination_directory.replace(self.layout_tests_dir + '/', '') + '/' +
path_base
if path_base in paths_to_skip:
if self.import_in_place:
- _log.debug(' pruning %s', path_base)
+ _log.debug('Pruning: %s', path_base)
self.filesystem.remove(path_full)
continue
else:
@@ -142,11 +145,17 @@ class TestCopier(object):
# FIXME: This block should really be a separate function, but
the early-continues make that difficult.
if filename.startswith('.') or filename.endswith('.pl'):
- # The w3cs repos may contain perl scripts, which we don't
care about.
+ _log.info('Skipping: %s', path_full)
+ _log.info(' Reason: Hidden files and perl scripts are not
necessary.')
continue
if filename == 'OWNERS' or filename == 'reftest.list':
- # These files fail our presubmits.
# See http://crbug.com/584660 and http://crbug.com/582838.
+ _log.info('Skipping: %s', path_full)
+ _log.info(' Reason: This file may cause Chromium presubmit
to fail.')
+ continue
+ if self.path_too_long(path_full):
+ _log.warning('Skipping: %s', path_full)
+ _log.warning(' Reason: Long path. Max length %d; see
http://crbug.com/609871.', MAX_PATH_LENGTH)
continue
fullpath = self.filesystem.join(root, filename)
@@ -168,12 +177,6 @@ class TestCopier(object):
copy_list.append({'src': fullpath, 'dest': filename})
continue
- if self.path_too_long(path_full):
- _log.warning('%s skipped due to long path. '
- 'Max length from repo base %d chars; see
http://crbug.com/609871.',
- path_full, MAX_PATH_LENGTH)
- continue
-
if 'reference' in test_info.keys():
test_basename = self.filesystem.basename(test_info['test'])
# Add the ref file, following WebKit style.
@@ -188,14 +191,14 @@ class TestCopier(object):
ref_file +=
self.filesystem.splitext(test_info['reference'])[1]
if not self.filesystem.exists(test_info['reference']):
- _log.warning('%s skipped because ref file %s was not
found.',
- path_full, ref_file)
+ _log.warning('Skipping: %s', path_full)
+ _log.warning(' Reason: Ref file "%s" was not found.',
ref_file)
continue
if self.path_too_long(path_full.replace(filename,
ref_file)):
- _log.warning('%s skipped because path of ref file %s
would be too long. '
- 'Max length from repo base %d chars; see
http://crbug.com/609871.',
- path_full, ref_file, MAX_PATH_LENGTH)
+ _log.warning('Skipping: %s', path_full)
+ _log.warning(' Reason: Ref file path length would be
too long: %s.', ref_file)
+ _log.warning(' Max length %d; see
http://crbug.com/609871.', MAX_PATH_LENGTH)
continue
reftests += 1
The CQ bit was checked by qyearsley@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jeffcarp@chromium.org Link to the patchset: https://codereview.chromium.org/2651673017/#ps40001 (title: "Rebased")
The CQ bit was unchecked by qyearsley@chromium.org
The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by qyearsley@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485713340468660,
"parent_rev": "e98c3a6945a7e0c4daca46054fa0290e047a2896", "commit_rev":
"c8c6ddd28bc30527059f4e5a0aebf42ce2f946cd"}
Message was sent while issue was closed.
Description was changed from ========== W3C test importer: improve log messages printed when skipping. BUG=684029 ========== to ========== W3C test importer: improve log messages printed when skipping. BUG=684029 Review-Url: https://codereview.chromium.org/2651673017 Cr-Commit-Position: refs/heads/master@{#446942} Committed: https://chromium.googlesource.com/chromium/src/+/c8c6ddd28bc30527059f4e5a0aeb... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/c8c6ddd28bc30527059f4e5a0aeb... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
