Auto merge of #7815 - mbrubeck:test-ref-include, r=SimonSapin

Fixes for reftest command-line handling

r? @SimonSapin

* Allow reftest harness to take 0 or more testname arguments.
* Change `mach test-ref` parameter from `--name` to `--include`. This is consistent with other test suites, and also fixes a bug in `mach test` caused by a conflicting keyword parameter in `Registrar.dispatch`.
* Allow `mach test-ref` to take any number of `include` arguments.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7815)
<!-- Reviewable:end -->

python/servo/testing_commands.py

@@ -83,7 +83,7 @@ class MachCommands(CommandBase):
             ("tidy", {}),
             ("ref", {"kwargs": {"kind": render_mode},
                      "paths": [path.abspath(path.join("tests", "ref"))],
-                     "include_arg": "name"}),
+                     "include_arg": "include"}),
             ("wpt", {"kwargs": {"release": release},
                      "paths": [path.abspath(path.join("tests", "wpt", "web-platform-tests")),
                                path.abspath(path.join("tests", "wpt", "mozilla"))],
@@ -189,14 +189,14 @@ class MachCommands(CommandBase):
                      help=HELP_RENDER_MODE)
     @CommandArgument('--release', '-r', action='store_true',
                      help='Run with a release build of Servo')
-    @CommandArgument('--name', default=None,
+    @CommandArgument('--include', default=None, nargs='+',
                      help="Only run tests that match this pattern. If the "
                           "path to the ref test directory is included, it "
                           "will automatically be trimmed out.")
     @CommandArgument(
         'servo_params', default=None, nargs=argparse.REMAINDER,
         help="Command-line arguments to be passed through to Servo")
-    def test_ref(self, kind=DEFAULT_RENDER_MODE, name=None, servo_params=None,
+    def test_ref(self, kind=DEFAULT_RENDER_MODE, include=None, servo_params=None,
                  release=False):
         self.ensure_bootstrapped()
         self.ensure_built_tests(release=release)
@@ -210,17 +210,17 @@ class MachCommands(CommandBase):
         for k in kinds:
             print("Running %s reftests..." % k)
             test_args = [k, test_path]
-            if name is not None:
-                maybe_path = path.normpath(name)
+            if include is not None:
                 ref_path = path.join("tests", "ref")
-
-                # Check to see if we were passed something leading with the
-                # path to the ref test directory, and trim it so that reftest
-                # knows how to filter it.
-                if ref_path in maybe_path:
-                    test_args.append(path.relpath(maybe_path, ref_path))
-                else:
-                    test_args.append(name)
+                for name in include:
+                    # Check to see if we were passed something leading with the
+                    # path to the ref test directory, and trim it so that reftest
+                    # knows how to filter it.
+                    maybe_path = path.normpath(name)
+                    if ref_path in maybe_path:
+                        test_args.append(path.relpath(maybe_path, ref_path))
+                    else:
+                        test_args.append(name)
             if servo_params is not None:
                 test_args += ["--"] + servo_params
             ret = self.run_test("reftest", test_args, release=release)

tests/reftest.rs

@@ -48,11 +48,10 @@ fn main() {
     let harness_args = parts.next().unwrap();  // .split() is never empty
     let servo_args = parts.next().unwrap_or(&[]);
 
-    let (render_mode_string, base_path, testname) = match harness_args {
-        [] | [_] => panic!("USAGE: cpu|gpu base_path [testname regex]"),
-        [ref render_mode_string, ref base_path] => (render_mode_string, base_path, None),
-        [ref render_mode_string, ref base_path, ref testname, ..] =>
-            (render_mode_string, base_path, Some(testname.clone())),
+    let (render_mode_string, base_path, testnames) = match harness_args {
+        [ref render_mode_string, ref base_path, testnames..] =>
+            (render_mode_string, base_path, testnames),
+        _ => panic!("USAGE: cpu|gpu base_path [testname ...]"),
     };
 
     let mut render_mode = match &**render_mode_string {
@@ -79,7 +78,8 @@ fn main() {
         match maybe_extension {
             Some(extension) => {
                 if extension == OsStr::new("list") && file.is_file() {
-                    let mut tests = parse_lists(&file, servo_args, render_mode, all_tests.len());
+                    let len = all_tests.len();
+                    let mut tests = parse_lists(&file, testnames, servo_args, render_mode, len);
                     println!("\t{} [{} tests]", file.display(), tests.len());
                     all_tests.append(&mut tests);
                 }
@@ -89,7 +89,7 @@ fn main() {
     }
 
     let test_opts = TestOpts {
-        filter: testname,
+        filter: None,
         run_ignored: false,
         logfile: None,
         run_tests: true,
@@ -165,7 +165,12 @@ struct TestLine<'a> {
     file_right: &'a str,
 }
 
-fn parse_lists(file: &Path, servo_args: &[String], render_mode: RenderMode, id_offset: usize) -> Vec<TestDescAndFn> {
+fn parse_lists(file: &Path,
+               filters: &[String],
+               servo_args: &[String],
+               render_mode: RenderMode,
+               id_offset: usize)
+               -> Vec<TestDescAndFn> {
     let mut tests = Vec::new();
     let contents = {
         let mut f = File::open(file).unwrap();
@@ -254,7 +259,9 @@ fn parse_lists(file: &Path, servo_args: &[String], render_mode: RenderMode, id_o
             pixel_ratio: pixel_ratio,
         };
 
-        tests.push(make_test(reftest));
+        if filters.is_empty() || filters.iter().any(|pattern| reftest.name.contains(pattern)) {
+            tests.push(make_test(reftest));
+        }
     }
     tests
 }