Reorganization of Firefox-UI tests in mozilla-central

Matthew N. 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:
>> /browser/components/privatebrowsing/test/firefox_ui/
>> * safe browsing tests:
>> /browser/components/safebrowsing/content/test/firefox_ui/
>> * session store tests:
>> /browser/components/sessionstore/test/firefox_ui/
>> * 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 
you said.

>> 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 
prefix)
* `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 
confusion.

Thanks,
Matthew N. (:MattN)



More information about the firefox-dev mailing list