Auto merge of #29923 - mrobinson:test-scripts, r=jdm

Combine all script tests into `test-scripts`

Remove:
    - tidy self test support from `./mach test`
    - `./mach test-idl`

Adds a `./mach test-scripts` command that is responsible for running all
Python script tests. Run this during the CI to catch regressions in
changes to scripts. The WebIDL tests are still *very* slow and there are
from Gecko, so only run them when "-a" is passed meaning all tests.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

.github/workflows/linux.yml

@@ -75,6 +75,8 @@ jobs:
         run: python3 ./mach build --release --with-${{ env.LAYOUT }}
       - name: Smoketest
         run: xvfb-run python3 ./mach smoketest
+      - name: Script tests
+        run: ./mach test-scripts
       - name: Unit tests
         if: ${{ inputs.unit-tests }}
         run: python3 ./mach test-unit --release --with-${{ env.LAYOUT }}

.github/workflows/mac.yml

@@ -63,6 +63,8 @@ jobs:
           python3 ./mach build --release --with-${{ env.LAYOUT }}
       - name: Smoketest
         run: python3 ./mach smoketest
+      - name: Script tests
+        run: ./mach test-scripts
       - name: Unit tests
         if: ${{ inputs.unit-tests }}
         run: python3 ./mach test-unit --release --with-${{ env.LAYOUT }}

.github/workflows/pull-request-wpt-export.yml

@@ -32,7 +32,7 @@ jobs:
           # See https://github.com/actions/checkout/issues/162.
           token: ${{ secrets.WPT_SYNC_TOKEN }}
       - name: Install requirements
-        run: pip install -r servo/python/wpt/requirements.txt
+        run: pip install -r servo/python/requirements.txt
       - name: Process pull request
         run: servo/python/wpt/export.py
         env:

.github/workflows/test-export-wpt-changes.yml

@@ -1,18 +0,0 @@
-name: WPT exporter test
-on:
-  pull_request:
-    branches: ["**"]
-    paths: ["python/wpt/exporter/**"]
-
-jobs:
-  test:
-    name: Test
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v3
-      - name: Install dependencies
-        run: |
-          python3 -m pip install --upgrade -r python/wpt/requirements-dev.txt
-      - name: Running tests
-        run: |
-          python3 python/wpt/test.py

python/requirements.txt

@@ -42,3 +42,8 @@ clang-format ~= 16.0.0
 # A few more requirements for tidy.
 voluptuous == 0.12.1
 PyYAML == 5.4
+
+# For wpt scripts and their tests.
+flask
+requests
+types-requests

python/servo/testing_commands.py

@@ -10,6 +10,7 @@
 from __future__ import print_function, unicode_literals
 
 import argparse
+import logging
 import re
 import sys
 import os
