Remove reference args for VirtualTestSuite

It was added several years ago but hasn't been used after its first use.
Remove to simplify code.

Change-Id: I9d7a18758b720ebad90624353da83659e68f0112
Bug: 1014162
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1881983
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710383}

third_party/blink/tools/blinkpy/web_tests/controllers/single_test_runner.py

@@ -41,12 +41,8 @@ from blinkpy.web_tests.models import testharness_results
 _log = logging.getLogger(__name__)
 
 
-def run_single_test(
-        port, options, results_directory, worker_name, primary_driver,
-        secondary_driver, test_input):
-    runner = SingleTestRunner(
-        port, options, results_directory, worker_name, primary_driver,
-        secondary_driver, test_input)
+def run_single_test(port, options, results_directory, worker_name, driver, test_input):
+    runner = SingleTestRunner(port, options, results_directory, worker_name, driver, test_input)
     try:
         test_result =  runner.run()
         test_result.create_artifacts()
@@ -57,14 +53,12 @@ def run_single_test(
 
 
 class SingleTestRunner(object):
-    def __init__(self, port, options, results_directory, worker_name,
-                 primary_driver, secondary_driver, test_input):
+    def __init__(self, port, options, results_directory, worker_name, driver, test_input):
         self._port = port
         self._filesystem = port.host.filesystem
         self._options = options
         self._results_directory = results_directory
-        self._driver = primary_driver
-        self._reference_driver = primary_driver
+        self._driver = driver
         self._timeout_ms = test_input.timeout_ms
         self._worker_name = worker_name
         self._test_name = test_input.test_name
@@ -80,14 +74,6 @@ class SingleTestRunner(object):
         TestResult.results_directory = self._results_directory
         TestResult.filesystem = port.host.filesystem
 
-        # If this is a virtual test that uses the default flags instead of the
-        # virtual flags for it's references, run it on the secondary driver so
-        # that the primary driver does not need to be restarted.
-        if (secondary_driver and
-                self._port.is_virtual_test(self._test_name) and
-                not self._port.lookup_virtual_reference_args(self._test_name)):
-            self._reference_driver = secondary_driver
-
     def _expected_driver_output(self):
         return DriverOutput(self._port.expected_text(self._test_name),
                             self._port.expected_image(self._test_name),
@@ -571,9 +557,7 @@ class SingleTestRunner(object):
         expected_output = None
         reference_test_names = []
         reftest_failures = []
-        args = self._port.lookup_virtual_reference_args(self._test_name)
-        if not args:
-            args = self._port.lookup_physical_reference_args(self._test_name)
+        args = self._port.args_for_test(self._test_name)
 
         # sort self._reference_files to put mismatch tests first
         for expectation, reference_filename in sorted(self._reference_files):
@@ -581,7 +565,7 @@ class SingleTestRunner(object):
             reference_test_names.append(reference_test_name)
             driver_input = DriverInput(reference_test_name, self._timeout_ms,
                                        image_hash=test_output.image_hash, args=args)
-            expected_output = self._reference_driver.run_test(driver_input)
+            expected_output = self._driver.run_test(driver_input)
             total_test_time += expected_output.test_time
             reftest_failures = self._compare_output_with_reference(
                 expected_output, test_output, reference_filename, expectation == '!=')

third_party/blink/tools/blinkpy/web_tests/controllers/web_test_runner.py

@@ -244,8 +244,7 @@ class Worker(object):
         self._port = None
         self._batch_count = None
         self._filesystem = None
-        self._primary_driver = None
-        self._secondary_driver = None
+        self._driver = None
         self._num_tests = 0
 
     def __del__(self):
@@ -259,11 +258,7 @@ class Worker(object):
         self._host = self._caller.host
         self._filesystem = self._host.filesystem
         self._port = self._host.port_factory.get(self._options.platform, self._options)
-        self._primary_driver = self._port.create_driver(self._worker_number)
-
-        if self._port.max_drivers_per_process() > 1:
-            self._secondary_driver = self._port.create_driver(self._worker_number)
-
+        self._driver = self._port.create_driver(self._worker_number)
         self._batch_count = 0
 
     def handle(self, name, source, test_list_name, test_inputs, batch_size):
@@ -275,9 +270,6 @@ class Worker(object):
                 self._caller.stop_running()
                 return
 
-        # Kill the secondary driver at the end of each test shard.
-        self._kill_driver(self._secondary_driver, 'secondary')
-
         self._caller.post('finished_test_list', test_list_name)
 
     def _update_test_input(self, test_input):
@@ -286,10 +278,9 @@ class Worker(object):
             test_input.reference_files = self._port.reference_files(test_input.test_name)
 
     def _run_test(self, test_input, shard_name, batch_size):
-        # If the batch size has been exceeded, kill the drivers.
+        # If the batch size has been exceeded, kill the driver.
         if batch_size > 0 and self._batch_count >= batch_size:
-            self._kill_driver(self._primary_driver, 'primary')
-            self._kill_driver(self._secondary_driver, 'secondary')
+            self._kill_driver()
             self._batch_count = 0
 
         self._batch_count += 1
@@ -301,7 +292,7 @@ class Worker(object):
         self._caller.post('started_test', test_input)
         result = single_test_runner.run_single_test(
             self._port, self._options, self._results_directory, self._name,
-            self._primary_driver, self._secondary_driver, test_input)
+            self._driver, test_input)
 
         result.shard_name = shard_name
         result.worker_name = self._name
@@ -314,25 +305,24 @@ class Worker(object):
 
     def stop(self):
         _log.debug('%s cleaning up', self._name)
-        self._kill_driver(self._primary_driver, 'primary')
-        self._kill_driver(self._secondary_driver, 'secondary')
+        self._kill_driver()
 
-    def _kill_driver(self, driver, label):
+    def _kill_driver(self):
         # Be careful about how and when we kill the driver; if driver.stop()
         # raises an exception, this routine may get re-entered via __del__.
-        if driver:
+        if self._driver:
             # When tracing we need to go through the standard shutdown path to
             # ensure that the trace is recorded properly.
             if any(i in ['--trace-startup', '--trace-shutdown']
                    for i in self._options.additional_driver_flag):
                 _log.debug('%s waiting %d seconds for %s driver to shutdown',
                            self._name, self._port.driver_stop_timeout(), label)
-                driver.stop(timeout_secs=self._port.driver_stop_timeout())
+                self._driver.stop(timeout_secs=self._port.driver_stop_timeout())
                 return
 
             # Otherwise, kill the driver immediately to speed up shutdown.
-            _log.debug('%s killing %s driver', self._name, label)
-            driver.stop()
+            _log.debug('%s killing driver', self._name)
+            self._driver.stop()
 
     def _clean_up_after_test(self, test_input, result):
         test_description = test_input.test_name
@@ -345,9 +335,8 @@ class Worker(object):
             if any([f.driver_needs_restart() for f in result.failures]):
                 # FIXME: Need more information in failure reporting so
                 # we know which driver needs to be restarted. For now
-                # we kill both drivers.
-                self._kill_driver(self._primary_driver, 'primary')
-                self._kill_driver(self._secondary_driver, 'secondary')
+                # we kill the driver.
+                self._kill_driver()
 
                 # Reset the batch count since the shell just bounced.
                 self._batch_count = 0

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

@@ -307,11 +307,6 @@ class AndroidPort(base.Port):
                                                     'Unable to find any attached Android devices.')
         return len(usable_devices)
 
