The idea is that we should be sharing things across all of the devtools tests (imports / cleanup functions / utilities like addTab, getFrameScript, etc). I believe this can be done by using an existing head.js file (like devtools/framework/head.js) and then having it import a directory-head.js file from each individual directory. Then each of the folders that wanted to share this head.js would just update their browser.ini to point to the new one and then add a directory-head.js file with any specific functions.

What do you think? This is my proposed path forward for one head.js file to rule them all - just starting with the eyedropper directory since it's easy.

Things that I'd like to see copied over in a general case is pref resetting: https://github.com/mozilla/gecko-dev/blob/53d1e1a9e5f5025135ec3722c5ee4d3bb542ccc1/browser/devtools/performance/test/head.js#L47-L64 and gets cleared up in the unregister function.

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #2) > Things that I'd like to see copied over in a general case is pref resetting: > https://github.com/mozilla/gecko-dev/blob/ > 53d1e1a9e5f5025135ec3722c5ee4d3bb542ccc1/browser/devtools/performance/test/ > head.js#L47-L64 > and gets cleared up in the unregister function. You mean some way to declare a list of prefs / values and have the main head.js file manage setting them before the test and then resetting them afterwards? Or that the shared file would maintain such a list? Sidenote: looks like there are lots of nice utility functions in the performance head.js file that we could pull into the shared head.js

Comment on attachment 8607855 [details] [diff] [review] test-cleanup.patch Review of attachment 8607855 [details] [diff] [review]: ----------------------------------------------------------------- The idea seems great! I think I would prefer to invert it though, so that the "main" head.js for a directory is the one in the directory and it would remain named "head.js" as it is today. More specifically: 1. The framework / shared head would be renamed to "shared-head.js" 2. In eyedropper, "head.js" would import "shared-head.js" from framework This way, directories still have a "head.js" like the rest of Gecko test dirs, and each directory's head is in control of the importing. What do you think?

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #4) > Comment on attachment 8607855 [details] [diff] [review] > test-cleanup.patch > > Review of attachment 8607855 [details] [diff] [review]: > ----------------------------------------------------------------- > > The idea seems great! > > I think I would prefer to invert it though, so that the "main" head.js for a > directory is the one in the directory and it would remain named "head.js" as > it is today. More specifically: > > 1. The framework / shared head would be renamed to "shared-head.js" > 2. In eyedropper, "head.js" would import "shared-head.js" from framework > > This way, directories still have a "head.js" like the rest of Gecko test > dirs, and each directory's head is in control of the importing. > > What do you think? Yeah, that makes more sense

+1 jryans naming scheme

I am working on bug 859058 (Net panel HAR export) and I've created new netmonitor/har subdirectory for related code. I also wanted to create netmonitor/har/test subdir for test files and reuse the head.js file from netmonitor/test dir. Is it the right approach? Is this bug going to help me? Honza

(In reply to Jan Honza Odvarko [:Honza] from comment #7) > I am working on bug 859058 (Net panel HAR export) and I've created new > netmonitor/har subdirectory for related code. I also wanted to create > netmonitor/har/test subdir for test files and reuse the head.js file from > netmonitor/test dir. Is it the right approach? Is this bug going to help me? In that case, you can simply include something like `../../head.js` in the browser.ini for the new directory and it will automatically use the netmonitor's head.js (with or without the changes from this patch).

(In reply to Brian Grinstead [:bgrins] from comment #8) > (In reply to Jan Honza Odvarko [:Honza] from comment #7) > > I am working on bug 859058 (Net panel HAR export) and I've created new > > netmonitor/har subdirectory for related code. I also wanted to create > > netmonitor/har/test subdir for test files and reuse the head.js file from > > netmonitor/test dir. Is it the right approach? Is this bug going to help me? > > In that case, you can simply include something like `../../head.js` in the > browser.ini for the new directory and it will automatically use the > netmonitor's head.js (with or without the changes from this patch). I think I may have gotten the relative path wrong there, probably something like ../../test/head.js. Or you could create a normal head.js file and then make something like this be the first line in it: Services.scriptloader.loadSubScript("chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js", this);

Inverted to use normal head.js files that import framework/test/shared-head.js

Comment on attachment 8608160 [details] [diff] [review] test-cleanup.patch Review of attachment 8608160 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/eyedropper/test/head.js @@ +1,5 @@ > /* Any copyright is dedicated to the Public Domain. > http://creativecommons.org/publicdomain/zero/1.0/ */ > > +// shared-head.js handles imports, constants, and utility functions > +Services.scriptloader.loadSubScript("chrome://mochitests/content/browser/browser/devtools/framework/test/shared-head.js", this); Does this work if you run the eyedropper directory only after clobbering? The part I am unsure about is: eyedropper's browser.ini does not include shared-head.js as a support-file, and this path references it from the framework directory. If it works when running only that directory, then I guess all support-files for all dirs are installed before running any tests.

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #11) > Comment on attachment 8608160 [details] [diff] [review] > test-cleanup.patch > > Review of attachment 8608160 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: browser/devtools/eyedropper/test/head.js > @@ +1,5 @@ > > /* Any copyright is dedicated to the Public Domain. > > http://creativecommons.org/publicdomain/zero/1.0/ */ > > > > +// shared-head.js handles imports, constants, and utility functions > > +Services.scriptloader.loadSubScript("chrome://mochitests/content/browser/browser/devtools/framework/test/shared-head.js", this); > > Does this work if you run the eyedropper directory only after clobbering? > > The part I am unsure about is: eyedropper's browser.ini does not include > shared-head.js as a support-file, and this path references it from the > framework directory. > > If it works when running only that directory, then I guess all support-files > for all dirs are installed before running any tests. Just tested this and it does indeed work (just like the import of commandline/test/helpers.js). It seems install-tests includes all dirs. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=281b89f3893e

remote: https://hg.mozilla.org/integration/fx-team/rev/6c8613e71877