Last Comment Bug 692625 - Flatten directories in the tree that only contain a single subdirectory and no files
: Flatten directories in the tree that only contain a single subdirectory and n...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Ed Morley [:emorley]
:
Mentors:
Depends on:
Blocks: 707650
  Show dependency treegraph
 
Reported: 2011-10-06 15:33 PDT by Ed Morley [:emorley]
Modified: 2011-12-16 05:49 PST (History)
8 users (show)
emorley: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Script to find such directories (715 bytes, text/plain)
2011-11-20 11:37 PST, Ed Morley [:emorley]
no flags Details
Raw results (8.38 KB, text/plain)
2011-11-20 11:52 PST, Ed Morley [:emorley]
no flags Details
A: browser/components/sessionstore/test (46.31 KB, patch)
2011-12-03 17:11 PST, Ed Morley [:emorley]
gavin.sharp: review+
Details | Diff | Review
B: browser/components/sidebar (1.98 KB, patch)
2011-12-03 17:12 PST, Ed Morley [:emorley]
gavin.sharp: review+
Details | Diff | Review
C: browser/components/test (1.97 KB, patch)
2011-12-03 17:13 PST, Ed Morley [:emorley]
gavin.sharp: review+
Details | Diff | Review
D: browser/devtools/styleinspector/test (7.17 KB, patch)
2011-12-03 17:15 PST, Ed Morley [:emorley]
rcampbell: review+
Details | Diff | Review
E: browser/devtools/webconsole/test (119.78 KB, patch)
2011-12-03 17:16 PST, Ed Morley [:emorley]
rcampbell: review+
Details | Diff | Review
F: toolkit/components/aboutmemory/tests (2.38 KB, patch)
2011-12-03 17:17 PST, Ed Morley [:emorley]
gavin.sharp: review+
Details | Diff | Review
G: toolkit/components/startup/tests (4.39 KB, patch)
2011-12-03 17:19 PST, Ed Morley [:emorley]
dtownsend: review+
Details | Diff | Review
H: Update *makefiles.sh to reflect new paths (4.46 KB, patch)
2011-12-03 17:23 PST, Ed Morley [:emorley]
emorley: review+
Details | Diff | Review

Description Ed Morley [:emorley] 2011-10-06 15:33:04 PDT
Broken out from bug 689884 comment 12.

There exists in the tree a number of directories whose parent, only contains that directory and nothing else - and so could be flattened into the parent.
Comment 1 Ed Morley [:emorley] 2011-11-20 11:37:37 PST
Created attachment 575760 [details]
Script to find such directories
Comment 2 Ed Morley [:emorley] 2011-11-20 11:52:36 PST
Created attachment 575761 [details]
Raw results

