blinkpy: Move disabled WPT tests to AndroidWPTNeverFixTests

This CL will make a change that makes expectations for unexpected skips
in the AndroidNeverFixTests file. This CL also changes the Android tags
produced by the test harness to include the Android WPT product
argument.This CL also removes the --android-apk command line argument
adds the --android-product command line argument to the
build_wpt_metadata.py script. The product argument for the wpt
runner will be passed in as the value for this argument.

Bug: 1050760, 1050754
Change-Id: I32a8df293ed1442cb810976ec643e4c63b0414ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2204543Reviewed-by: Robert Ma <robertma@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Commit-Queue: Rakib Hasan <rmhasan@google.com>
Cr-Commit-Position: refs/heads/master@{#773302}

testing/scripts/run_android_wpt.py

@@ -179,19 +179,22 @@ class WPTAndroidAdapter(common.BaseIsolatedScriptArgsAdapter):
     metadata_builder_cmd = [
          sys.executable,
          os.path.join(BLINK_TOOLS_DIR, 'build_wpt_metadata.py'),
-         '--android-apk',
-         self._test_apk,
+         '--android-product',
+         self.options.product,
+         '--ignore-default-expectations',
          '--metadata-output-dir',
          self._metadata_dir,
          '--additional-expectations',
          os.path.join(WEB_TESTS_DIR, 'android', 'AndroidWPTNeverFixTests')
     ]
     metadata_builder_cmd.extend(self._extra_metadata_builder_args())
-    common.run_command(metadata_builder_cmd)
+    return common.run_command(metadata_builder_cmd)
 
   def run_test(self):
     with NamedTemporaryDirectory() as self._metadata_dir, self._install_apks():
-      self._maybe_build_metadata()
+      metadata_command_ret = self._maybe_build_metadata()
+      if metadata_command_ret != 0:
+          return metadata_command_ret
       return super(WPTAndroidAdapter, self).run_test()
 
   def _install_apks(self):

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

@@ -24,10 +24,10 @@ from blinkpy.common.memoized import memoized
 from blinkpy.common.system.executive import ScriptError
 from blinkpy.w3c.wpt_expectations_updater import WPTExpectationsUpdater
 from blinkpy.web_tests.models.test_expectations import TestExpectations
-from blinkpy.web_tests.models.typ_types import Expectation
+from blinkpy.web_tests.models.typ_types import Expectation, ResultType
 from blinkpy.web_tests.port.android import (
     PRODUCTS, PRODUCTS_TO_STEPNAMES, PRODUCTS_TO_BROWSER_TAGS,
-    PRODUCTS_TO_EXPECTATION_FILE_PATHS)
+    PRODUCTS_TO_EXPECTATION_FILE_PATHS, ANDROID_DISABLED_TESTS)
 
 _log = logging.getLogger(__name__)
 
@@ -36,6 +36,7 @@ AndroidConfig = namedtuple('AndroidConfig', ['port_name', 'browser'])
 
 class AndroidWPTExpectationsUpdater(WPTExpectationsUpdater):
     MARKER_COMMENT = '# Add untriaged failures in this block'
+    NEVER_FIX_MARKER_COMMENT = '# Add untriaged disabled tests in this block'
     UMBRELLA_BUG = 'crbug.com/1050754'
 
     def __init__(self, host, args=None):
@@ -48,6 +49,10 @@ class AndroidWPTExpectationsUpdater(WPTExpectationsUpdater):
 
         self._test_expectations = TestExpectations(
             self.port, expectations_dict=expectations_dict)
+        self._never_fix_expectations = TestExpectations(
+            self.port, {
+                ANDROID_DISABLED_TESTS:
+                host.filesystem.read_text_file(ANDROID_DISABLED_TESTS)})
 
     def _get_web_test_results(self, build):
         """Gets web tests results for Android builders. We need to
@@ -114,13 +119,42 @@ class AndroidWPTExpectationsUpdater(WPTExpectationsUpdater):
         Android at the moment."""
         return False
 
+    @staticmethod
     @memoized
-    def _get_marker_line_number(self, path):
-        for line in self._test_expectations.get_updated_lines(path):
-            if line.to_string() == self.MARKER_COMMENT:
+    def _get_marker_line_number(test_expectations, path, marker_comment):
+        for line in test_expectations.get_updated_lines(path):
+            if line.to_string() == marker_comment:
                 return line.lineno
         raise ScriptError('Marker comment does not exist in %s' % path)
 
