Reorganization of Firefox-UI tests in mozilla-central
MattN at mozilla.com
Thu Sep 1 23:15:43 UTC 2016
On 2016-09-01 9:24 AM, Gijs Kruitbosch wrote:
> (CC'ing firefox-dev which doesn't seem to have had the original email
> though it should have done, please follow up to m.d.platform.)
> On 01/09/2016 16:37, Henrik Skupin wrote:
>> * locationbar tests: /browser/base/content/test/urlbar/firefox_ui/
>> * private browsing tests:
>> * safe browsing tests:
>> * session store tests:
>> * update tests: /toolkit/mozapps/update/tests/firefox_ui/
Is there a plan to merge those with
/toolkit/mozapps/update/tests/marionette? It seems unusual to use both
when this new test suite is basically a layer on top of Marionette as
>> Do those locations sound good? I have heard at least once that
>> "firefox_ui" might not be the best choice as folder name, but that's how
>> the harness is called, and corresponds to what we have for other
>> harnesses too.
> As I did over IRC, I would like to strongly object to the continued use
> of per-test-type subfolders in our test directories. You can already use
> a specific mach command per test type, and the tests are listed in
> different manifests, *and* there's all the different filename
> conventions (browser_, test_....html, test_....xul, <whatever>.js) that
> further point out what type of test you're looking at. The subfolders
> add nothing useful.
As someone who has been adding the directory levels to
toolkit/components/passwordmgr/test/ recently, I disagree with this
since they add a grouping of relevant files making it more obvious which
files go with which test suites.
* Chrome mochitests, plain mochitests and xpcshell share the same prefix
(test_) so they are interleaved together in directory listings.
* Files that accompany tests have no prefix convention.
* head.js files have no indication of what suite they're for (i.e. no
* `mach test` doesn't support specifying a flavor of mochitest.
* Changing the subdirectory of my `mach mochitest` command is usually
faster than adding the flavor argument since the path is usually at the
end of the command. Since `mach test` doesn't support the flavor
argument I don't have to remember whether to use the argument or change
the path as I can always change the path when directories are used.
* xpcshell and browser-chrome both use "head.js" as the filename for
helpers though they run in very different contexts.
For a new contributor, having each suite in their own directory is much
less confusing/overwhelming for the above reasons.
Password Manager may be special in that it's using four different suites
so I'm not suggesting that every component needs to put their tests in a
subdirectory named after the test suite but I don't think we should be
dumping tests of different suites in one directory unless the
distinction between files would be clear to a newcomer.
> Furthermore, only the toolkit case is currently meaningfully split out
> into subdirs. The sessionstore test/ dir has a subdir (but also has a
> bundle of tests directly in that dir)
Sure, but there isn't a mix of multiple suites in one directory here.
> , and the privatebrowsing one has
> no leafs and only a subdir ("browser"). None of the others have any
> subdirs at all.
That just seems like good planning for the future when other suites are
used for that code. The paths of the tests would need to change.
> Getting back to the toolkit case, the subdirs are
> actually confusing because only some of the subdirs have tests (as a
> counterexample, "data" just has random helper files) and the root
> testing dir also has .cpp files in it (I guess for gtests?).
Nobody is saying that directories under a "test"/"tests" directory
should only include test file themselves. Having files to support tests
in organized directories doesn't seem like a problem to me.
> IOW, in the general case, I think that in most of those directories
> there's no precedent to do what you propose.
Having the new subdirectory in these specific cases may not fit but I
disagree that it's the wrong approach in general.
> Finally, "firefox_ui" (as well as "ui") as a name for a directory is
> going to cause all kinds of confusion for people exploring the repo
> without detailed knowledge of what's going on. Additionally, it's not
> like most of the mochitest-browser tests aren't tests of the Firefox
> UI... If we absolutely must have some kind of subdirectory because of
> reasons I have yet to hear, I think "integration" would be a better
> choice of name as far as subdirs of "test" go (as juxtaposed to "unit"
> for our xpcshell tests).
"firefox_ui", "ui", and "integration" all overlap with what
mochitest-browser-chrome is about IMO and I think naming the suite
"Firefox-UI" was confusing from the beginning. If I was a new
contributor working on a UI feature and decided I wanted to write tests,
I wouldn't want to be misled into thinking I should write a "Firefox-UI"
test when mochitest-browser-chrome is actually the desired suite. I
would suggest "puppeteer" or "marionette" for directory names to avoid
Matthew N. (:MattN)
More information about the firefox-dev