Filed by: philringnalda@gmail.com https://treeherder.mozilla.org/logviewer.html#?job_id=271391&repo=autoland http://archive.mozilla.org/pub/firefox/tinderbox-builds/autoland-macosx64/1467764121/autoland_yosemite_r7_test-mochitest-2-bm108-tests1-macosx-build13.txt.gz

The timeout here is caused by failing to set a promise rejection handler in the call to dir.getFilesAndDirectories() within checkSubDir() in dom/filesystem/tests/filesystem_commons.js. When we set a rejection handler, we end up receiving NS_ERROR_DOM_FILE_NOT_FOUND_ERR. I suspect that during the breadth-first expansion from the root, we're queuing up temporary directories that get deleted before the promises calling getFilesAndDirectories() on them resolve. Although exploring the root is a good set of data to test traversal on, perhaps we should consider a more deterministic data set.

Sorry, I meant the profile directory, not the root- we only recurse on the former.

Review commit: https://reviewboard.mozilla.org/r/68660/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68660/

Comment on attachment 8777063 [details] Bug 1284742 - Replace profile directory traversal with a generated directory tree in dom/filesystem/test/test_basic.html. https://reviewboard.mozilla.org/r/68660/#review65726 ::: dom/filesystem/tests/script_fileList.js:11 (Diff revision 1) > +// Creates a parametric arity directory hierarchy as a function of depth. > +// Returns the parent directory of the subtree. Maybe include in the comment what the directory would look like with, say, depth=3. It would make it easier to reason about what is happening below. e.g. ``` dir-tree-test/ - subdir3 - file.txt - subdir1 - file.txt - subdir2 - file.txt - subdir1 - file.txt ``` ::: dom/filesystem/tests/script_fileList.js:15 (Diff revision 1) > + parent = Cc["@mozilla.org/file/directory_service;1"] > + .getService(Ci.nsIDirectoryService) > + .QueryInterface(Ci.nsIProperties) > + .get('TmpD', Ci.nsIFile); Is Services.jsm included here? It would be nice to use Services.dirsvc instead of getService().QueryInterface() and the Cc string. ``` parent = Services.dirsvc.get('TmpD', Ci.nsIFile); ``` ::: dom/filesystem/tests/script_fileList.js:19 (Diff revision 1) > + if (!parent) { > + parent = Cc["@mozilla.org/file/directory_service;1"] > + .getService(Ci.nsIDirectoryService) > + .QueryInterface(Ci.nsIProperties) > + .get('TmpD', Ci.nsIFile); > + parent.append('dir-tree-test'); Shouldn't we make this generate a different name each time? What if this directory already exists?

(In reply to Michael Layzell [:mystor] from comment #6) > Comment on attachment 8777063 [details] > Bug 1284742 - Replace profile directory traversal with a generated directory > tree in dom/filesystem/test/test_basic.html. > > https://reviewboard.mozilla.org/r/68660/#review65726 > > ::: dom/filesystem/tests/script_fileList.js:11 > (Diff revision 1) > > +// Creates a parametric arity directory hierarchy as a function of depth. > > +// Returns the parent directory of the subtree. > > Maybe include in the comment what the directory would look like with, say, > depth=3. It would make it easier to reason about what is happening below. > > e.g. > ``` > dir-tree-test/ > - subdir3 > - file.txt > - subdir1 > - file.txt > - subdir2 > - file.txt > - subdir1 > - file.txt > ``` Sure thing. > ::: dom/filesystem/tests/script_fileList.js:15 > (Diff revision 1) > > + parent = Cc["@mozilla.org/file/directory_service;1"] > > + .getService(Ci.nsIDirectoryService) > > + .QueryInterface(Ci.nsIProperties) > > + .get('TmpD', Ci.nsIFile); > > Is Services.jsm included here? It would be nice to use Services.dirsvc > instead of getService().QueryInterface() and the Cc string. > > ``` > parent = Services.dirsvc.get('TmpD', Ci.nsIFile); > ``` No, it is not included in any of the dom filesystem tests (other tests in this file use the same approach). > ::: dom/filesystem/tests/script_fileList.js:19 > (Diff revision 1) > > + if (!parent) { > > + parent = Cc["@mozilla.org/file/directory_service;1"] > > + .getService(Ci.nsIDirectoryService) > > + .QueryInterface(Ci.nsIProperties) > > + .get('TmpD', Ci.nsIFile); > > + parent.append('dir-tree-test'); > > Shouldn't we make this generate a different name each time? What if this > directory already exists? nsIFile::CreateUnique handles this, enumerating non-unique file names (so it'll create dir-tree-test[1..n] if dir-tree-test exists).

Comment on attachment 8777063 [details] Bug 1284742 - Replace profile directory traversal with a generated directory tree in dom/filesystem/test/test_basic.html. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68660/diff/1-2/

Comment on attachment 8777063 [details] Bug 1284742 - Replace profile directory traversal with a generated directory tree in dom/filesystem/test/test_basic.html. https://reviewboard.mozilla.org/r/68660/#review65732 lgtm :)

Pushed by acomminos@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4330a90be8b0 Replace profile directory traversal with a generated directory tree in dom/filesystem/test/test_basic.html. r=mystor

FWIW, the directory that triggered this issue on the desktop-large AWS instances was $PROFILE/safebrowsing-backup (a temporary backup copy of $PROFILE/safebrowsing that gets scheduled for deletion on a worker thread).