Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Creating a new mach command for adding tests

88 views
Skip to first unread message

Brian Grinstead

unread,
Apr 1, 2019, 2:36:51 PM4/1/19
to Mozilla dev-platform mailing list mailing list
Based on my own experience and discussions with others, the workflow for adding new mochitests isn't great. Commonly, it looks like: "copy/paste a test in the same directory, add the new test to the relevant manifest file, empty out the actual test bits, write your test". In my experience this is prone to issues like forgetting to add the new test to the manifest, or not fully replacing boilerplate like bug numbers from the copied test.

There's a script in tree I was unaware of until last week called gen_template.pl that's intended to help here, but it does leave a few issues open:

1) It doesn't help with finding the manifest file and adding the new test to it.
2) The boilerplate it generates is outdated (for example, it sets type="application/javascript" even in HTML documents, it doesn't include add_task, etc).
3) It supports only mochitest-chrome and mochitest-plain.

Last week I prototyped a new mach command to fix (1) and (2), and expand (3) to include browser-chrome mochitests. If it's helpful, it could be extended to more test types as well. When you run the command it will create a file with the appropriate boilerplate and add it to the manifest file (chrome.ini, mochitest.ini, browser.ini depending on the type). This way you can immediately run the test with `./mach mochitest`. It looks like this:

```
# Chrome mochitests
./mach addtest js/xpconnect/tests/chrome/test_chrome.html
./mach addtest js/xpconnect/tests/chrome/test_chrome.xhtml
./mach addtest js/xpconnect/tests/chrome/test_chrome.xul

# Plain mochitests
./mach addtest js/xpconnect/tests/mochitest/test_plain.html
./mach addtest js/xpconnect/tests/mochitest/test_plain.xhtml
./mach addtest js/xpconnect/tests/mochitest/test_plain.xul

# Browser mochitests
./mach addtest browser/base/content/test/alerts/browser_mochitest.js
```

It's not landed because I’d like to get some feedback (see next paragraph), but if you'd like to try it locally it can be applied from https://phabricator.services.mozilla.com/D25482.

This change modifies the existing templates used by gen_template.pl and removes the perl script. I think the template changes are mostly uncontroversial, but there is one notable change that I'd like feedback on before landing. We currently add a bug number to new tests generated with the script (in 4 places). For example, see "{BUGNUMBER}" in https://searchfox.org/mozilla-central/source/testing/mochitest/static/chrome.template.txt. My patch removes this. The reasoning is:

1) To tighten up the boilerplate and prevent the bug number from not being updated in any or all of the spots when a test is created via copy/paste.
2) Require less information up front when generating the test (in case you are building a test before filing a bug, or have a test not associated with any one particular bug).

Links to bugs/comments/etc can be added in the test if they are relevant, but I don't know that it's important enough to add another step in front of getting a useful test case built. I did also consider adding a TODO comment in the template to add a bug link (though in a single place instead of 4), but not to require that information up front.

If I don’t hear objections on this point I’ll work towards getting it landed soon.

Thanks,
Brian

Jared Hirsch

unread,
Apr 1, 2019, 2:55:21 PM4/1/19
to Brian Grinstead, Mozilla dev-platform mailing list mailing list
This sounds great! I land features in the tree periodically, but
infrequently enough to forget lots of little details. It would really save
time (mine and the time of people I ping on IRC for help...) if our tooling
simplified the process of creating new tests.

I don't have any historical context on the reasons why the bug number was
added to the template, but removing it seems reasonable to me, fwiw.

One small feature request: could you add some basic usage documentation to
the firefox-source-docs as part of this patch? I'm sure I'll forget the
right sequence of commands in between uses ;-)

Cheers,

Jared


On Mon, Apr 1, 2019 at 11:36 AM Brian Grinstead <bgrin...@mozilla.com>
wrote:
> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>

Boris Zbarsky

unread,
Apr 1, 2019, 3:15:07 PM4/1/19
to
On 4/1/19 2:36 PM, Brian Grinstead wrote:
> When you run the command it will create a file with the appropriate boilerplate and add it to the manifest file (chrome.ini, mochitest.ini, browser.ini depending on the type).

