wpt-import: Remove tests from expectations when WPT test harnesses are deleted

Several web platform tests do not have physical files checked into the
WPT repository. Instead they are generated and mapped to test harnesses
checked into the WPT repo. When those test harnesses are deleted then
we should delete all tests mapped to those files. Only affects the
behavior of the script when --clean-up-affected-tests-only is used.

Bug: 1050760, 1110003, 801357
Change-Id: Id1c30b096c73974ed0ded2d74284fbe43005ee0d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2439417
Commit-Queue: Rakib Hasan <rmhasan@google.com>
Reviewed-by: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816865}

third_party/blink/tools/blinkpy/w3c/android_wpt_expectations_updater_unittest.py

@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import json
 import logging
 
 from blinkpy.common.host_mock import MockHost
@@ -10,6 +11,7 @@ from blinkpy.common.net.git_cl_mock import MockGitCL
 from blinkpy.common.net.results_fetcher import Build
 from blinkpy.common.net.web_test_results import WebTestResults
 from blinkpy.common.system.log_testing import LoggingTestCase
+from blinkpy.w3c.wpt_manifest import BASE_MANIFEST_NAME
 from blinkpy.web_tests.builder_list import BuilderList
 from blinkpy.web_tests.port.factory_mock import MockPortFactory
 from blinkpy.web_tests.port.android import (
@@ -76,6 +78,18 @@ class AndroidWPTExpectationsUpdaterTest(LoggingTestCase):
                 'is_try_builder': True,
             },
         })
+        host.filesystem.write_text_file(
+            host.port_factory.get().web_tests_dir() + '/external/' +
+            BASE_MANIFEST_NAME,
+            json.dumps({
+                'items': {
+                    'testharness': {
+                        'ghi.html': ['abcdef123', [None, {}]],
+                        'van.html': ['abcdef123', [None, {}]],
+                    },
+                },
+            }))
+
         # Write dummy expectations
         for path in PRODUCTS_TO_EXPECTATION_FILE_PATHS.values():
             host.filesystem.write_text_file(

third_party/blink/tools/blinkpy/w3c/test_importer.py

@@ -45,9 +45,10 @@ _log = logging.getLogger(__file__)
 
 
 class TestImporter(object):
-    def __init__(self, host, wpt_github=None):
+    def __init__(self, host, wpt_github=None, wpt_manifests=None):
         self.host = host
         self.wpt_github = wpt_github
+        self.port = host.port_factory.get()
 
         self.executive = host.executive
         self.fs = host.filesystem
@@ -72,7 +73,8 @@ class TestImporter(object):
 
         args = ['--clean-up-affected-tests-only',
                 '--clean-up-test-expectations']
-        self._expectations_updater = WPTExpectationsUpdater(self.host, args)
+        self._expectations_updater = WPTExpectationsUpdater(
+            self.host, args, wpt_manifests)
 
     def main(self, argv=None):
         # TODO(robertma): Test this method! Split it to make it easier to test
@@ -155,14 +157,15 @@ class TestImporter(object):
         # TODO(robertma): Implement `add --all` in Git (it is different from `commit --all`).
         self.chromium_git.run(['add', '--all', self.dest_path])
 
+        # Remove expectations for tests that were deleted and rename tests
+        # in expectations for renamed tests.
+        self._expectations_updater.cleanup_test_expectations_files()
+
         self._generate_manifest()
 
         # TODO(crbug.com/800570 robertma): Re-enable it once we fix the bug.
         # self._delete_orphaned_baselines()
 
-        # Remove expectations for tests that were deleted and rename tests
-        # in expectations for renamed tests.
-        self._expectations_updater.cleanup_test_expectations_files()
 
         if not self.chromium_git.has_working_directory_changes():
             _log.info('Done: no changes to import.')

third_party/blink/tools/blinkpy/w3c/wpt_expectations_updater_unittest.py

@@ -18,7 +18,8 @@ from blinkpy.common.system.log_testing import LoggingTestCase
 
 from blinkpy.w3c.wpt_expectations_updater import (
     WPTExpectationsUpdater, SimpleTestResult, DesktopConfig)
-from blinkpy.w3c.wpt_manifest import BASE_MANIFEST_NAME
+from blinkpy.w3c.wpt_manifest import (
+    WPTManifest, BASE_MANIFEST_NAME, MANIFEST_NAME)
 
 from blinkpy.web_tests.builder_list import BuilderList
 from blinkpy.web_tests.port.android import PRODUCTS_TO_EXPECTATION_FILE_PATHS
@@ -84,6 +85,14 @@ class WPTExpectationsUpdaterTest(LoggingTestCase):
                     'testharness': {
                         'test/path.html': ['abcdef123', [None, {}]],
                         'test/zzzz.html': ['ghijkl456', [None, {}]],
+                        'fake/some_test.html': [
+                            'ghijkl456', ['fake/some_test.html?HelloWorld', {}]],
+                        'fake/file/deleted_path.html': [
+                            'ghijkl456', [None, {}]],
+                        'test/task.js': [
+                            'mnpqrs789',
+                            ['test/task.html', {}],
+                            ['test/task2.html', {}]],
                     },
                     'manual': {
                         'x-manual.html': ['abcdef123', [None, {}]],
@@ -801,6 +810,63 @@ class WPTExpectationsUpdaterTest(LoggingTestCase):
                     WPTExpectationsUpdater.MARKER_COMMENT + '\n' +
                     '[ linux ] external/wpt/fake/new.html?HelloWorld [ Failure ]\n'))
 
