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

> On Apr 1, 2019, at 3:13 PM, Steve Fink <sf...@mozilla.com> 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.)
> 
> 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! ;-)
> 
> 
> _______________________________________________
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Reply via email to