@@ -49,9 +50,6 @@ CLANGFMT_CPP_DIRS = ["support/hololens/"]
 CLANGFMT_VERSION = "15"
 
 TEST_SUITES = OrderedDict([
-    ("tidy", {"kwargs": {"all_files": False, "no_progress": False, "self_test": False,
-                         "stylo": False},
-              "include_arg": "include"}),
     ("wpt", {"kwargs": {"release": False},
              "paths": [path.abspath(WEB_PLATFORM_TESTS_PATH),
                        path.abspath(SERVO_TESTS_PATH)],
@@ -80,25 +78,12 @@ class MachCommands(CommandBase):
     @CommandArgument('params', default=None, nargs="...",
                      help="Optionally select test based on "
                           "test file directory")
-    @CommandArgument('--render-mode', '-rm', default=DEFAULT_RENDER_MODE,
-                     help="The render mode to be used on all tests. "
-                          + HELP_RENDER_MODE)
     @CommandArgument('--release', default=False, action="store_true",
                      help="Run with a release build of servo")
-    @CommandArgument('--tidy-all', default=False, action="store_true",
-                     help="Check all files, and run the WPT lint in tidy, "
-                          "even if unchanged")
-    @CommandArgument('--no-progress', default=False, action="store_true",
-                     help="Don't show progress for tidy")
-    @CommandArgument('--self-test', default=False, action="store_true",
-                     help="Run unit tests for tidy")
     @CommandArgument('--all', default=False, action="store_true", dest="all_suites",
                      help="Run all test suites")
-    def test(self, params, render_mode=DEFAULT_RENDER_MODE, release=False, tidy_all=False,
-             no_progress=False, self_test=False, all_suites=False):
+    def test(self, params, release=False, all_suites=False):
         suites = copy.deepcopy(TEST_SUITES)
-        suites["tidy"]["kwargs"] = {"all_files": tidy_all, "no_progress": no_progress, "self_test": self_test,
-                                    "stylo": False}
         suites["wpt"]["kwargs"] = {"release": release}
         suites["unit"]["kwargs"] = {}
 
@@ -293,50 +278,65 @@ class MachCommands(CommandBase):
                      help="Skip checking that web-platform-tests manifests are up to date")
     @CommandArgument('--no-progress', default=False, action="store_true",
                      help="Don't show progress for tidy")
-    @CommandArgument('--self-test', default=False, action="store_true",
-                     help="Run unit tests for tidy")
     @CommandArgument('--stylo', default=False, action="store_true",
                      help="Only handle files in the stylo tree")
-    def test_tidy(self, all_files, no_progress, self_test, stylo, force_cpp=False, no_wpt=False):
-        if self_test:
-            return tidy.do_tests()
+    def test_tidy(self, all_files, no_progress, stylo, no_wpt=False):
+        if no_wpt:
+            manifest_dirty = False
         else:
-            if no_wpt:
-                manifest_dirty = False
-            else:
-                manifest_dirty = wpt.manifestupdate.update(check_clean=True)
-            tidy_failed = tidy.scan(not all_files, not no_progress, stylo=stylo, no_wpt=no_wpt)
-            self.install_rustfmt()
-            rustfmt_failed = self.call_rustup_run(["cargo", "fmt", "--", "--check"])
+            manifest_dirty = wpt.manifestupdate.update(check_clean=True)
+        tidy_failed = tidy.scan(not all_files, not no_progress, stylo=stylo, no_wpt=no_wpt)
+        self.install_rustfmt()
+        rustfmt_failed = self.call_rustup_run(["cargo", "fmt", "--", "--check"])
 
-            print("Checking C++ files for tidiness...")
-            env = self.build_env()
-            clangfmt_failed = not run_clang_format(env, ["--dry-run", "--Werror"])
+        print("Checking C++ files for tidiness...")
+        env = self.build_env()
+        clangfmt_failed = not run_clang_format(env, ["--dry-run", "--Werror"])
 
-            if rustfmt_failed or clangfmt_failed:
-                print("Run `./mach fmt` to fix the formatting")
+        if rustfmt_failed or clangfmt_failed:
+            print("Run `./mach fmt` to fix the formatting")
 
-            return tidy_failed or manifest_dirty or rustfmt_failed or clangfmt_failed
+        return tidy_failed or manifest_dirty or rustfmt_failed or clangfmt_failed
 
-    @Command('test-webidl',
-             description='Run the WebIDL parser tests',
+    @Command('test-scripts',
+             description='Run tests for all build and support scripts.',
              category='testing')
-    @CommandArgument('--quiet', '-q', default=False, action="store_true",
-                     help="Don't print passing tests.")
+    @CommandArgument('--verbose', '-v', default=False, action="store_true",
+                     help="Enable verbose output")
+    @CommandArgument('--very-verbose', '-vv', default=False, action="store_true",
+                     help="Enable very verbose output")
+    @CommandArgument('--all', '-a', default=False, action="store_true",
+                     help="Run all script tests, even the slow ones.")
     @CommandArgument('tests', default=None, nargs="...",
-                     help="Specific tests to run, relative to the tests directory")
-    def test_webidl(self, quiet, tests):
-        test_file_dir = path.abspath(path.join(PROJECT_TOPLEVEL_PATH, "components", "script",
-                                               "dom", "bindings", "codegen", "parser"))
-        # For the `import WebIDL` in runtests.py
-        sys.path.insert(0, test_file_dir)
+                     help="Specific WebIDL tests to run, relative to the tests directory")
+    def test_scripts(self, verbose, very_verbose, all, tests):
+        if very_verbose:
+            logging.getLogger().level = logging.DEBUG
+        elif verbose:
+            logging.getLogger().level = logging.INFO
+        else:
+            logging.getLogger().level = logging.WARN
 
-        run_file = path.abspath(path.join(test_file_dir, "runtests.py"))
-        run_globals = {"__file__": run_file}
-        exec(compile(open(run_file).read(), run_file, 'exec'), run_globals)
+        passed = True
 
-        verbose = not quiet
-        return run_globals["run_tests"](tests, verbose)
+        print("Running tidy tests...")
+        passed = tidy.run_tests() and passed
+
+        print("Running WPT tests...")
+        passed = wpt.run_tests() and passed
+
+        if all or tests:
+            print("Running WebIDL tests...")
+            test_file_dir = path.abspath(path.join(PROJECT_TOPLEVEL_PATH, "components", "script",
+                                                   "dom", "bindings", "codegen", "parser"))
+            # For the `import WebIDL` in runtests.py
+            sys.path.insert(0, test_file_dir)
+            run_file = path.abspath(path.join(test_file_dir, "runtests.py"))
+            run_globals = {"__file__": run_file}
+            exec(compile(open(run_file).read(), run_file, 'exec'), run_globals)
+            passed = run_globals["run_tests"](tests, verbose or very_verbose) and passed
+
+        return 0 if passed else 1
 
     @Command('test-wpt-failure',
              description='Run the tests harness that verifies that the test failures are reported correctly',

python/tidy/__init__.py

@@ -8,4 +8,4 @@
 # except according to those terms.
 
 from .tidy import scan  # noqa
-from .test import do_tests  # noqa
+from .test import run_tests  # noqa

python/tidy/test.py

@@ -7,6 +7,7 @@
 # option. This file may not be copied, modified, or distributed
 # except according to those terms.
 
+import logging
 import os
 import unittest
 
@@ -48,7 +49,7 @@ class CheckTidiness(unittest.TestCase):
             os.path.join(BASE_PATH, "dir_check/webidl_plus"): ['webidl', 'test'],
             os.path.join(BASE_PATH, "dir_check/only_webidl"): ['webidl']
         }
-        errors = tidy.check_directory_files(dirs)
+        errors = tidy.check_directory_files(dirs, print_text=False)
         error_dir = os.path.join(BASE_PATH, "dir_check/webidl_plus")
         self.assertEqual("Unexpected extension found for test.rs. We only expect files with webidl, test extensions in {0}".format(error_dir), next(errors)[2])
         self.assertEqual("Unexpected extension found for test2.rs. We only expect files with webidl, test extensions in {0}".format(error_dir), next(errors)[2])
@@ -100,7 +101,7 @@ class CheckTidiness(unittest.TestCase):
         self.assertNoMoreErrors(errors)
 
     def test_apache2_incomplete(self):
-        errors = tidy.collect_errors_for_files(iterFile('apache2_license.rs'), [], [tidy.check_license])
+        errors = tidy.collect_errors_for_files(iterFile('apache2_license.rs'), [], [tidy.check_license], print_text=False)
         self.assertEqual('incorrect license', next(errors)[2])
 
     def test_rust(self):
@@ -249,19 +250,21 @@ class CheckTidiness(unittest.TestCase):
 
     def test_file_list(self):
         file_path = os.path.join(BASE_PATH, 'test_ignored')
-        file_list = tidy.FileList(file_path, only_changed_files=False, exclude_dirs=[])
+        file_list = tidy.FileList(file_path, only_changed_files=False, exclude_dirs=[], progress=False)
         lst = list(file_list)
         self.assertEqual([os.path.join(file_path, 'whee', 'test.rs'), os.path.join(file_path, 'whee', 'foo', 'bar.rs')], lst)
         file_list = tidy.FileList(file_path, only_changed_files=False,
-                                  exclude_dirs=[os.path.join(file_path, 'whee', 'foo')])
+                                  exclude_dirs=[os.path.join(file_path, 'whee', 'foo')],
+                                  progress=False)
         lst = list(file_list)
         self.assertEqual([os.path.join(file_path, 'whee', 'test.rs')], lst)
 
     def test_multiline_string(self):
-        errors = tidy.collect_errors_for_files(iterFile('multiline_string.rs'), [], [tidy.check_rust], print_text=True)
+        errors = tidy.collect_errors_for_files(iterFile('multiline_string.rs'), [], [tidy.check_rust], print_text=False)
         self.assertNoMoreErrors(errors)
 
 
-def do_tests():
+def run_tests():
+    verbosity = 1 if logging.getLogger().level >= logging.WARN else 2
     suite = unittest.TestLoader().loadTestsFromTestCase(CheckTidiness)
-    return 0 if unittest.TextTestRunner(verbosity=2).run(suite).wasSuccessful() else 1
+    return unittest.TextTestRunner(verbosity=verbosity).run(suite).wasSuccessful()

python/wpt/__init__.py

@@ -12,6 +12,8 @@ import sys
 
 import mozlog.commandline
 
+from . import test
+
 SCRIPT_PATH = os.path.abspath(os.path.dirname(__file__))
 SERVO_ROOT = os.path.abspath(os.path.join(SCRIPT_PATH, "..", ".."))
 WPT_PATH = os.path.join(SERVO_ROOT, "tests", "wpt")
@@ -62,3 +64,7 @@ def update_args_for_legacy_layout(kwargs: dict):
     kwargs["include_manifest"] = os.path.join(
         WPT_PATH, "include-legacy-layout.ini"
     )
+
+
+def run_tests():
+    return test.run_tests()

python/wpt/exporter/__init__.py

@@ -127,7 +127,7 @@ class SyncRun:
         return SyncRun.clean_up_body_text(body) + f"\nReviewed in {self.servo_pr}"
 
 
-@dataclasses.dataclass(kw_only=True)
+@dataclasses.dataclass()
 class WPTSync:
     servo_repo: str
     wpt_repo: str

python/wpt/requirements-dev.txt

@@ -1,4 +0,0 @@
--r requirements.txt
-
-flask
-types-requests

python/wpt/requirements.txt

@@ -1 +0,0 @@
-requests

python/wpt/test.py

@@ -26,30 +26,27 @@ import logging
 import os
 import shutil
 import subprocess
-import sys
 import tempfile
 import threading
 import time
 import unittest
 
 from functools import partial
-from typing import Any, Optional, Tuple
+from typing import Any, Optional, Tuple, Type
 from wsgiref.simple_server import WSGIRequestHandler, make_server
 
 import flask
 import flask.cli
 import requests
-from exporter import SyncRun, WPTSync
-from exporter.step import CreateOrUpdateBranchForPRStep
+
+from .exporter import SyncRun, WPTSync
+from .exporter.step import CreateOrUpdateBranchForPRStep
 
 TESTS_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "tests")
 SYNC: Optional[WPTSync] = None
 TMP_DIR: Optional[str] = None
 PORT = 9000
 
-if "-v" in sys.argv or "--verbose" in sys.argv:
-    logging.getLogger().level = logging.DEBUG
-
 
 @dataclasses.dataclass
 class MockPullRequest():
@@ -666,8 +663,16 @@ def tearDownModule():
     shutil.rmtree(TMP_DIR)
 
 
-if __name__ == "__main__":
-    # Uncomment this line to enable verbose logging.
-    # logging.getLogger().setLevel(logging.INFO)
+def run_tests():
+    verbosity = 1 if logging.getLogger().level >= logging.WARN else 2
 
-    unittest.main()
+    def run_suite(test_case: Type[unittest.TestCase]):
+        return unittest.TextTestRunner(verbosity=verbosity).run(
+            unittest.TestLoader().loadTestsFromTestCase(test_case)
+        ).wasSuccessful()
+
+    return all([
+        run_suite(TestApplyCommitsToWPT),
+        run_suite(TestCleanUpBodyText),
+        run_suite(TestFullSyncRun),
+    ])