Brian, thank you for putting this together!

I have one concern with the mach bits themselves: It looks like the way
the type-detection works is that it looks for "browser_" in the test
name, and if that's present uses browser.ini. Otherwise it uses
chrome.ini if it's present in the dir, else mochitest.ini if it's
present, else errors out.

We have a fair number of dirs that have both a chrome.ini _and_ a
mochitest.ini, and defaulting to chrome.ini for those dirs seems odd.
It might be better to error out of the filename is test_foo and the dir
has both chrome.ini and mochitest.ini and tell the developer to pick one
or the other explicitly.

> Links to bugs/comments/etc can be added in the test if they are relevant, but I don't know that it's important enough to add another step in front of getting a useful test case built. I did also consider adding a TODO comment in the template to add a bug link (though in a single place instead of 4), but not to require that information up front.

I think realistically getting to the bug through the version control
history is reasonable, so there's not that much reason to have a bug
link in the test itself.

I would further argue that the <title> in just about all our mochitests
is pointless and could go from the template.

I do have one other question on the templates: for mochitest-plain,
add_task is a pretty rare thing to do, so I'm not sure defaulting the
template to that makes sense. For mochitest-chrome I'm not really sure;
for mochitest-browse I agree that defaulting to add_task makes sense.

For the cases where we don't default to add_task (if any) we probably
shouldn't include AddTask.js either, like we don't include other things
that are helpful in only some test files (EventUtils.js, etc).

-Boris

Felipe G

unread,
Apr 1, 2019, 3:22:05 PM4/1/19
to Jared Hirsch, Brian Grinstead, dev-pl...@lists.mozilla.org
Something that would be a nice flare would be if it got the information
from the moz.build file to tell which component the new file would be
related to.

Definitely not essential, though.

And a question: will it also work if you're already in the right dir and
just specifies the filename? That's usually how I do it.
All the examples were from the repo root dir so I thought to double check.
> > extended to more test types as well. When you run the command it will
> > create a file with the appropriate boilerplate and add it to the manifest
> > Links to bugs/comments/etc can be added in the test if they are relevant,
> > but I don't know that it's important enough to add another step in front
> of
> > getting a useful test case built. I did also consider adding a TODO
> comment
> > in the template to add a bug link (though in a single place instead of
> 4),
> > but not to require that information up front.
> >

Brian Grinstead

unread,
Apr 1, 2019, 3:38:18 PM4/1/19
to Boris Zbarsky, Mozilla dev-platform mailing list mailing list


> On Apr 1, 2019, at 12:15 PM, Boris Zbarsky <bzba...@mit.edu> wrote:
>
> On 4/1/19 2:36 PM, Brian Grinstead wrote:
>> When you run the command it will create a file with the appropriate boilerplate and add it to the manifest file (chrome.ini, mochitest.ini, browser.ini depending on the type).
>
> Brian, thank you for putting this together!
>
> I have one concern with the mach bits themselves: It looks like the way the type-detection works is that it looks for "browser_" in the test name, and if that's present uses browser.ini. Otherwise it uses chrome.ini if it's present in the dir, else mochitest.ini if it's present, else errors out.
>
> We have a fair number of dirs that have both a chrome.ini _and_ a mochitest.ini, and defaulting to chrome.ini for those dirs seems odd. It might be better to error out of the filename is test_foo and the dir has both chrome.ini and mochitest.ini and tell the developer to pick one or the other explicitly.

Good catch, just fixed this in the latest version on phab:

```
> ./mach addtest toolkit/components/windowcreator/test/test_chrome.html
Error: directory contains both a chrome.ini and mochitest.ini. Please specify either `chrome` or `plain` with the --type argument.
> ./mach addtest toolkit/components/windowcreator/test/test_chrome.html --type="chrome"
Adding a test of type `chrome` and doc type `html`
Please make sure to add the new test to your commit. You can now run the test with:
./mach mochitest toolkit/components/windowcreator/test/test_chrome.html
```