+    def test_clean_expectations_for_deleted_test_harness(self):
+        host = self.mock_host()
+        port = host.port_factory.get()
+        expectations_path = \
+            port.path_to_generic_test_expectations_file()
+        host.filesystem.write_text_file(
+            expectations_path,
+            '# tags: [ Win Linux ]\n' +
+            '# results: [ Pass Failure ]\n' +
+            WPTExpectationsUpdater.MARKER_COMMENT + '\n' +
+            '[ linux ] wpt_internal/test/task.html [ Failure ]\n' +
+            '[ win ] wpt_internal/test/task2.html [ Failure ]\n' +
+            '[ linux ] external/wpt/test/task.html [ Failure ]\n' +
+            'external/wpt/test/task2.html [ Pass ]\n')
+
+        def _git_command_return_val(cmd):
+            if '--diff-filter=D' in cmd:
+                return '\n'.join(['external/wpt/test/task.js',
+                                  'wpt_internal/test/task.js'])
+            return ''
+
+        wpt_manifest = port.wpt_manifest('external/wpt')
+        host.filesystem.maybe_make_directory(
+            port.web_tests_dir(), 'wpt_internal')
+        host.filesystem.copyfile(
+            host.filesystem.join(port.web_tests_dir(),
+                                 'external', 'wpt', MANIFEST_NAME),
+            host.filesystem.join(port.web_tests_dir(), 'wpt_internal',
+                                 MANIFEST_NAME))
+        wpt_internal_manifest = WPTManifest(host, host.filesystem.join(
+            port.web_tests_dir(), 'wpt_internal', MANIFEST_NAME))
+
+        updater = WPTExpectationsUpdater(
+            host,
+            ['--clean-up-affected-tests-only',
+             '--clean-up-test-expectations-only'],
+            [wpt_manifest, wpt_internal_manifest])
+        updater.git.run = _git_command_return_val
+        updater._relative_to_web_test_dir = lambda test_path: test_path
+        updater.cleanup_test_expectations_files()
+
+        test_expectations = {'external/wpt/fake/file/path.html': {
+            tuple([DesktopConfig(port_name='test-linux-trusty')]):
+            SimpleTestResult(actual='PASS', expected='', bug='crbug.com/123')}}
+        skip_path = host.port_factory.get().path_to_never_fix_tests_file()
+        skip_value_origin = host.filesystem.read_text_file(skip_path)
+
+        updater.write_to_test_expectations(test_expectations)
+        value = host.filesystem.read_text_file(expectations_path)
+        self.assertMultiLineEqual(
+            value, ('# tags: [ Win Linux ]\n' +
+                    '# results: [ Pass Failure ]\n\n' +
+                    WPTExpectationsUpdater.MARKER_COMMENT + '\n' +
+                    'crbug.com/123 [ Trusty ] external/wpt/fake/file/path.html [ Pass ]'))
+        skip_value = host.filesystem.read_text_file(skip_path)
+        self.assertMultiLineEqual(skip_value, skip_value_origin)
+
     def test_write_to_test_expectations_and_cleanup_expectations(self):
         host = self.mock_host()
         expectations_path = \