-    def max_drivers_per_process(self):
-        # Android falls over when we try to run multiple content_shells per worker.
-        # See https://codereview.chromium.org/1158323009/
-        return 1
-
     def check_build(self, needs_http, printer):
         exit_status = super(AndroidPort, self).check_build(needs_http, printer)
         if exit_status:

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

@@ -321,10 +321,6 @@ class Port(object):
         """Returns the number of child processes to use for this port."""
         return self._executive.cpu_count()
 
-    def max_drivers_per_process(self):
-        """Returns the maximum number of drivers a child process can use for this port."""
-        return 2
-
     def default_max_locked_shards(self):
         """Returns the number of "locked" shards to run in parallel (like the http tests)."""
         max_locked_shards = int(self.default_child_processes()) / 4
@@ -1051,10 +1047,10 @@ class Port(object):
 
     @memoized
     def args_for_test(self, test_name):
-        virtual_args = self.lookup_virtual_test_args(test_name)
+        virtual_args = self._lookup_virtual_test_args(test_name)
         if virtual_args:
             return virtual_args
-        return self.lookup_physical_test_args(test_name)
+        return self._lookup_physical_test_args(test_name)
 
     @memoized
     def name_for_test(self, test_name):
@@ -1694,17 +1690,14 @@ class Port(object):
             for test in base_tests:
                 suite.tests[suite.full_prefix + test] = test
 
-    def is_virtual_test(self, test_name):
-        return bool(self.lookup_virtual_suite(test_name))
-
-    def lookup_virtual_suite(self, test_name):
+    def _lookup_virtual_suite(self, test_name):
         for suite in self.virtual_test_suites():
             if test_name.startswith(suite.full_prefix):
                 return suite
         return None
 
     def lookup_virtual_test_base(self, test_name):
