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 <bgrinst...@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.

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

Thanks,
Brian

_______________________________________________
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

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

Reply via email to