[blinkpy] Fix virtual tests discovery on Windows

On Windows, when a partial path to a virtual suite (or tests) is given,
the runner may fail to find all the tests because it adds an
OS-dependent path separator ('\' on Windows) instead of TEST_SEPARATOR
(always '/') to the given path.

Also fixes some code-health issues (unused imports & pylint warnings).

Bug: 880609
Change-Id: I2f5ba3f0864c164432d7b81379c1b81719ec7b79
Reviewed-on: https://chromium-review.googlesource.com/c/1437708
Auto-Submit: Robert Ma <robertma@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626408}

third_party/blink/tools/blinkpy/web_tests/port/base.py

@@ -33,11 +33,9 @@ in the web test infrastructure.
 """
 
 import collections
-import errno
 import json
 import logging
 import optparse
-import os
 import re
 import sys
 import tempfile
@@ -47,7 +45,6 @@ from blinkpy.common import find_files
 from blinkpy.common import read_checksum_from_png
 from blinkpy.common.memoized import memoized
 from blinkpy.common.path_finder import PathFinder
-from blinkpy.common.system.executive import ScriptError
 from blinkpy.common.system.path import abspath_to_uri
 from blinkpy.w3c.wpt_manifest import WPTManifest
 from blinkpy.web_tests.layout_package.bot_test_expectations import BotTestExpectationsFactory
@@ -260,8 +257,8 @@ class Port(object):
         # increases test run time by 2-5X, but provides more consistent results
         # [less state leaks between tests].
         if (self.get_option('reset_shell_between_tests') or
-            self.get_option('repeat_each') > 1 or
-            self.get_option('iterations') > 1):
+                self.get_option('repeat_each') > 1 or
+                self.get_option('iterations') > 1):
             flags += ['--reset-shell-between-tests']
         return flags
 
@@ -1588,8 +1585,10 @@ class Port(object):
 
     def _virtual_tests_matching_paths(self, paths, suites):
         tests = []
+        paths_with_trailing_slash = [p.rstrip(self.TEST_PATH_SEPARATOR) + self.TEST_PATH_SEPARATOR for p in paths]
         for suite in suites:
-            if any(p.startswith(suite.name) for p in paths) or any(suite.name.startswith(os.path.join(p, '')) for p in paths):
+            if (any(p.startswith(suite.name) for p in paths) or
+                    any(suite.name.startswith(p) for p in paths_with_trailing_slash)):
                 self._populate_virtual_suite(suite)
             for test in suite.tests:
                 if any(test.startswith(p) for p in paths):
@@ -1733,8 +1732,8 @@ class Port(object):
         for (font_dirs, font_file, package) in FONT_FILES:
             exists = False
             for font_dir in font_dirs:
-                font_path = os.path.join(font_dir, font_file)
-                if not os.path.isabs(font_path):
+                font_path = self._filesystem.join(font_dir, font_file)
+                if not self._filesystem.isabs(font_path):
                     font_path = self._build_path(font_path)
                 if self._check_file_exists(font_path, '', more_logging=False):
                     result.append(font_path)

third_party/blink/tools/blinkpy/web_tests/port/base_unittest.py

@@ -31,16 +31,13 @@ import json
 import optparse
 import unittest
 
-from blinkpy.common.path_finder import PathFinder
 from blinkpy.common.path_finder import RELATIVE_WEB_TESTS
-from blinkpy.common.system.executive import ScriptError
 from blinkpy.common.system.executive_mock import MockExecutive
 from blinkpy.common.system.log_testing import LoggingTestCase
 from blinkpy.common.system.output_capture import OutputCapture
 from blinkpy.common.system.platform_info_mock import MockPlatformInfo
 from blinkpy.common.system.system_host import SystemHost
 from blinkpy.common.system.system_host_mock import MockSystemHost
-from blinkpy.web_tests.models.test_input import TestInput
 from blinkpy.web_tests.port.base import Port, VirtualTestSuite
 from blinkpy.web_tests.port.test import add_unit_tests_to_mock_filesystem, WEB_TEST_DIR, TestPort
 
@@ -297,7 +294,7 @@ class PortTest(LoggingTestCase):
         test_file = 'fast/test.html'
 
         # Simple additional platform directory
-        port._options.additional_platform_directory = ['/tmp/local-baselines']
+        port._options.additional_platform_directory = ['/tmp/local-baselines']  # pylint: disable=protected-access
         self.assertEqual(port.baseline_version_dir(), '/tmp/local-baselines')
 
         self.assertEqual(port.expected_baselines(test_file, '.txt'),
@@ -313,7 +310,7 @@ class PortTest(LoggingTestCase):
                          '/tmp/local-baselines/fast/test-expected.txt')
 
         # Multiple additional platform directories
-        port._options.additional_platform_directory = ['/foo', '/tmp/local-baselines']
+        port._options.additional_platform_directory = ['/foo', '/tmp/local-baselines']  # pylint: disable=protected-access
         self.assertEqual(port.baseline_version_dir(), '/foo')
 
         self.assertEqual(port.expected_baselines(test_file, '.txt'),
@@ -349,6 +346,7 @@ class PortTest(LoggingTestCase):
 
         self.assertEqual('\n'.join(port.expectations_dict().values()), '')
 
+        # pylint: disable=protected-access
         port._options.additional_expectations = [
             '/tmp/additional-expectations-1.txt']
         self.assertEqual('\n'.join(port.expectations_dict().values()), 'content1\n')
@@ -777,6 +775,11 @@ class PortTest(LoggingTestCase):
         self.assertIn('passes/virtual_passes/test-virtual-passes.html', tests)
         self.assertNotIn('virtual/virtual_passes/passes/text.html', tests)
 
+        # crbug.com/880609: test trailing slashes
+        tests = port.tests(['virtual/virtual_passes'])
+        self.assertIn('virtual/virtual_passes/passes/test-virtual-passes.html', tests)
+        self.assertIn('virtual/virtual_passes/passes_two/test-virtual-passes.html', tests)
+
         tests = port.tests(['virtual/virtual_passes/'])
         self.assertIn('virtual/virtual_passes/passes/test-virtual-passes.html', tests)
         self.assertIn('virtual/virtual_passes/passes_two/test-virtual-passes.html', tests)
@@ -915,6 +918,7 @@ class NaturalCompareTest(unittest.TestCase):
         self._port = TestPort(MockSystemHost())
 
     def assert_cmp(self, x, y, result):
+        # pylint: disable=protected-access
         self.assertEqual(cmp(self._port._natural_sort_key(x), self._port._natural_sort_key(y)), result)
 
     def test_natural_compare(self):