-        suite = self.lookup_virtual_suite(test_name)
+        suite = self._lookup_virtual_suite(test_name)
         if not suite:
             return None
         assert test_name.startswith(suite.full_prefix)
@@ -1715,31 +1708,19 @@ class Port(object):
                 return maybe_base
         return None
 
-    def lookup_virtual_test_args(self, test_name):
+    def _lookup_virtual_test_args(self, test_name):
         normalized_test_name = self.normalize_test_name(test_name)
         for suite in self.virtual_test_suites():
             if normalized_test_name.startswith(suite.full_prefix):
                 return suite.args
         return []
 
-    def lookup_virtual_reference_args(self, test_name):
-        for suite in self.virtual_test_suites():
-            if test_name.startswith(suite.full_prefix):
-                return suite.reference_args
-        return []
-
-    def lookup_physical_test_args(self, test_name):
+    def _lookup_physical_test_args(self, test_name):
         for suite in self.physical_test_suites():
             if test_name.startswith(suite.name):
                 return suite.args
         return []
 
-    def lookup_physical_reference_args(self, test_name):
-        for suite in self.physical_test_suites():
-            if test_name.startswith(suite.name):
-                return suite.reference_args
-        return []
-
     def _build_path(self, *comps):
         """Returns a path from the build directory."""
         return self._build_path_with_target(self._options.target, *comps)
@@ -1836,7 +1817,7 @@ class Port(object):
 
 class VirtualTestSuite(object):
 
-    def __init__(self, prefix=None, bases=None, args=None, references_use_default_args=False):
+    def __init__(self, prefix=None, bases=None, args=None):
         assert isinstance(bases, list)
         assert args
         assert isinstance(args, list)
@@ -1844,21 +1825,19 @@ class VirtualTestSuite(object):
         self.full_prefix = 'virtual/' + prefix + '/'
         self.bases = bases
         self.args = args
-        self.reference_args = [] if references_use_default_args else args
         self.tests = {}
 
     def __repr__(self):
-        return "VirtualTestSuite('%s', %s, %s, %s)" % (self.full_prefix, self.bases, self.args, self.reference_args)
+        return "VirtualTestSuite('%s', %s, %s)" % (self.full_prefix, self.bases, self.args)
 
 
 class PhysicalTestSuite(object):
 
-    def __init__(self, base, args, reference_args=None):
+    def __init__(self, base, args):
         self.name = base
         self.base = base
         self.args = args
-        self.reference_args = args if reference_args is None else reference_args
         self.tests = set()
 
     def __repr__(self):
-        return "PhysicalTestSuite('%s', '%s', %s, %s)" % (self.name, self.base, self.args, self.reference_args)
+        return "PhysicalTestSuite('%s', '%s', %s)" % (self.name, self.base, self.args)

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

@@ -946,10 +946,6 @@ class PortTest(LoggingTestCase):
         # 'failures/expected' is not a specified base of virtual/virtual_failures
         self.assertIsNone(port.lookup_virtual_test_base('virtual/virtual_failures/failures/expected/image.html'))
 
-        # Matching base file.
-        self.assertEqual('passes/reftest.html', port.lookup_virtual_test_base('virtual/references_use_default_args/passes/reftest.html'))
-        self.assertEqual('passes/', port.lookup_virtual_test_base('virtual/references_use_default_args/passes'))
-
         # Partial match of base with multiple levels.
         self.assertEqual('failures/', port.lookup_virtual_test_base('virtual/virtual_failures/failures/'))
         self.assertEqual('failures/', port.lookup_virtual_test_base('virtual/virtual_failures/failures'))
@@ -1123,24 +1119,12 @@ class VirtualTestSuiteTest(unittest.TestCase):
         self.assertEqual(suite.full_prefix, 'virtual/suite/')
         self.assertEqual(suite.bases, ['base/foo', 'base/bar'])
         self.assertEqual(suite.args, ['--args'])
-        self.assertEqual(suite.reference_args, suite.args)
 
     def test_empty_bases(self):
         suite = VirtualTestSuite(prefix='suite', bases=[], args=['--args'])
         self.assertEqual(suite.full_prefix, 'virtual/suite/')
         self.assertEqual(suite.bases, [])
         self.assertEqual(suite.args, ['--args'])