@@ -1225,16 +1291,21 @@ class WPTExpectationsUpdaterTest(LoggingTestCase):
 
     def test_cleanup_all_deleted_tests_in_expectations_files(self):
         host = MockHost()
-
+        port = host.port_factory.get()
         host.filesystem.files[MOCK_WEB_TESTS + 'TestExpectations'] = (
             '# results: [ Failure ]\n'
-            'some/test/a.html?hello%20world [ Failure ]\n'
+            'external/wpt/some/test/a.html?hello%20world [ Failure ]\n'
             'some/test/b.html [ Failure ]\n'
+            '# This line should be deleted\n'
+            'some/test/c.html [ Failure ]\n'
             '# line below should exist in new file\n'
             'some/test/d.html [ Failure ]\n')
         host.filesystem.files[MOCK_WEB_TESTS + 'VirtualTestSuites'] = '[]'
         host.filesystem.files[MOCK_WEB_TESTS + 'new/a.html'] = ''
         host.filesystem.files[MOCK_WEB_TESTS + 'new/b.html'] = ''
+        host.filesystem.files[
+            host.filesystem.join(
+                port.web_tests_dir(), 'some', 'test', 'd.html')] = ''
         # TODO(rmhasan): Remove creation of Android files within
         # tests.
         for path in PRODUCTS_TO_EXPECTATION_FILE_PATHS.values():
@@ -1248,7 +1319,6 @@ class WPTExpectationsUpdaterTest(LoggingTestCase):
             return ''
 
         updater.git.run = _git_command_return_val
-        updater.port.tests = lambda: ['some/test/d.html']
         updater._relative_to_web_test_dir = lambda test_path: test_path
         updater.cleanup_test_expectations_files()
         self.assertMultiLineEqual(
@@ -1297,8 +1367,8 @@ class WPTExpectationsUpdaterTest(LoggingTestCase):
             'external/wpt/webdriver/some/test/a*.html': 'old/a*.html',
             'external/wpt/webdriver/some/test/c.html': 'old/c.html',
         }
