446256 - Automate litmus test Testcase ID #4655 - Resume deleted file

Status: NEW

(Toolkit :: Downloads API, defect)

Description

[Anoop Saldanha (Poona)]

Proposed type of test: chrome

Proposed location of test: toolkit/mozapps/downloads/tests/chrome

Comment 1

[Anoop Saldanha (Poona)]

Attachment: v1.0

Have run the test.  It passes.

I have used if inside the listener instead of switch so that I can include this.wasPaused in the main if itself.

The test passes but I have some questions on this.  I am not sure if the target file exists, once I do addDownload().  So will the deletion of the target file really work once I pause the download?  Because I tried to put a is(1,1,"Inside dmFile.exists()") inside if(dmFile.exists()) and I couldn't see the flow entering the if in the results.  The test passes nevertheless.

Comment 2

[Anoop Saldanha (Poona)]

Attachment: v2.0

Test updated to accommodate the new utils.js.  The test passes

Comment 3

[Shawn Wilsher :sdwilsh]

Comment on attachment 331489 [details] [diff] [review]
v2.0

>diff --git a/toolkit/mozapps/downloads/tests/chrome/Makefile.in b/toolkit/mozapps/downloads/tests/chrome/Makefile.in
>--- a/toolkit/mozapps/downloads/tests/chrome/Makefile.in
>+++ b/toolkit/mozapps/downloads/tests/chrome/Makefile.in
>@@ -62,6 +62,7 @@ _CHROME_FILES = \
>   test_queued_download_context.xul \
>   test_remove_download_pre_select_next_file.xul \
>   test_removeDownload_updates_ui.xul \
>+  test_resume_deleted_download.xul \
nit: test_resume_deleted_local_file.xul please

>+function runTest()
>+{
>+  const dmui = Cc["@mozilla.org/download-manager-ui;1"].
>+             getService(Ci.nsIDownloadManagerUI);
>+  const dm = Cc["@mozilla.org/download-manager;1"].
>+           getService(Ci.nsIDownloadManager);
lose the const and just use let.  You messed up your spacing in the process of using const

>+  var listener = {
s/var/let/ please

>+    onDownloadStateChange: function(aOldState, aDownload) {
>+      if(aDownload.state == Ci.nsIDownloadManager.DOWNLOAD_DOWNLOADING && !this.wasPaused) {
>+        dm.pauseDownload(aDownload.id);
>+      }
nit: no brace for one line if statement please

>+      if(aDownload.state == Ci.nsIDownloadManager.DOWNLOAD_PAUSED) {
nit: one line of space between the if blocks please

>+        this.wasPaused = true;
>+
>+        // Delete the target file of the paused download
>+        let targetFile = aDownload.targetFile;
>+        if (targetFile.exists())
You should probably do an ok(targetFile.exists()) since it should at this point.

>+      if(aDownload.state == Ci.nsIDownloadManager.DOWNLOAD_FINISHED) {
nit: one line of space between if blocks please

>+        is(1,1,"The paused download resumed and completed, inspite of deleting the " +
>+          "paused downloads file from the disk");
ok(this.wasPuased, "...") please

>+        dm.removeListener(listener);
please run this in executeSoon - we shouldn't remove listeners inside a listener call (bad things can happen)


However, I don't see why this isn't an xpcshell test.  It would be pretty easy to port the existing code to one.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.