Enable jsdownloads unit tests on B2G

RESOLVED INVALID

Status

Firefox OS
General
RESOLVED INVALID
5 years ago
8 months ago

People

(Reporter: fabrice, Assigned: hiro)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
We don't run most of jsdownloads tests for now on b2g, but support is added in bug 936342 and bug 934677.

Paolo, can you point me to the disabled tests?

Comment 1

5 years ago
(In reply to Fabrice Desré [:fabrice] from comment #0)
> Paolo, can you point me to the disabled tests?

The tests are all contained in the "toolkit/components/jsdownloads/test/unit" folder.

You can see those in the Desktop build log:

https://tbpl.mozilla.org/php/getParsedLog.php?id=30476271&tree=Mozilla-Central&full=1

Those are not run on B2G for the same revision:

https://tbpl.mozilla.org/php/getParsedLog.php?id=30477293&tree=Mozilla-Central&full=1

I've noticed that the only xpcshell run visible in TBPL is for the "B2G ICS Emulator Opt" configuration.

Updated

5 years ago
Blocks: 825588
(Assignee)

Comment 2

3 years ago
Taking.

Currently some tests depends on places (PlacesTestUtils actually), so those tests should be skipped on b2g because places is not built in b2g.

We have no easy way to skip test on a certain platform.
I've opened bug 1150818 and bug 1150822 to skip test easily.
Assignee: nobody → hiikezoe
Depends on: 1150822, 1150818
(Assignee)

Comment 3

3 years ago
Created attachment 8592551 [details] [diff] [review]
WIP v1

* Tests for DownloadObserver in test_DownloadIntegration.js split into a new file (test_DownloadObserver.js)
* Rest of tests in test_DownloadIntegration.js skip since those tests are related to DownloadIntegration.getSystemDownloadsDirectory. (i.e. browser window is needed)
* test_DownloadLegacy.js skips since legacy saver is unnecessary on b2g I think
* tests using places utility in test_DownloadList.js skip 
* tests related to Downloads.getSystemDownloadsDirectory in test_Downloads skip

With this patch rest of tests almost work fine except a failure in test_DownloadCore.js:

 4:27.05 LOG: Thread-1 ERROR The download should have failed.
    test_DownloadCore.js -> file:///data/local/tests/xpcshell/toolkit/components/jsdownloads/test/unit/common_test_Download.js:test_error_target:1281
    self-hosted:next:675
    _run_next_test@/data/local/tests/xpcshell/head.js:1416:9
    do_execute_soon/<.run@/data/local/tests/xpcshell/head.js:653:9
    _do_main@/data/local/tests/xpcshell/head.js:207:5
    _execute_test@/data/local/tests/xpcshell/head.js:513:5
    @-e:1:1

I have no idea why the test failed now. Need to investigate.
(Assignee)

Comment 4

3 years ago
The failure of test_error_target might be affected by other tests.

Moving test_error_target and test_error_restart at the head of common_test_Download.js solves the failure.
(Assignee)

Comment 5

3 years ago
(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)
> The failure of test_error_target might be affected by other tests.
> 
> Moving test_error_target and test_error_restart at the head of
> common_test_Download.js solves the failure.

whoa, moving those tests did not solve the failure. It makes those tests intermittent failure...
(Assignee)

Comment 6

3 years ago
From test_error_target in common_test_Download.js:

        download = yield Downloads.createDownload({
          source: httpUrl("source.txt"),
          target: targetFile,
        });
        yield download.start();
      } else {
        // When testing DownloadLegacySaver, we cannot be sure whether we are
        // testing the promise returned by the "start" method or we are testing
        // the "error" property checked by promiseDownloadStopped.  This happens
        // because we don't have control over when the download is started.
        download = yield promiseStartLegacyDownload(null,
                                                    { targetFile: targetFile });
        yield promiseDownloadStopped(download);
      }
      do_throw("The download should have failed.");

Though I don't investigate implementation of DownloadCore.jsm, before throwing ex exception we should wait for start of saving the target file on local storage?
Flags: needinfo?(paolo.mozmail)
(Assignee)

Comment 7

3 years ago
Clearing needinfo request.

The target file permission was changed to 666 in failure case.
Flags: needinfo?(paolo.mozmail)

Comment 8

3 years ago
(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)
> * Tests for DownloadObserver in test_DownloadIntegration.js split into a new
> file (test_DownloadObserver.js)

Nice!

> * Rest of tests in test_DownloadIntegration.js skip since those tests are
> related to DownloadIntegration.getSystemDownloadsDirectory. (i.e. browser
> window is needed)

Okay, maybe B2G needs a bug on file to implement this test in the appropriate framework, if one isn't there already.

> * test_DownloadLegacy.js skips since legacy saver is unnecessary on b2g I
> think

It's actually used and an important piece, it's how single-click downloads (for example pages with Content-Disposition headers) are handled. Did you observe specific failures with them?

> * tests using places utility in test_DownloadList.js skip 
> * tests related to Downloads.getSystemDownloadsDirectory in test_Downloads
> skip

Okay.

Comment 9

3 years ago
(In reply to Hiroyuki Ikezoe (:hiro) from comment #7)
> The target file permission was changed to 666 in failure case.

Can you elaborate?
(Assignee)

Updated

3 years ago
Depends on: 1154955
(Assignee)

Comment 10

3 years ago
(In reply to :Paolo Amadini from comment #8)

> > * Rest of tests in test_DownloadIntegration.js skip since those tests are
> > related to DownloadIntegration.getSystemDownloadsDirectory. (i.e. browser
> > window is needed)
> 
> Okay, maybe B2G needs a bug on file to implement this test in the
> appropriate framework, if one isn't there already.

I will open a bug for it.

> > * test_DownloadLegacy.js skips since legacy saver is unnecessary on b2g I
> > think
> 
> It's actually used and an important piece, it's how single-click downloads
> (for example pages with Content-Disposition headers) are handled. Did you
> observe specific failures with them?

Thanks! I did not know the fact. Now I tried, and there is the same failure in test_DownloadCore.js caused by OS.file.open bug (bug 1154955). So the failure will be solved by that bug.
(Assignee)

Comment 11

3 years ago
The failures of test_error_target and test_error_restart hid other intermittent failures.
test_public_and_private and others sometimes fails on local.

Comment 12

8 months ago
Closing this old B2G bug.
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.