+    def _get_untriaged_test_expectations(
+            self, test_expectations, marker_comment):
+        untriaged_exps = defaultdict(dict)
+        for path in test_expectations.expectations_dict:
+            marker_lineno = self._get_marker_line_number(
+                test_expectations, path, marker_comment)
+            exp_lines = test_expectations.get_updated_lines(path)
+            for i in range(marker_lineno, len(exp_lines)):
+                if (not exp_lines[i].to_string().strip() or
+                        exp_lines[i].to_string().startswith('#')):
+                    break
+                untriaged_exps[path].setdefault(
+                    exp_lines[i].test, []).append(exp_lines[i])
+        return untriaged_exps
+
+    def _maybe_create_never_fix_expectation(
+            self, path, test, test_skipped, tags):
+        if test_skipped:
+            exps = self._test_expectations.get_expectations_from_file(
+                path, test)
+            wontfix = self._never_fix_expectations.matches_an_expected_result(
+                test, ResultType.Skip)
+            temporary_skip = any(ResultType.Skip in exp.results for exp in exps)
+            if not (wontfix or temporary_skip):
+                return Expectation(
+                    test=test, reason=self.UMBRELLA_BUG,
+                    results={ResultType.Skip}, tags=tags, raw_tags=tags)
+
     def write_to_test_expectations(self, test_to_results):
         """Each expectations file is browser specific, and currently only
         runs on pie. Therefore we do not need any configuration specifiers
@@ -136,48 +170,75 @@ class AndroidWPTExpectationsUpdater(WPTExpectationsUpdater):
         browser_to_exp_path = {
             browser: PRODUCTS_TO_EXPECTATION_FILE_PATHS[product]
             for product, browser in PRODUCTS_TO_BROWSER_TAGS.items()}
-        untriaged_exps = defaultdict(dict)
-
-        for path in self._test_expectations.expectations_dict:
-            marker_lineno = self._get_marker_line_number(path)
-            exp_lines = self._test_expectations.get_updated_lines(path)
-            for i in range(marker_lineno, len(exp_lines)):
-                if (not exp_lines[i].to_string().strip() or
-                        exp_lines[i].to_string().startswith('#')):
-                    break
-                untriaged_exps[path][exp_lines[i].test] = exp_lines[i]
+        untriaged_exps = self._get_untriaged_test_expectations(
+            self._test_expectations, self.MARKER_COMMENT)
+        neverfix_tests = self._get_untriaged_test_expectations(
+            self._never_fix_expectations, self.NEVER_FIX_MARKER_COMMENT)[
+                ANDROID_DISABLED_TESTS]
 
         for path, test_exps in untriaged_exps.items():
             self._test_expectations.remove_expectations(
-                path, test_exps.values())
+                path, reduce(lambda x, y: x + y, test_exps.values()))
+
+        if neverfix_tests:
+            self._never_fix_expectations.remove_expectations(
+                ANDROID_DISABLED_TESTS,
+                reduce(lambda x, y: x + y, neverfix_tests.values()))
 
         for results_test_name, platform_results in test_to_results.items():
             exps_test_name = 'external/wpt/%s' % results_test_name
             for configs, test_results in platform_results.items():
                 for config in configs:
                     path = browser_to_exp_path[config.browser]
-                    # no system specifiers are necessary because we are
-                    # writing to browser specific expectations files for
-                    # only one Android version.
-                    unexpected_results = {r for r in test_results.actual.split()
-                                          if r not in test_results.expected.split()}
-
-                    if exps_test_name not in untriaged_exps[path]:
-                        untriaged_exps[path][exps_test_name] = Expectation(
-                            test=exps_test_name, reason=self.UMBRELLA_BUG,
-                            results=unexpected_results)
+                    neverfix_exp = self._maybe_create_never_fix_expectation(
+                        path, exps_test_name,
+                        ResultType.Skip in test_results.actual,
+                        {config.browser.lower()})
+                    if neverfix_exp:
+                        neverfix_tests.setdefault(exps_test_name, []).append(
+                            neverfix_exp)
                     else:
-                        untriaged_exps[path][exps_test_name].add_expectations(
-                            unexpected_results, reason=self.UMBRELLA_BUG)
+                        # no system specifiers are necessary because we are
+                        # writing to browser specific expectations files for
+                        # only one Android version.
+                        unexpected_results = {
+                            r for r in test_results.actual.split()
+                            if r not in test_results.expected.split()}
+
+                        if exps_test_name not in untriaged_exps[path]:
+                            untriaged_exps[path].setdefault(
+                                exps_test_name, []).append(Expectation(
+                                    test=exps_test_name, reason=self.UMBRELLA_BUG,
+                                    results=unexpected_results))
+                        else:
+                            exp = untriaged_exps[path][exps_test_name][0]
+                            exp.add_expectations(
+                                unexpected_results, reason=self.UMBRELLA_BUG)
 
         for path in untriaged_exps:
-            marker_lineno = self._get_marker_line_number(path)
+            marker_lineno = self._get_marker_line_number(
+                self._test_expectations, path, self.MARKER_COMMENT)
             self._test_expectations.add_expectations(
                 path,
-                sorted(untriaged_exps[path].values(), key=lambda e: e.test),
+                sorted([exps[0] for exps in untriaged_exps[path].values()],
+                       key=lambda e: e.test),
                 marker_lineno)
 
+        disabled_tests_marker_lineno = self._get_marker_line_number(
+            self._never_fix_expectations,
+            ANDROID_DISABLED_TESTS,
+            self.NEVER_FIX_MARKER_COMMENT)
+
+        if neverfix_tests:
+            self._never_fix_expectations.add_expectations(
+                ANDROID_DISABLED_TESTS,
+                sorted(reduce(lambda x, y: x + y, neverfix_tests.values()),
+                       key=lambda e: e.test),
+                disabled_tests_marker_lineno)
+
         self._test_expectations.commit_changes()
+        self._never_fix_expectations.commit_changes()
+
         # TODO(rmhasan): Return dictionary mapping test names to lists of
         # test expectation strings.
         return {}

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

@@ -12,7 +12,8 @@ from blinkpy.common.net.web_test_results import WebTestResults
 from blinkpy.common.system.log_testing import LoggingTestCase
 from blinkpy.web_tests.builder_list import BuilderList
 from blinkpy.web_tests.port.factory_mock import MockPortFactory
-from blinkpy.web_tests.port.android import PRODUCTS_TO_EXPECTATION_FILE_PATHS
+from blinkpy.web_tests.port.android import (
+    PRODUCTS_TO_EXPECTATION_FILE_PATHS, ANDROID_DISABLED_TESTS)
 from blinkpy.w3c.android_wpt_expectations_updater import (
     AndroidWPTExpectationsUpdater)
 
@@ -35,6 +36,13 @@ class AndroidWPTExpectationsUpdaterTest(LoggingTestCase):
         '# This comment will not be deleted\n'
         'crbug.com/111111 external/wpt/hello_world.html [ Crash ]\n')
 
+    _raw_android_never_fix_tests = (
+        '# tags: [ android-weblayer android-webview chrome-android ]\n'
+        '# results: [ Skip ]\n'
+        '\n'
+        '# Add untriaged disabled tests in this block\n'
+        'crbug.com/1050754 [ android-webview ] external/wpt/disabled.html [ Skip ]\n')
+
     def _setup_host(self):
         """Returns a mock host with fake values set up for testing."""
         self.set_logging_level(logging.DEBUG)
@@ -65,6 +73,9 @@ class AndroidWPTExpectationsUpdaterTest(LoggingTestCase):
         for path in PRODUCTS_TO_EXPECTATION_FILE_PATHS.values():
             host.filesystem.write_text_file(
                 path, self._raw_android_expectations)
+
+        host.filesystem.write_text_file(
+            ANDROID_DISABLED_TESTS, self._raw_android_never_fix_tests)
         return host
 
     def testUpdateTestExpectationsForWebview(self):
@@ -88,6 +99,11 @@ class AndroidWPTExpectationsUpdaterTest(LoggingTestCase):
                         'actual': 'CRASH CRASH TIMEOUT',
                         'is_unexpected': True,
                     },
+                    'dog.html': {
+                        'expected': 'SKIP',
+                        'actual': 'SKIP',
+                        'is_unexpected': True,
+                    },
                 },
             }, step_name='system_webview_wpt (with patch)'),
             step_name='system_webview_wpt (with patch)')
@@ -118,6 +134,16 @@ class AndroidWPTExpectationsUpdaterTest(LoggingTestCase):
              '\n'
              '# This comment will not be deleted\n'
              'crbug.com/111111 external/wpt/hello_world.html [ Crash ]\n'))
+        neverfix_content = host.filesystem.read_text_file(
+            ANDROID_DISABLED_TESTS)
+        self.assertEqual(
+            neverfix_content,
+            ('# tags: [ android-weblayer android-webview chrome-android ]\n'
+             '# results: [ Skip ]\n'
+             '\n'
+             '# Add untriaged disabled tests in this block\n'
+             'crbug.com/1050754 [ android-webview ] external/wpt/disabled.html [ Skip ]\n'
+             'crbug.com/1050754 [ android-webview ] external/wpt/dog.html [ Skip ]\n'))
         # check that chrome android's expectation file was not modified
         # since the same bot is used to update chrome android & webview
         # expectations
@@ -209,6 +235,11 @@ class AndroidWPTExpectationsUpdaterTest(LoggingTestCase):
                         'actual': 'CRASH CRASH FAIL',
                         'is_unexpected': True,
                     },
+                    'disabled_weblayer_only.html': {
+                        'expected': 'SKIP',
+                        'actual': 'SKIP',
+                        'is_unexpected': True,
+                    },
                 },
             }, step_name='weblayer_shell_wpt (with patch)'),
             step_name='weblayer_shell_wpt (with patch)')
@@ -227,6 +258,10 @@ class AndroidWPTExpectationsUpdaterTest(LoggingTestCase):
                         'actual': 'TIMEOUT',
                         'is_unexpected': True,
                     },
+                    'disabled.html': {
+                        'expected': 'SKIP',
+                        'actual': 'SKIP',
+                    },
                 },
             }, step_name='system_webview_wpt (with patch)'),
             step_name='system_webview_wpt (with patch)')
@@ -245,6 +280,11 @@ class AndroidWPTExpectationsUpdaterTest(LoggingTestCase):
                         'actual': 'CRASH CRASH TIMEOUT',
                         'is_unexpected': True,
                     },
+                    'disabled.html': {
+                        'expected': 'SKIP',
+                        'actual': 'SKIP',
+                        'is_unexpected': True,
+                    },
                 },
             }, step_name='chrome_public_wpt (with patch)'),
             step_name='chrome_public_wpt (with patch)')
@@ -321,3 +361,14 @@ class AndroidWPTExpectationsUpdaterTest(LoggingTestCase):
              '\n'
              '# This comment will not be deleted\n'
              'crbug.com/111111 external/wpt/hello_world.html [ Crash ]\n'))
+        # Check disabled test file
+        neverfix_content = host.filesystem.read_text_file(ANDROID_DISABLED_TESTS)
+        self.assertEqual(
+            neverfix_content,
+            ('# tags: [ android-weblayer android-webview chrome-android ]\n'
+             '# results: [ Skip ]\n'
+             '\n'
+             '# Add untriaged disabled tests in this block\n'
+             'crbug.com/1050754 [ android-webview ] external/wpt/disabled.html [ Skip ]\n'
+             'crbug.com/1050754 [ chrome-android ] external/wpt/disabled.html [ Skip ]\n'
+             'crbug.com/1050754 [ android-weblayer ] external/wpt/disabled_weblayer_only.html [ Skip ]\n'))

third_party/blink/tools/blinkpy/web_tests/models/test_expectations.py

@@ -151,6 +151,11 @@ class TestExpectations(object):
             raise ParseError(expectation_errors)
         self._add_expectations_from_bot()
 
+    def set_system_condition_tags(self, tags):
+        for test_exps in self._expectations:
+            test_exps.set_tags(tags)
+        self._system_condition_tags = tags
+
     @staticmethod
     def _maybe_remove_comments_and_whitespace(lines):
         """If the last expectation in a block is deleted, then remove all associated

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

