Closed Bug 1169410 Opened 9 years ago Closed 9 years ago

Add wpt support to |mach test|

Categories

(Testing :: web-platform-tests, defect)

defect
Not set
normal

Tracking

(firefox41 fixed)

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: jgraham, Assigned: jgraham)

Details

Attachments

(1 file)

Should allow

mach test web-platform-tests
mach test wpt
mach test testing/web-platform/tests/dom/historical.html
mach test /dom/historical.html
Bug 1169410 Add support for web-platform-tests to |mach test|
Attachment #8612509 - Flags: review?(gps)
Assignee: nobody → james
Comment on attachment 8612509 [details]
MozReview Request: Bug 1169410 Add support for web-platform-tests to |mach test|

https://reviewboard.mozilla.org/r/9589/#review8559

This looks mostly good. Just fix up the import code and this should be an easy r+ on the follow-up.

::: python/mozbuild/mozbuild/frontend/context.py:1303
(Diff revision 1)
> +        """List of [manifest_path, test_path] defining web-platform-tests.

Nit: Should use () not [] to denote tuples in docs.

::: python/mozbuild/mozbuild/frontend/emitter.py:1189
(Diff revision 1)
> +        old_path = sys.path[:]
> +        paths = []
> +        paths_file = os.path.join(context.config.topsrcdir, "testing",
> +                                  "web-platform", "tests", "tools", "localpaths.py")
> +        _globals = {"__file__": paths_file}
> +        execfile(paths_file, _globals)
> +        import manifest as wptmanifest
> +        sys.path = old_path

This definitely requires some comments.

::: python/mozbuild/mozbuild/frontend/emitter.py:1196
(Diff revision 1)
> +        sys.path = old_path

This should be a try..finally.

::: python/mozbuild/mozbuild/frontend/emitter.py:1198
(Diff revision 1)
> +        manifest = wptmanifest.manifest.load(tests_root, manifest_full_path)

How about extracting this logic for obtaining the wptmanifest module into a standalone function. While you are there, add some caching so we don't do all this work for every wpt manifest.

::: python/mozbuild/mozbuild/frontend/emitter.py:1219
(Diff revision 1)
> +                    'head': '',
> +                    'tail': '',
> +                    'support-files': '',

Using empty strings seems weird. Are you sure we do that instead of using [] or None?

::: testing/web-platform/moz.build:7
(Diff revision 1)
> +WEB_PLATFORM_TESTS_MANIFESTS += [('meta/MANIFEST.json', 'tests/'),
> +                                 ('mozilla/meta/MANIFEST.json', 'mozilla/tests/')]

Nit: newline after the [.

::: testing/web-platform/mach_commands.py:131
(Diff revision 1)
> +            del params["test_objects"]

I'm not a huge fan of mutating input like this. But it is probably acceptable.
Attachment #8612509 - Flags: review?(gps)
https://reviewboard.mozilla.org/r/9589/#review8561

> How about extracting this logic for obtaining the wptmanifest module into a standalone function. While you are there, add some caching so we don't do all this work for every wpt manifest.

In practice this function is only called once, so I don't think that adding caching is a win.

> Using empty strings seems weird. Are you sure we do that instead of using [] or None?

I literally just copied the reftests code above.
Comment on attachment 8612509 [details]
MozReview Request: Bug 1169410 Add support for web-platform-tests to |mach test|

Bug 1169410 Add support for web-platform-tests to |mach test|
Comment on attachment 8612509 [details]
MozReview Request: Bug 1169410 Add support for web-platform-tests to |mach test|

Bug 1169410 Add support for web-platform-tests to |mach test|
Attachment #8612509 - Flags: review?(gps)
Attachment #8612509 - Flags: review?(gps) → review+
Comment on attachment 8612509 [details]
MozReview Request: Bug 1169410 Add support for web-platform-tests to |mach test|

https://reviewboard.mozilla.org/r/9589/#review8765

Ship It!
https://hg.mozilla.org/mozilla-central/rev/ad8d961bafef
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.