W3C test importer: improve log messages printed when skipping.

third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py

 # Copyright (C) 2013 Adobe Systems Incorporated. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions
 # are met:
 #
 # 1. Redistributions of source code must retain the above
 #    copyright notice, this list of conditions and the following
 #    disclaimer.
 # 2. Redistributions in binary form must reproduce the above
@@ skipping 37 matching lines @@
 
 _log = logging.getLogger(__name__)
 
 
 class TestCopier(object):
 
     def __init__(self, host, source_repo_path, dest_dir_name='external'):
         """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.

[qyearsley, 2017/01/28 20:27:29]

I think that in an earlier change, I started to write this docstring but then
left it incomplete.

         """
         self.host = host
 
         assert self.host.filesystem.exists(source_repo_path)
         self.source_repo_path = source_repo_path
         self.dest_dir_name = dest_dir_name
 
         self.filesystem = self.host.filesystem
         self.webkit_finder = WebKitFinder(self.filesystem)
         self._webkit_root = self.webkit_finder.webkit_base()
@@ skipping 20 matching lines @@
     def find_importable_tests(self):
         """Walks through the source directory to find what tests should be imported.
 
         This function sets self.import_list, which contains information about how many
         tests are being imported, and their source and destination paths.
         """
         paths_to_skip = self.find_paths_to_skip()
 
         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
 
             # Files in 'tools' are not for browser testing, so we skip them.
             # See: http://testthewebforward.org/docs/test-format-guidelines.html#tools
             dirs_to_skip = ('.git', 'test-plan', 'tools')
 
             # We copy all files in 'support', including HTML without metadata.
             # See: http://testthewebforward.org/docs/test-format-guidelines.html#support-files
             dirs_to_include = ('resources', 'support')
 
             if dirs:
                 for name in dirs_to_skip:
                     if name in dirs:
                         dirs.remove(name)
 
                 for path in paths_to_skip:
                     path_base = path.replace(self.dest_dir_name + '/', '')
                     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 = []
 
             for filename in files:
                 path_full = self.filesystem.join(root, filename)
                 path_base = path_full.replace(self.source_repo_path + '/', '')
                 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:
                         continue
                 # 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)
 
                 mimetype = mimetypes.guess_type(fullpath)
                 if ('html' not in str(mimetype[0]) and
                         'application/xhtml+xml' not in str(mimetype[0]) and
                         'application/xml' not in str(mimetype[0])):
                     copy_list.append({'src': fullpath, 'dest': filename})
                     continue
 
                 if self.filesystem.basename(root) in dirs_to_include:
                     copy_list.append({'src': fullpath, 'dest': filename})
                     continue
 
                 test_parser = TestParser(fullpath, self.host)
                 test_info = test_parser.analyze_test()
                 if test_info is None:
                     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.
                     # FIXME: Ideally we'd support reading the metadata
                     # directly rather than relying on a naming convention.
                     # Using a naming convention creates duplicate copies of the
                     # reference files (http://crrev.com/268729).
                     ref_file = self.filesystem.splitext(test_basename)[0] + '-expected'
                     # Make sure to use the extension from the *reference*, not
                     # from the test, because at least flexbox tests use XHTML
                     # references but HTML tests.
                     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
                     total_tests += 1
                     copy_list.append({'src': test_info['reference'], 'dest': ref_file,
                                       'reference_support_info': test_info['reference_support_info']})
                     copy_list.append({'src': test_info['test'], 'dest': filename})
 
                 elif 'jstest' in test_info.keys():
                     jstests += 1
@@ skipping 138 matching lines @@
         """Checks whether a source path is too long to import.
 
         Args:
             Absolute path of file to be imported.
 
         Returns:
             True if the path is too long to import, False if it's OK.
         """
         path_from_repo_base = os.path.relpath(source_path, self.source_repo_path)
         return len(path_from_repo_base) > MAX_PATH_LENGTH