@@ -64,6 +64,8 @@ intent = None
 perf_control = None
 # pylint: enable=invalid-name
 
+_sanitize_android_tag = lambda t: t.replace('_', '-')
+
 # product constants used by the wpt runner.
 ANDROID_WEBLAYER = 'android_weblayer'
 ANDROID_WEBVIEW = 'android_webview'
@@ -78,10 +80,8 @@ PRODUCTS_TO_STEPNAMES = {
 }
 
 PRODUCTS_TO_BROWSER_TAGS = {
-    ANDROID_WEBLAYER: 'weblayer',
-    ANDROID_WEBVIEW: 'webview',
-    CHROME_ANDROID: 'chrome',
-}
+    product: _sanitize_android_tag(product)
+    for product in PRODUCTS}
 
 # Android web tests directory, which contains override expectation files
 ANDROID_WEB_TESTS_DIR = os.path.join(get_blink_dir(), 'web_tests', 'android')
@@ -95,6 +95,10 @@ PRODUCTS_TO_EXPECTATION_FILE_PATHS = {
         ANDROID_WEB_TESTS_DIR, 'ClankWPTOverrideExpectations'),
 }
 
+# Disabled WPT tests on Android
+ANDROID_DISABLED_TESTS = os.path.join(
+    ANDROID_WEB_TESTS_DIR, 'AndroidWPTNeverFixTests')
+
 _friendly_browser_names = {
     'weblayershell': 'weblayer',
     'systemwebviewshell': 'webview',
@@ -297,19 +301,19 @@ class AndroidPort(base.Port):
 
     BUILD_REQUIREMENTS_URL = 'https://www.chromium.org/developers/how-tos/android-build-instructions'
 
-    def __init__(self, host, port_name='', apk='', options=None, **kwargs):
+    def __init__(self, host, port_name='', apk='', product='', options=None, **kwargs):
         super(AndroidPort, self).__init__(
             host, port_name, options=options, **kwargs)
         self._operating_system = 'android'
         self._version = 'kitkat'
         fs = host.filesystem
         self._local_port = factory.PortFactory(host).get(**kwargs)
-
-        if apk:
+        if apk or product:
             self._driver_details = DriverDetails(apk)
             browser_type = fs.splitext(fs.basename(apk))[0].lower()
             self._browser_type = _friendly_browser_names.get(
                 browser_type, browser_type)
+            self._wpt_product_arg = product
         else:
             # The legacy test runner will be used to run web tests on Android.
             # So we need to initialize several port member variables.
@@ -318,6 +322,7 @@ class AndroidPort(base.Port):
             self._browser_type = 'content_shell'
             self._debug_logging = self.get_option('android_logging')
             self.server_process_constructor = self._android_server_process_constructor
+            self._wpt_product_arg = ''
 
             if not self.get_option('disable_breakpad'):
                 self._dump_reader = DumpReaderAndroid(host, self._build_path())
@@ -349,10 +354,14 @@ class AndroidPort(base.Port):
         # TODO(rmhasan) Add bot expectations to WPT metadata.
         return {}
 
+    def expected_test(self, _):
+      return
+
     def get_platform_tags(self):
-        _sanitize_tag = lambda t: t.replace('_', '-').replace(' ', '-')
-        return frozenset(
-            ['android', 'android-' + _sanitize_tag(self._browser_type)])
+        tags = {'android'}
+        if self._wpt_product_arg:
+            tags.add(_sanitize_android_tag(self._wpt_product_arg))
+        return frozenset(tags)
 
     def default_smoke_test_only(self):
         return True

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

@@ -132,15 +132,16 @@ class AndroidPortTest(port_testcase.PortTestCase):
 
     def test_weblayer_expectation_tags(self):
         host = MockSystemHost()
-        port = android.AndroidPort(host, apk='apks/WebLayerShell.apk')
+        port = android.AndroidPort(
+            host, product='android_weblayer')
         self.assertEqual(port.get_platform_tags(),
                          set(['android', 'android-weblayer']))
 
-    def test_content_shell_expectation_tags(self):
+    def test_default_no_wpt_product_tag(self):
         host = MockSystemHost()
         port = android.AndroidPort(host)
         self.assertEqual(port.get_platform_tags(),
-                         set(['android', 'android-content-shell']))
+                         set(['android']))
 
     # Test that an HTTP server indeed is required by Android (as we serve all tests over them)
     def test_requires_http_server(self):