Output of the script. Obviously contains a number of directories that will not presumably not want to be flattened (eg db/sqlite/ even though it's the only folder in db/), and will need to go through manually.
Comment 3 Ed Morley [:emorley] 2011-11-20 13:12:59 PST
Dao, what are your thoughts about the results attachment? This bug was created on your suggestion (like you, I agree with it's intent), however I can see this turning into a nightmare of chasing module owners up and then people getting cross for weeks after, since none of their patch queues will then apply. 

Do you have any suggestions as to how best to go about this / roughly which of the types (eg tests folders that only contain one type of tests and so on) in the results file you see as worth being moved?
Comment 4 Ted Mielczarek [:ted.mielczarek] 2011-11-21 05:07:21 PST
An alternative solution is to change the DIRS line for parent dirs. If you have foo/bar, which just has "DIRS = xyz", you could change foo/Makefile.in to read "DIRS = bar/xyz", remove foo/bar/Makefile.in, and not have to move any files around.
Comment 5 Ed Morley [:emorley] 2011-11-21 05:15:45 PST
(In reply to Ted Mielczarek [:ted, :luser] from comment #4)
> An alternative solution is to change the DIRS line for parent dirs

That's what I did in bug 689884 (all instances should now be changed, unless anyone's added any since), however in that bug Dao suggested that I should have collapsed the directories instead (bug 689884 comment 12), which was why I filed this. 

However now that I've scanned the tree I'm somewhat indifferent, given the sheer number of directories (only some were as a result of bug 689884), and given that for many of them their existence is justified or else perhaps going to cause more upset than it's worth to move.
Comment 6 Ed Morley [:emorley] 2011-11-21 05:23:33 PST
errr... and by Dao I of course meant Gavin throughout, whoops.

Sorry for the spam Dao.
Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-11-23 17:10:09 PST
I think these should be fixed:
browser/components/sessionstore/test
browser/components/sidebar
browser/components/test
browser/devtools/styleinspector/test
browser/devtools/webconsole/test
toolkit/components/aboutmemory/tests
toolkit/components/startup/tests

I don't really care about the others.
Comment 8 Ed Morley [:emorley] 2011-12-03 17:11:51 PST
Created attachment 578867 [details] [diff] [review]
A: browser/components/sessionstore/test
Comment 9 Ed Morley [:emorley] 2011-12-03 17:12:42 PST
Created attachment 578868 [details] [diff] [review]
B: browser/components/sidebar
Comment 10 Ed Morley [:emorley] 2011-12-03 17:13:19 PST
Created attachment 578869 [details] [diff] [review]
C: browser/components/test
Comment 11 Ed Morley [:emorley] 2011-12-03 17:15:30 PST
Created attachment 578871 [details] [diff] [review]
D: browser/devtools/styleinspector/test

Rob, moves browser/devtools/styleinspector/test/browser/ to browser/devtools/styleinspector/test/ since the parent directory is otherwise empty.
Comment 12 Ed Morley [:emorley] 2011-12-03 17:16:23 PST
Created attachment 578872 [details] [diff] [review]
E: browser/devtools/webconsole/test

Moves browser/devtools/webconsole/test/browser/ to browser/devtools/webconsole/test/
Comment 13 Ed Morley [:emorley] 2011-12-03 17:17:17 PST
Created attachment 578873 [details] [diff] [review]
F: toolkit/components/aboutmemory/tests

Moves toolkit/components/aboutmemory/tests/chrome/ to toolkit/components/aboutmemory/tests/
Comment 14 Ed Morley [:emorley] 2011-12-03 17:19:43 PST
Created attachment 578874 [details] [diff] [review]
G: toolkit/components/startup/tests

Moves toolkit/components/startup/tests/browser/ to toolkit/components/startup/tests/
Comment 15 Ed Morley [:emorley] 2011-12-03 17:23:44 PST
Created attachment 578875 [details] [diff] [review]
H: Update *makefiles.sh to reflect new paths

(standing rs=build, so not requesting review)
Comment 16 Ed Morley [:emorley] 2011-12-03 17:30:28 PST
Meant to add: Everything passes try at the moment, and when reviews are complete, I'll be landing over a weekend whilst it's quiet, so I can tryserver again + land straight after, before other things can break it. (Given how unsuccessfully mercurial backs out renames, I want to make 110% sure this sticks).
Comment 17 Nicholas Nethercote [:njn] 2011-12-03 17:41:27 PST
Comment on attachment 578873 [details] [diff] [review]
F: toolkit/components/aboutmemory/tests

Review of attachment 578873 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like gavin already r+'d, but this seems fine to me -- I just copied the structure from somewhere else so I'm not greatly attached to it.
Comment 18 Rob Campbell [:rc] (:robcee) 2011-12-05 05:42:59 PST
Comment on attachment 578871 [details] [diff] [review]
D: browser/devtools/styleinspector/test

looks ok.
Comment 19 Rob Campbell [:rc] (:robcee) 2011-12-05 05:47:01 PST
Comment on attachment 578872 [details] [diff] [review]
E: browser/devtools/webconsole/test

in:

--- a/browser/devtools/webconsole/test/browser/browser_warn_user_about_replaced_api.js
+++ b/browser/devtools/webconsole/test/browser_warn_user_about_replaced_api.js
@@ -32,17 +32,17 @@
  * use your version of this file under the terms of the MPL, indicate your
  * decision by deleting the provisions above and replace them with the notice
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
-const TEST_REPLACED_API_URI = "http://example.com/browser/browser/devtools/webconsole/test//browser/test-console-replaced-api.html";
+const TEST_REPLACED_API_URI = "http://example.com/browser/browser/devtools/webconsole/test//test-console-replaced-api.html";

Extra '/' character in that URL. Should probably fix that.

another one of these in:

+++ b/browser/devtools/webconsole/test/browser_webconsole_basic_net_logging.js

I'm wondering if our previous move script was broken because all of these URI consts seem to have them.

Since they were like that when you got them, I'll R+ this with a follow-up to clean up the URIs.

Thanks for doing this!
Comment 20 Rob Campbell [:rc] (:robcee) 2011-12-05 05:50:35 PST
filed bug 707650 to clean up the extra characters in the webconsole tests.
Comment 21 Dave Townsend [:mossop] 2011-12-05 13:29:01 PST
Comment on attachment 578874 [details] [diff] [review]
G: toolkit/components/startup/tests

Review of attachment 578874 [details] [diff] [review]:
-----------------------------------------------------------------

What is the benefit of this move?
Comment 22 Ed Morley [:emorley] 2011-12-07 14:40:03 PST
(In reply to Dave Townsend (:Mossop) from comment #21)
> What is the benefit of this move?

Reducing unnecessary hierarchy depth in the trees + Dao asked that I do it in bug 689884 comment 12. Happy for you and Dao to come to an agreement either way and obsolete this part's patch if decided easiest.
Comment 23 Ed Morley [:emorley] 2011-12-07 14:42:03 PST
I've done it again!
s/Dao/Gavin/

(also remembering to add to CC this time)
Comment 24 Ed Morley [:emorley] 2011-12-15 16:49:10 PST
Slight rebase + including the move of a few more tests added in those directories since:
https://tbpl.mozilla.org/?tree=Try&rev=e26d522933e4

Note You need to log in before you can comment on or make changes to this bug.