> On Apr 1, 2019, at 12:15 PM, Boris Zbarsky <bzbar...@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
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