third_party/blink/tools/build_wpt_metadata.py

@@ -8,7 +8,8 @@ import optparse
 import sys
 
 from blinkpy.common.host import Host
-from blinkpy.web_tests.port.android import AndroidPort
+from blinkpy.web_tests.port.android import (
+    AndroidPort, PRODUCTS)
 from blinkpy.w3c.wpt_metadata_builder import WPTMetadataBuilder
 from blinkpy.web_tests.models.test_expectations import TestExpectations
 
@@ -20,16 +21,23 @@ def main(args):
         action="append",
         help="Paths to additional expectations files for WPT.")
     parser.add_argument(
-        "--android-apk",
+        "--android-product",
         default=None,
-        help="Path to Android APK that is being tested")
+        choices=PRODUCTS,
+        help="Android product argument for wpt runner.")
+    parser.add_argument(
+        '--ignore-default-expectations',
+        action='store_true',
+        help='Do not use the default set of test expectations files.'
+             ' i.e TestExpectations, NeverFixTests, etc...')
 
     known_args, rest_args = parser.parse_known_args(args)
     options = optparse.Values(vars(known_args))
     host = Host()
 
-    if known_args.android_apk:
-        port = AndroidPort(host, apk=known_args.android_apk, options=options)
+    if known_args.android_product:
+        port = AndroidPort(
+            host, product=known_args.android_product, options=options)
     else:
         port = host.port_factory.get(options=options)
 

third_party/blink/web_tests/android/AndroidWPTNeverFixTests

-# tags: [ android-weblayer ]
+# tags: [ android-weblayer android-webview chrome-android ]
 # results: [ Skip ]
 
 # This expectations file will disable tests for functionality which there are no
 # plans to implement in any of the Android browsers.
 
+
+# Add untriaged disabled tests in this block