>> Links to bugs/comments/etc can be added in the test if they are relevant, but I don't know that it's important enough to add another step in front of getting a useful test case built. I did also consider adding a TODO comment in the template to add a bug link (though in a single place instead of 4), but not to require that information up front.
>
> I think realistically getting to the bug through the version control history is reasonable, so there's not that much reason to have a bug link in the test itself.
>
> I would further argue that the <title> in just about all our mochitests is pointless and could go from the template.

Would be happy to remove that - I was wondering why it was useful when I started changing the templates and couldn’t think of a great reason.

> I do have one other question on the templates: for mochitest-plain, add_task is a pretty rare thing to do, so I'm not sure defaulting the template to that makes sense. For mochitest-chrome I'm not really sure; for mochitest-browse I agree that defaulting to add_task makes sense.
>
> For the cases where we don't default to add_task (if any) we probably shouldn't include AddTask.js either, like we don't include other things that are helpful in only some test files (EventUtils.js, etc).

add_task does seem relatively rare on chrome right now, but I don’t think it should be. It cleans up the boilerplate (no need to call SimpleTest.waitForExplicitFinish and SimpleTest.finish, no need to figure out how to hook up [onload], and gives async tests right off the bat). I’m not as familiar with plain test, but I’d be fine to drop that from the template if it’s not as useful.

I also think we should import the AddTask.js contents into SimpleTest.js to make it available without an extra script, but that’s another discussion/bug :).

Brian


Brian Grinstead

unread,
Apr 1, 2019, 3:47:42 PM4/1/19
to Felipe G, Jared Hirsch, dev-pl...@lists.mozilla.org

> On Apr 1, 2019, at 12:23 PM, Felipe G <fel...@gmail.com> wrote:
>
> Something that would be a nice flare would be if it got the information from the moz.build file to tell which component the new file would be related to.
>
> Definitely not essential, though.

I think that would be doable, though I don’t know off-hand the right python modules to use to get the information.

> And a question: will it also work if you're already in the right dir and just specifies the filename? That's usually how I do it.
> All the examples were from the repo root dir so I thought to double check.

I just checked and it does seem to work fine from a subfolder (assuming mach is on your path).
> > extended to more test types as well. When you run the command it will
> > create a file with the appropriate boilerplate and add it to the manifest
> > Links to bugs/comments/etc can be added in the test if they are relevant,
> > but I don't know that it's important enough to add another step in front of
> > getting a useful test case built. I did also consider adding a TODO comment
> > in the template to add a bug link (though in a single place instead of 4),
> > but not to require that information up front.
> >

Brian Grinstead

unread,
Apr 1, 2019, 3:51:36 PM4/1/19
to Boris Zbarsky, Mozilla dev-platform mailing list mailing list

> On Apr 1, 2019, at 12:38 PM, Brian Grinstead <bgrin...@mozilla.com> wrote:
>>> Links to bugs/comments/etc can be added in the test if they are relevant, but I don't know that it's important enough to add another step in front of getting a useful test case built. I did also consider adding a TODO comment in the template to add a bug link (though in a single place instead of 4), but not to require that information up front.
>>
>> I think realistically getting to the bug through the version control history is reasonable, so there's not that much reason to have a bug link in the test itself.
>>
>> I would further argue that the <title> in just about all our mochitests is pointless and could go from the template.
>
> Would be happy to remove that - I was wondering why it was useful when I started changing the templates and couldn’t think of a great reason.

I realize now that <title> is used to display a summary of the test from searchfox listing, so perhaps we shouldn't remove it. For example: https://searchfox.org/mozilla-central/rev/201450283cddc9e409cec707acb65ba6cf6037b1/accessible/tests/mochitest/test_descr.html#4 sets the title as "nsIAccessible::description tests”, which can also be seen in the directory listing at https://searchfox.org/mozilla-central/source/accessible/tests/mochitest.

Brian

Brian Grinstead

unread,
Apr 1, 2019, 3:58:20 PM4/1/19
to Jared Hirsch, Mozilla dev-platform mailing list mailing list


> On Apr 1, 2019, at 11:54 AM, Jared Hirsch <6a...@mozilla.com> wrote:
>
> This sounds great! I land features in the tree periodically, but infrequently enough to forget lots of little details. It would really save time (mine and the time of people I ping on IRC for help...) if our tooling simplified the process of creating new tests.
>
> I don't have any historical context on the reasons why the bug number was added to the template, but removing it seems reasonable to me, fwiw.
>
> One small feature request: could you add some basic usage documentation to the firefox-source-docs as part of this patch? I'm sure I'll forget the right sequence of commands in between uses ;-)