-        self.assertEqual(suite.reference_args, suite.args)
-
-    def test_default_reference_args(self):
-        suite = VirtualTestSuite(prefix='suite', bases=['base/foo'], args=['--args'], references_use_default_args=True)
-        self.assertEqual(suite.args, ['--args'])
-        self.assertEqual(suite.reference_args, [])
-
-    def test_non_default_reference_args(self):
-        suite = VirtualTestSuite(prefix='suite', bases=['base/foo'], args=['--args'], references_use_default_args=False)
-        self.assertEqual(suite.args, ['--args'])
-        self.assertEqual(suite.reference_args, suite.args)
 
     def test_no_slash(self):
         self.assertRaises(AssertionError, VirtualTestSuite, prefix='suite/bar', bases=['base/foo'], args=['--args'])

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

@@ -123,11 +123,7 @@ class MockDRTPort(object):
         env['PATH'] = self.host.environ.get('PATH')
         return env
 
-    def lookup_virtual_test_args(self, test_name):
-        # MockDRTPort doesn't support virtual test suites.
-        raise NotImplmentedError()
-
-    def lookup_virtual_reference_args(self, test_name):
+    def _lookup_virtual_test_args(self, test_name):
         # MockDRTPort doesn't support virtual test suites.
         raise NotImplmentedError()
 

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

@@ -116,7 +116,7 @@ class TestList(object):
 #
 # These numbers may need to be updated whenever we add or delete tests. This includes virtual tests.
 #
-TOTAL_TESTS = 153
+TOTAL_TESTS = 152
 TOTAL_WONTFIX = 3
 TOTAL_SKIPS = 20 + TOTAL_WONTFIX
 TOTAL_CRASHES = 78
@@ -574,8 +574,6 @@ class TestPort(Port):
             VirtualTestSuite(prefix='virtual_passes', bases=['passes', 'passes_two'], args=['--virtual-arg']),
             VirtualTestSuite(prefix='skipped', bases=['failures/expected'], args=['--virtual-arg-skipped']),
             VirtualTestSuite(prefix='virtual_failures', bases=['failures/unexpected'], args=['--virtual-arg-failures']),
-            VirtualTestSuite(prefix='references_use_default_args', bases=['passes/reftest.html'],
-                             args=['--virtual-arg-reftest'], references_use_default_args=True),
             VirtualTestSuite(prefix='virtual_wpt', bases=['external/wpt'], args=['--virtual-arg-wpt']),
             VirtualTestSuite(prefix='virtual_wpt_dom', bases=['external/wpt/dom', 'wpt_internal/dom'], args=['--virtual-arg-wpt-dom']),
             VirtualTestSuite(prefix='virtual_empty_bases', bases=[], args=['--virtual-arg-empty-bases']),

third_party/blink/tools/blinkpy/web_tests/run_web_tests_unittest.py

@@ -1263,19 +1263,7 @@ class RunTest(unittest.TestCase, StreamTestingMixin):
     def test_reftest_with_virtual_reference(self):
         _, err, _ = logging_run(['--details', 'virtual/virtual_passes/passes/reftest.html'], tests_included=True)
         self.assertTrue('ref: virtual/virtual_passes/passes/reftest-expected.html' in err.getvalue())
-        self.assertTrue(
-            re.search(r'args: --virtual-arg\s*reference_args: --virtual-arg\s*ref:', err.getvalue()))
-
-    def test_reftest_virtual_references_use_default_args(self):
-        test_name = 'virtual/references_use_default_args/passes/reftest.html'
-        run_details, err, _ = logging_run(['--details', test_name], tests_included=True)
-        self.assertEqual(run_details.exit_code, 0)
-        self.assertEqual(run_details.initial_results.total, 1)
-        test_result = run_details.initial_results.all_results[0]
-        self.assertEqual(test_result.test_name, test_name)
-        self.assertEqual(test_result.references[0], 'passes/reftest-expected.html')
-        # reference_args should be empty since we are using the default flags.
-        self.assertTrue(re.search(r'reference_args:\s*ref:', err.getvalue()))
+        self.assertTrue(re.search(r'args: --virtual-arg\s*ref:', err.getvalue()))
 
     def test_reftest_matching_text_expectation(self):
         test_name = 'passes/reftest-with-text.html'

third_party/blink/tools/blinkpy/web_tests/views/printing.py

@@ -275,11 +275,10 @@ class Printer(object):
 
         base = port.lookup_virtual_test_base(test_name)
         if base:
-            args = ' '.join(port.lookup_virtual_test_args(test_name))
-            reference_args = ' '.join(port.lookup_virtual_reference_args(test_name))
             self._print_default(' base: %s' % base)
-            self._print_default(' args: %s' % args)
-            self._print_default(' reference_args: %s' % reference_args)
+        args = port.args_for_test(test_name)
+        if args:
+            self._print_default(' args: %s' % ' '.join(args))
 
         references = port.reference_files(test_name)
         if references: