Re: Reorganization of Firefox-UI tests in mozilla-central
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, .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
Re: Reorganization of Firefox-UI tests in mozilla-central
Henrik Skupin writes: > 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. I would suggest s/firefox_ui/ui/g, or to eliminate the need for a sub-directory overall if it is obvious from the file names what they do. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Reorganization of Firefox-UI tests in mozilla-central
(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/
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, .js) that
further point out what type of test you're looking at. The subfolders
add nothing useful.
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), and the privatebrowsing one has
no leafs and only a subdir ("browser"). None of the others have any
subdirs at all. 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?).
IOW, in the general case, I think that in most of those directories
there's no precedent to do what you propose.
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).
~ Gijs
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Reorganization of Firefox-UI tests in mozilla-central
Hello, Via bug 1272145 we want to move our existing Firefox-UI tests from /testing/firefox-ui/ closer to the code which these are testing, so that the former location only contains the test harness and appropriate unit tests. Link to the current set of tests: https://dxr.mozilla.org/mozilla-central/source/testing/firefox-ui/tests For those of you who haven't heard about these tests yet, they are basically Marionette tests with an additional layer (firefox-puppeteer) on top to ease the test creation for ui specific checks. Benefits we have are restarting and quitting the browser, and running the tests in any localized build of Firefox. As listed below you can find the current type of tests and a proposed location: * 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/ 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. Locations for the following tests are not clear yet: * l10n tests (shortcuts): not clear yet (maybe under /browser/base/content/test/) * security tests: not clear yet L10n related tests mostly cover shortcuts and that the correct command is invoked to find failures like bug 1173735. Nearly all of the security tests are running checks against a real server with various SSL certificates (DV, OV, EV) and protocol versions. We will have a meeting soon to determine which of those tests are needed and which ones can be removed. So we might also be able to find the correct location for the tests. For now I would like to know if the proposal is fine or if we should go a completely different route. Thanks -- Henrik ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
