Closed
Bug 1284742
Opened 8 years ago
Closed 8 years ago
Intermittent dom/filesystem/tests/test_basic.html | application timed out after 330 seconds with no output
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla51
People
(Reporter: intermittent-bug-filer, Assigned: acomminos)
References
Details
(Keywords: intermittent-failure)
Attachments
(1 file)
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
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 3•8 years ago
|
||
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.
Assignee | ||
Comment 4•8 years ago
|
||
Sorry, I meant the profile directory, not the root- we only recurse on the former.
Assignee | ||
Comment 5•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/68660/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68660/
Attachment #8777063 -
Flags: review?(michael)
Comment 6•8 years ago
|
||
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?
Attachment #8777063 -
Flags: review?(michael)
Assignee | ||
Comment 7•8 years ago
|
||
(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).
Assignee | ||
Comment 8•8 years ago
|
||
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/
Attachment #8777063 -
Flags: review?(michael)
Comment 9•8 years ago
|
||
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 :)
Attachment #8777063 -
Flags: review?(michael) → review+
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → andrew
Status: NEW → ASSIGNED
Comment 10•8 years ago
|
||
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
Assignee | ||
Comment 11•8 years ago
|
||
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).
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4330a90be8b0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment hidden (Intermittent Failures Robot) |
Updated•8 years ago
|
status-firefox50:
--- → affected
Comment 14•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/5e9947ac9881
Flags: in-testsuite+
Updated•8 years ago
|
status-firefox49:
--- → affected
Comment 15•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/4e78a1131df0
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•