-        updater._list_deleted_test_files = lambda: deleted_files
-        updater._list_renamed_test_files = lambda: renamed_file_pairs
+        updater._list_deleted_files = lambda: deleted_files
+        updater._list_renamed_files = lambda: renamed_file_pairs
         updater.cleanup_test_expectations_files()
         self.assertMultiLineEqual(
             host.filesystem.read_text_file(MOCK_WEB_TESTS +

third_party/blink/tools/blinkpy/w3c/wpt_manifest.py

@@ -84,8 +84,11 @@ class WPTManifest(object):
         [[reference_url1, "=="], [reference_url2, "!="], ...]
     """
 
-    def __init__(self, json_content):
-        self.raw_dict = json.loads(json_content)
+    def __init__(self, host, manifest_path):
+        self.host = host
+        self.port = self.host.port_factory.get()
+        self.raw_dict = json.loads(
+            self.host.filesystem.read_text_file(manifest_path))
         # As a workaround to handle the change from a flat-list to a trie
         # structure in the v8 manifest, flatten the items back to the v7 format.
         #
@@ -93,9 +96,16 @@ class WPTManifest(object):
         self.raw_dict['items'] = self._flatten_items(
             self.raw_dict.get('items', {}))
 
+        self.wpt_manifest_path = manifest_path
         self.test_types = ('manual', 'reftest', 'testharness', 'crashtest')
         self.test_name_to_file = {}
 
+    @property
+    def wpt_dir(self):
+        return self.host.filesystem.dirname(
+            self.host.filesystem.relpath(
+                self.wpt_manifest_path, self.port.web_tests_dir()))
+
     def _items_for_file_path(self, path_in_wpt):
         """Finds manifest items for the given WPT path.
 
@@ -275,9 +285,8 @@ class WPTManifest(object):
     @staticmethod
     def generate_manifest(host, dest_path):
         """Generates MANIFEST.json on the specified directory."""
-        finder = PathFinder(host.filesystem)
-        wpt_exec_path = finder.path_from_blink_tools('blinkpy', 'third_party',
-                                                     'wpt', 'wpt', 'wpt')
+        wpt_exec_path = PathFinder(host.filesystem).path_from_blink_tools(
+            'blinkpy', 'third_party', 'wpt', 'wpt', 'wpt')
         cmd = [
             'python', wpt_exec_path, 'manifest', '--no-download',
             '--tests-root', dest_path

third_party/blink/tools/blinkpy/w3c/wpt_manifest_unittest.py

@@ -89,7 +89,11 @@ class WPTManifestUnitTest(unittest.TestCase):
     }
 }
         '''
-        manifest = WPTManifest(manifest_json)
+        host = MockHost()
+        host.filesystem.write_text_file(
+            WEB_TEST_DIR + '/external/wpt/MANIFEST.json', manifest_json)
+        manifest = WPTManifest(
+            host, WEB_TEST_DIR + '/external/wpt/MANIFEST.json')
         self.assertTrue(manifest.is_test_file('test.any.js'))
         self.assertEqual(manifest.all_url_items(),
                          {u'test.any.html': [u'test.any.html', {}]})
@@ -111,7 +115,11 @@ class WPTManifestUnitTest(unittest.TestCase):
     }
 }
         '''
-        manifest = WPTManifest(manifest_json)
+        host = MockHost()
+        host.filesystem.write_text_file(
+            WEB_TEST_DIR + '/external/wpt/MANIFEST.json', manifest_json)
+        manifest = WPTManifest(
+            host, WEB_TEST_DIR + '/external/wpt/MANIFEST.json')
         self.assertEqual(manifest.all_url_items(),
                          {u'test.any.html': [u'test.any.html', {}]})
 
@@ -132,7 +140,11 @@ class WPTManifestUnitTest(unittest.TestCase):
         }
     }
 }       '''
-        manifest = WPTManifest(manifest_json)
+        host = MockHost()
+        host.filesystem.write_text_file(
+            WEB_TEST_DIR + '/external/wpt/MANIFEST.json', manifest_json)
+        manifest = WPTManifest(
+            host, WEB_TEST_DIR + '/external/wpt/MANIFEST.json')
         self.assertEqual(
             manifest.all_url_items(), {
                 u'test.any.html': [u'test.any.html', {}],
@@ -168,7 +180,11 @@ class WPTManifestUnitTest(unittest.TestCase):
     }
 }
         '''
-        manifest = WPTManifest(manifest_json)
+        host = MockHost()
+        host.filesystem.write_text_file(
+            WEB_TEST_DIR + '/external/wpt/MANIFEST.json', manifest_json)
+        manifest = WPTManifest(
+            host, WEB_TEST_DIR + '/external/wpt/MANIFEST.json')
         self.assertEqual(
             manifest.all_url_items(), {
                 u'test.html': [u'test.html', {}],

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

@@ -144,7 +144,7 @@ class LintTest(LoggingTestCase):
         port = host.port_factory.get(options.platform, options=options)
         port.expectations_dict = lambda: {'foo': '-- syntax error1', 'bar': '-- syntax error2'}
 
-        host.port_factory.get = lambda platform, options=None: port
+        host.port_factory.get = lambda platform=None, options=None: port
         host.port_factory.all_port_names = lambda platform=None: [port.name()]
 
         failures, warnings = lint_test_expectations.lint(host, options)
@@ -166,7 +166,7 @@ class LintTest(LoggingTestCase):
         port = host.port_factory.get(options.platform, options=options)
         port.expectations_dict = lambda: {}
 
-        host.port_factory.get = lambda platform, options=None: port
+        host.port_factory.get = lambda platform=None, options=None: port
         host.port_factory.all_port_names = lambda platform=None: [port.name()]
         host.filesystem.write_text_file(WEB_TEST_DIR + '/LeakExpectations',
                                         '-- syntax error')
@@ -189,7 +189,7 @@ class LintTest(LoggingTestCase):
         port = host.port_factory.get(options.platform, options=options)
         port.expectations_dict = lambda: {'flag-specific': 'does/not/exist', 'noproblem': ''}
 
-        host.port_factory.get = lambda platform, options=None: port
+        host.port_factory.get = lambda platform=None, options=None: port
         host.port_factory.all_port_names = lambda platform=None: [port.name()]
 
         failures, warnings = lint_test_expectations.lint(host, options)
@@ -218,7 +218,7 @@ class LintTest(LoggingTestCase):
         port.expectations_dict = lambda: {
             'testexpectations': test_expectations}
 
-        host.port_factory.get = lambda platform, options=None: port
+        host.port_factory.get = lambda platform=None, options=None: port
         host.port_factory.all_port_names = lambda platform=None: [port.name()]
 
         failures, warnings = lint_test_expectations.lint(host, options)
@@ -267,7 +267,7 @@ class LintTest(LoggingTestCase):
             host.filesystem.join(port.web_tests_dir(), 'virtual', 'foo',
                                  'README.md'), 'foo')
 
-        host.port_factory.get = lambda platform, options=None: port
+        host.port_factory.get = lambda platform=None, options=None: port
         host.port_factory.all_port_names = lambda platform=None: [port.name()]
 
         failures, warnings = lint_test_expectations.lint(host, options)
@@ -300,7 +300,7 @@ class LintTest(LoggingTestCase):
                             'non-wpt/test.html [ Failure ]\n')
         for path in PRODUCTS_TO_EXPECTATION_FILE_PATHS.values():
             host.filesystem.write_text_file(path, raw_expectations)
