Closed Bug 937740 Opened 11 years ago Closed 7 years ago

Enable jsdownloads unit tests on B2G

Categories

(Firefox OS Graveyard :: General, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: fabrice, Assigned: hiro)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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?
(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.
Blocks: jsdownloads
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
Attached patch WIP v1 (deleted) — Splinter Review
* 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.
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.
(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...
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)
Clearing needinfo request. The target file permission was changed to 666 in failure case.
Flags: needinfo?(paolo.mozmail)
(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.
(In reply to Hiroyuki Ikezoe (:hiro) from comment #7) > The target file permission was changed to 666 in failure case. Can you elaborate?
Depends on: 1154955
(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.
The failures of test_error_target and test_error_restart hid other intermittent failures. test_public_and_private and others sometimes fails on local.
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.

Attachment

General

Created:
Updated:
Size: