Closed Bug 1284742 Opened 3 years ago Closed 3 years ago

Intermittent dom/filesystem/tests/test_basic.html | application timed out after 330 seconds with no output

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox49 --- fixed
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: acomminos)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

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.
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)
(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/
Attachment #8777063 - Flags: review?(michael)
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: nobody → andrew
Status: NEW → ASSIGNED
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
Blocks: 1281241
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).
https://hg.mozilla.org/mozilla-central/rev/4330a90be8b0
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.