-        host.port_factory.get = lambda platform, options=None: port
+        host.port_factory.get = lambda platform=None, options=None: port
         host.port_factory.all_port_names = lambda platform=None: [port.name()]
         port.test_exists = lambda _: True
         port.tests = lambda _: {'external/wpt/test.html', 'non-wpt/test.html'}
@@ -325,7 +325,7 @@ class LintTest(LoggingTestCase):
         host.filesystem.maybe_make_directory(
             host.filesystem.join(port.web_tests_dir(), 'test2'))
 
-        host.port_factory.get = lambda platform, options=None: port
+        host.port_factory.get = lambda platform=None, options=None: port
         host.port_factory.all_port_names = lambda platform=None: [port.name()]
 
         failures, warnings = lint_test_expectations.lint(host, options)
@@ -364,7 +364,7 @@ class LintTest(LoggingTestCase):
             'testexpectations': test_expectations
         }
         port.test_exists = lambda test: True
-        host.port_factory.get = lambda platform, options=None: port
+        host.port_factory.get = lambda platform=None, options=None: port
         host.port_factory.all_port_names = lambda platform=None: [port.name()]
 
         failures, warnings = lint_test_expectations.lint(host, options)
@@ -399,7 +399,7 @@ class LintTest(LoggingTestCase):
                              'virtual/foo/test1/* [ Pass ]\n')
         port.expectations_dict = lambda: {'NeverFixTests': test_expectations}
         port.test_exists = lambda test: True
-        host.port_factory.get = lambda platform, options=None: port
+        host.port_factory.get = lambda platform=None, options=None: port
         host.port_factory.all_port_names = lambda platform=None: [port.name()]
 
         failures, warnings = lint_test_expectations.lint(host, options)

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

@@ -959,7 +959,7 @@ class Port(object):
                 'manifest_update', True):
             _log.debug('Generating MANIFEST.json for %s...', path)
             WPTManifest.ensure_manifest(self, path)
-        return WPTManifest(self._filesystem.read_text_file(manifest_path))
+        return WPTManifest(self.host, manifest_path)
 
     def is_wpt_crash_test(self, test_file):
         """Returns whether a WPT test is a crashtest.

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

@@ -32,6 +32,7 @@ import unittest
 import mock
 
 from blinkpy.common.path_finder import RELATIVE_WEB_TESTS
+from blinkpy.common.host_mock import MockHost
 from blinkpy.common.system.executive_mock import MockExecutive
 from blinkpy.common.system.log_testing import LoggingTestCase
 from blinkpy.common.system.output_capture import OutputCapture
@@ -51,7 +52,7 @@ class PortTest(LoggingTestCase):
                   with_tests=False,
                   port_name=None,
                   **kwargs):
-        host = MockSystemHost()
+        host = MockHost()
         if executive:
             host.executive = executive
         if with_tests: