Closed
Bug 937740
Opened 11 years ago
Closed 7 years ago
Enable jsdownloads unit tests on B2G
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
INVALID
People
(Reporter: fabrice, Assigned: hiro)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
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•11 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•11 years ago
|
Blocks: jsdownloads
Assignee | ||
Comment 2•10 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 | ||
Comment 3•10 years ago
|
||
* 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•10 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•10 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•10 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•10 years ago
|
||
Clearing needinfo request.
The target file permission was changed to 666 in failure case.
Flags: needinfo?(paolo.mozmail)
Comment 8•10 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•10 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 | ||
Comment 10•10 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•10 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•7 years ago
|
||
Closing this old B2G bug.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•