Glad to hear this would save time, makes me more sure I should push it forward.

For now I was planning to update the existing test template docs at https://developer.mozilla.org/en-US/docs/Mozilla/Projects/Mochitest#Test_templates to use the mach command, since that’s where a lot of related APIs are already documented. I’m open to the argument that this entire page should live on firefox-source-docs, but would like to decouple that from this command if possible :).

Brian

> Cheers,
>
> Jared
>
>
> On Mon, Apr 1, 2019 at 11:36 AM Brian Grinstead <bgrin...@mozilla.com> wrote:
> Based on my own experience and discussions with others, the workflow for adding new mochitests isn't great. Commonly, it looks like: "copy/paste a test in the same directory, add the new test to the relevant manifest file, empty out the actual test bits, write your test". In my experience this is prone to issues like forgetting to add the new test to the manifest, or not fully replacing boilerplate like bug numbers from the copied test.
>
> There's a script in tree I was unaware of until last week called gen_template.pl that's intended to help here, but it does leave a few issues open:
>
> 1) It doesn't help with finding the manifest file and adding the new test to it.
> 2) The boilerplate it generates is outdated (for example, it sets type="application/javascript" even in HTML documents, it doesn't include add_task, etc).
> 3) It supports only mochitest-chrome and mochitest-plain.
>
> Last week I prototyped a new mach command to fix (1) and (2), and expand (3) to include browser-chrome mochitests. If it's helpful, it could be extended to more test types as well. When you run the command it will create a file with the appropriate boilerplate and add it to the manifest file (chrome.ini, mochitest.ini, browser.ini depending on the type). This way you can immediately run the test with `./mach mochitest`. It looks like this:
>
> ```
> # Chrome mochitests
> ./mach addtest js/xpconnect/tests/chrome/test_chrome.html
> ./mach addtest js/xpconnect/tests/chrome/test_chrome.xhtml
> ./mach addtest js/xpconnect/tests/chrome/test_chrome.xul
>
> # Plain mochitests
> ./mach addtest js/xpconnect/tests/mochitest/test_plain.html
> ./mach addtest js/xpconnect/tests/mochitest/test_plain.xhtml
> ./mach addtest js/xpconnect/tests/mochitest/test_plain.xul
>
> # Browser mochitests
> ./mach addtest browser/base/content/test/alerts/browser_mochitest.js
> ```
>
> It's not landed because I’d like to get some feedback (see next paragraph), but if you'd like to try it locally it can be applied from https://phabricator.services.mozilla.com/D25482.
>
> This change modifies the existing templates used by gen_template.pl and removes the perl script. I think the template changes are mostly uncontroversial, but there is one notable change that I'd like feedback on before landing. We currently add a bug number to new tests generated with the script (in 4 places). For example, see "{BUGNUMBER}" in https://searchfox.org/mozilla-central/source/testing/mochitest/static/chrome.template.txt. My patch removes this. The reasoning is:
>
> 1) To tighten up the boilerplate and prevent the bug number from not being updated in any or all of the spots when a test is created via copy/paste.
> 2) Require less information up front when generating the test (in case you are building a test before filing a bug, or have a test not associated with any one particular bug).
>
> Links to bugs/comments/etc can be added in the test if they are relevant, but I don't know that it's important enough to add another step in front of getting a useful test case built. I did also consider adding a TODO comment in the template to add a bug link (though in a single place instead of 4), but not to require that information up front.
>

Steve Fink

unread,
Apr 1, 2019, 6:13:54 PM4/1/19
to dev-pl...@lists.mozilla.org
On 4/1/19 11:36 AM, Brian Grinstead wrote:
> Based on my own experience and discussions with others, the workflow for adding new mochitests isn't great. Commonly, it looks like: "copy/paste a test in the same directory, add the new test to the relevant manifest file, empty out the actual test bits, write your test". In my experience this is prone to issues like forgetting to add the new test to the manifest, or not fully replacing boilerplate like bug numbers from the copied test.
>
> There's a script in tree I was unaware of until last week called gen_template.pl that's intended to help here, but it does leave a few issues open:
>
> 1) It doesn't help with finding the manifest file and adding the new test to it.
> 2) The boilerplate it generates is outdated (for example, it sets type="application/javascript" even in HTML documents, it doesn't include add_task, etc).
> 3) It supports only mochitest-chrome and mochitest-plain.
>
> Last week I prototyped a new mach command to fix (1) and (2), and expand (3) to include browser-chrome mochitests. If it's helpful, it could be extended to more test types as well. When you run the command it will create a file with the appropriate boilerplate and add it to the manifest file (chrome.ini, mochitest.ini, browser.ini depending on the type). This way you can immediately run the test with `./mach mochitest`.

It sounds great to me, but I'm wondering if the generic name is
intentional or not. Various groups within Mozilla assume different
things by 'test'. Is`mach addtest` intended to only be for mochitests?
If so, then perhaps `mach addmochitest` is a better name, even it's a
bit of mouthful. My reasoning is that there's already a distinction
between `mach mochitest` and `mach test`, where the latter attempts to
be general and support a bunch of different kinds of tests. Having `mach
test` assume mochitests would be highly confusing to me, at least.
(Though I'm not sure that `mach test` really works; it seems like I
usually have to run the more specific command.)

Following the existing model exactly would mean adding `mach
addmochitest`, and additionally `mach addtest` that errors out if the
path you give it leads to a non-mochitest directory.

Or you could go for the even more generic `mach add mochitest` ... then
you could implement adding all kinds of things! ;-)


Brian Grinstead

unread,
Apr 1, 2019, 6:37:09 PM4/1/19
to Steve Fink, Mozilla dev-platform mailing list mailing list
I was hoping that other types could be added over time, so making the name generic was intentional. It will already error if you try to add something that doesn’t look like a mochitest, but it’s a good suggestion to make that more obvious. I can file bugs for other test types and then include the bug # for in the error message to make that more likely to happen. I think this command is conceptually similar to `./mach test`, and we just won't have a direct analog to `./mach mochitest` - this would be done via something like `./mach addtest —type=browser` if you needed to explicitly set the type.

Brian

James Graham

unread,
Apr 2, 2019, 5:40:32 AM4/2/19
to dev-pl...@lists.mozilla.org
I'm also pretty worried by this. For web-platform features ensuring
interop is critical and as such web-platform-tests should be preferred
over mochitests where possible. But every time we build features with a
mochitest-first approach it undermines that.

For web-platform-tests we already have ./mach wpt-create, so I think we
should either roll that functionality in to the new command as part of
the initial featureset or have one command per supported test type (i.e.
call this mach mochitest-create).

Honza Bambas

unread,
Apr 2, 2019, 9:03:35 AM4/2/19
to Jared Hirsch, Brian Grinstead, Mozilla dev-platform mailing list mailing list
This sounds great!  One nit - could this be done also for unit
(xpcshell) tests?  good examples are in netwerk/test/unit/ and unit_ipc/.
Thanks!
-hb-

On 2019-04-01 20:54, Jared Hirsch wrote:
> This sounds great! I land features in the tree periodically, but
> infrequently enough to forget lots of little details. It would really save
> time (mine and the time of people I ping on IRC for help...) if our tooling
> simplified the process of creating new tests.
>
> I don't have any historical context on the reasons why the bug number was
> added to the template, but removing it seems reasonable to me, fwiw.
>
> One small feature request: could you add some basic usage documentation to
> the firefox-source-docs as part of this patch? I'm sure I'll forget the
> right sequence of commands in between uses ;-)
>
> Cheers,
>
> Jared
>
>
> On Mon, Apr 1, 2019 at 11:36 AM Brian Grinstead <bgrin...@mozilla.com>
> wrote:
>
>> Based on my own experience and discussions with others, the workflow for
>> adding new mochitests isn't great. Commonly, it looks like: "copy/paste a
>> test in the same directory, add the new test to the relevant manifest file,
>> empty out the actual test bits, write your test". In my experience this is
>> prone to issues like forgetting to add the new test to the manifest, or not
>> fully replacing boilerplate like bug numbers from the copied test.
>>
>> There's a script in tree I was unaware of until last week called
>> gen_template.pl that's intended to help here, but it does leave a few
>> issues open:
>>
>> 1) It doesn't help with finding the manifest file and adding the new test
>> to it.
>> 2) The boilerplate it generates is outdated (for example, it sets
>> type="application/javascript" even in HTML documents, it doesn't include
>> add_task, etc).
>> 3) It supports only mochitest-chrome and mochitest-plain.
>>
>> Last week I prototyped a new mach command to fix (1) and (2), and expand
>> (3) to include browser-chrome mochitests. If it's helpful, it could be
>> extended to more test types as well. When you run the command it will
>> create a file with the appropriate boilerplate and add it to the manifest
>> file (chrome.ini, mochitest.ini, browser.ini depending on the type). This

Brian Grinstead

unread,
Apr 2, 2019, 2:12:11 PM4/2/19
to James Graham, dev-pl...@lists.mozilla.org
I don't think that having papercuts in the workflow for writing one type of test is the right way to nudge developers into writing another type. It also doesn't seem effective - otherwise people would be using the wpt-create tool to avoid jumping through hoops to add a mochitest.

That said, given there’s already a convention for this perhaps the tool as-is would be better named `./mach mochitest-create`. Based on Steve’s suggestion, if we did want a single API we could do something like:

# Attempt to automatically determine the type of test (mochitest-chrome, xpcshell, wpt, etc)
`./mach addtest path/to/test`

# If you want to pass extra arguments specific to that type, then you use a subcommand:
`./mach addtest mochitest --flavor=chrome toolkit/components/windowcreator/test/test_chrome.html`
`./mach addtest wpt testing/web-platform/tests/accelerometer/test.html --long-timeout`

Brian

> On Apr 2, 2019, at 2:40 AM, James Graham <ja...@hoppipolla.co.uk> wrote:
>
> On 01/04/2019 23:13, Steve Fink wrote:
>> On 4/1/19 11:36 AM, Brian Grinstead wrote:
>>> Based on my own experience and discussions with others, the workflow for adding new mochitests isn't great. Commonly, it looks like: "copy/paste a test in the same directory, add the new test to the relevant manifest file, empty out the actual test bits, write your test". In my experience this is prone to issues like forgetting to add the new test to the manifest, or not fully replacing boilerplate like bug numbers from the copied test.
>>>
>>> There's a script in tree I was unaware of until last week called gen_template.pl that's intended to help here, but it does leave a few issues open:
>>>
>>> 1) It doesn't help with finding the manifest file and adding the new test to it.
>>> 2) The boilerplate it generates is outdated (for example, it sets type="application/javascript" even in HTML documents, it doesn't include add_task, etc).
>>> 3) It supports only mochitest-chrome and mochitest-plain.
>>>
>>> Last week I prototyped a new mach command to fix (1) and (2), and expand (3) to include browser-chrome mochitests. If it's helpful, it could be extended to more test types as well. When you run the command it will create a file with the appropriate boilerplate and add it to the manifest file (chrome.ini, mochitest.ini, browser.ini depending on the type). This way you can immediately run the test with `./mach mochitest`.
>> It sounds great to me, but I'm wondering if the generic name is intentional or not. Various groups within Mozilla assume different things by 'test'. Is`mach addtest` intended to only be for mochitests? If so, then perhaps `mach addmochitest` is a better name, even it's a bit of mouthful. My reasoning is that there's already a distinction between `mach mochitest` and `mach test`, where the latter attempts to be general and support a bunch of different kinds of tests. Having `mach test` assume mochitests would be highly confusing to me, at least. (Though I'm not sure that `mach test` really works; it seems like I usually have to run the more specific command.)
>
> I'm also pretty worried by this. For web-platform features ensuring interop is critical and as such web-platform-tests should be preferred over mochitests where possible. But every time we build features with a mochitest-first approach it undermines that.
>
> For web-platform-tests we already have ./mach wpt-create, so I think we should either roll that functionality in to the new command as part of the initial featureset or have one command per supported test type (i.e. call this mach mochitest-create).

James Graham

unread,
Apr 3, 2019, 5:47:12 AM4/3/19
to Brian Grinstead, dev-pl...@lists.mozilla.org
On 02/04/2019 19:11, Brian Grinstead wrote:
> I don't think that having papercuts in the workflow for writing one type of test is the right way to nudge developers into writing another type. It also doesn't seem effective - otherwise people would be using the wpt-create tool to avoid jumping through hoops to add a mochitest.

To be clear, my intent was not to add papercuts to any workflow; I'd
really like all our developer workflows to be as ergonomic as possible,
and adding better tooling to help people create tests seems like a great
idea.

That said, there's a pattern that we've often fallen into where we make
broadly applicable improvements to only a single testsuite — often
mochitest — and then consider it to be job done. Although each change
individually is small, over time it adds up and re-enforces an implied
hierarchy of testsuites that doesn't match current best practices.

> That said, given there’s already a convention for this perhaps the tool as-is would be better named `./mach mochitest-create`. Based on Steve’s suggestion, if we did want a single API we could do something like:
>
> # Attempt to automatically determine the type of test (mochitest-chrome, xpcshell, wpt, etc)
> `./mach addtest path/to/test`
>
> # If you want to pass extra arguments specific to that type, then you use a subcommand:
> `./mach addtest mochitest --flavor=chrome toolkit/components/windowcreator/test/test_chrome.html`
> `./mach addtest wpt testing/web-platform/tests/accelerometer/test.html --long-timeout`

I think the idea of a single mach command for test creation that, as far
as possible, guesses the test type from its location is great. I'd be
happy to provide whatever support is needed to make this replace the
wpt-specific command.

Brian Grinstead

unread,
Apr 11, 2019, 1:22:17 PM4/11/19
to Mozilla dev-platform mailing list mailing list
This has now landed (with initial support for xpcshell, mochitests, and web platform tests). Thanks to Andrew Halberstadt and James Graham for improving upon the initial prototype and making it easier to extend to new suites.

If the suite supports it (currently xpcshell and mochitests), we will also automatically write the new test into the manifest file. This means you can immediately run the test after running addtest - for example:

./mach addtest browser/components/extensions/test/xpcshell/test_xpcshell.js && ./mach test test_xpcshell.js

The current global options are:
--overwrite: recreate the file even if it already exists
--editor: open the test in the specified editor (or default editor if it’s passed without an argument)
--suite: explicitly specify the test suite (you generally shouldn’t need this since it will detect from the path)
--doc: explicitly specify the document type (you generally shouldn’t need this since it will detect from the path)

If you’d like to add support for a new suite or find bugs / missing features, please block Bug 1540285.

Brian

> On Apr 1, 2019, at 11:36 AM, Brian Grinstead <bgrin...@mozilla.com> wrote:
>
> Based on my own experience and discussions with others, the workflow for adding new mochitests isn't great. Commonly, it looks like: "copy/paste a test in the same directory, add the new test to the relevant manifest file, empty out the actual test bits, write your test". In my experience this is prone to issues like forgetting to add the new test to the manifest, or not fully replacing boilerplate like bug numbers from the copied test.
>
> There's a script in tree I was unaware of until last week called gen_template.pl that's intended to help here, but it does leave a few issues open:
>
> 1) It doesn't help with finding the manifest file and adding the new test to it.
> 2) The boilerplate it generates is outdated (for example, it sets type="application/javascript" even in HTML documents, it doesn't include add_task, etc).
> 3) It supports only mochitest-chrome and mochitest-plain.
>

James Graham

unread,
Apr 11, 2019, 1:29:26 PM4/11/19
to dev-pl...@lists.mozilla.org
On 11/04/2019 18:22, Brian Grinstead wrote:
> This has now landed (with initial support for xpcshell, mochitests, and web platform tests). Thanks to Andrew Halberstadt and James Graham for improving upon the initial prototype and making it easier to extend to new suites.

Eager users should note that, as I write, the wpt support is in a commit
that is on autoland but not yet merged to central.

Once that has merged this new tool will replace the `mach wpt-create`
command.
0 new messages