Bug 1299333 test code only - Fix app update test file in use checks. r=mhowell
authorRobert Strong <robert.bugzilla@gmail.com>
Wed, 31 Aug 2016 12:35:05 -0700
changeset 312057 586deb42b345e257bc5ea51556db4010b891f1eb
parent 312056 d981eb1bef3c5815a269a1402b13083a07fcee28
child 312058 ea8a13e335c90ffa477ae7aaf0c0a3774985bb9f
push id81275
push userrstrong@mozilla.com
push dateWed, 31 Aug 2016 19:35:14 +0000
treeherdermozilla-inbound@ea8a13e335c9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmhowell
bugs1299333
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1299333 test code only - Fix app update test file in use checks. r=mhowell
toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js
--- a/toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js
+++ b/toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js
@@ -117,16 +117,18 @@ const MAX_TIMEOUT_RUNS = 20000;
 // Time in seconds the helper application should sleep before exiting. The
 // helper can also be made to exit by writing |finish| to its input file.
 const HELPER_SLEEP_TIMEOUT = 180;
 
 // Maximum number of milliseconds the process that is launched can run before
 // the test will try to kill it.
 const APP_TIMER_TIMEOUT = 120000;
 
+const FILE_IN_USE_TIMEOUT_MS = 1000;
+
 const PIPE_TO_NULL = IS_WIN ? ">nul" : "> /dev/null 2>&1";
 
 const LOG_FUNCTION = do_print;
 
 const gHTTPHandlerPath = "updates.xml";
 
 // This default value will be overridden when using the http server.
 var gURLData = URL_HOST + "/";
@@ -1899,17 +1901,26 @@ function stageUpdate() {
  * present after staging and update for updater tests and then calls
  * stageUpdateFinished.
  *
  * @param   aUpdateState
  *          The update state received by the observer notification.
  */
 function checkUpdateStagedState(aUpdateState) {
   if (IS_WIN) {
-    waitForApplicationStop(FILE_UPDATER_BIN);
+    if (IS_SERVICE_TEST) {
+      waitForServiceStop(false);
+    } else {
+      let updater = getApplyDirFile(FILE_UPDATER_BIN, true);
+      if (isFileInUse(updater)) {
+        do_timeout(FILE_IN_USE_TIMEOUT_MS,
+                   checkUpdateStagedState.bind(null, aUpdateState));
+        return;
+      }
+    }
   }
 
   Assert.equal(aUpdateState, STATE_AFTER_STAGE,
                "the notified state" + MSG_SHOULD_EQUAL);
 
   if (!gStagingRemovedUpdate) {
     Assert.equal(readStatusState(), STATE_AFTER_STAGE,
                  "the status file state" + MSG_SHOULD_EQUAL);
@@ -2330,34 +2341,16 @@ function waitForApplicationStop(aApplica
   let args = ["wait-for-application-exit", aApplication, "120"];
   let exitValue = runTestHelperSync(args);
   Assert.equal(exitValue, 0,
                "the process should have stopped, process name: " +
                aApplication);
 }
 
 /**
- * Checks if an application is running.
- *
- * @param   aApplication
- *          The application binary name to check if it is running.
- */
-function isProcessRunning(aApplication) {
-  if (!IS_WIN) {
-    do_throw("Windows only function called by a different platform!");
-  }
-
-  debugDump("checking if " + aApplication + " is running");
-  // Use the helper bin to ensure the application is stopped. If not stopped,
-  // then wait for it to stop (at most 120 seconds).
-  let args = ["is-process-running", aApplication];
-  let exitValue = runTestHelperSync(args);
-  return exitValue;
-}
-/**
  * Helper function for updater tests for launching the updater using the
  * maintenance service to apply a mar file. When complete runUpdateFinished
  * will be called.
  *
  * @param   aExpectedStatus
  *          The expected value of update.status when the test finishes.
  * @param   aSwitchApp
  *          If true the update should switch the application with an updated
@@ -2385,25 +2378,18 @@ function runUpdateUsingService(aExpected
   function readServiceLogFile() {
     let file = getMaintSvcDir();
     file.append("logs");
     file.append("maintenanceservice.log");
     return readFile(file);
   }
 
   function checkServiceUpdateFinished() {
-    if (isProcessRunning(FILE_MAINTENANCE_SERVICE_BIN)) {
-      do_execute_soon(checkServiceUpdateFinished);
-      return;
-    }
-
-    if (isProcessRunning(FILE_UPDATER_BIN)) {
-      do_execute_soon(checkServiceUpdateFinished);
-      return;
-    }
+    waitForApplicationStop(FILE_MAINTENANCE_SERVICE_BIN);
+    waitForApplicationStop(FILE_UPDATER_BIN);
 
     // Wait for the expected status
     let status;
     try {
       status = readStatusFile();
     } catch (e) {
       do_execute_soon(checkServiceUpdateFinished);
       return;
@@ -3484,53 +3470,75 @@ function checkCallbackServiceLog() {
     do_execute_soon(checkCallbackServiceLog);
     return;
   }
   Assert.ok(true, "the callback service log contents" + MSG_SHOULD_EQUAL);
 
   waitForFilesInUse();
 }
 
-// Waits until files that are in use that break tests are no longer in use and
-// then calls doTestFinish to end the test.
+/**
+ * Helper function to check if a file is in use on Windows by making a copy of
+ * a file and attempting to delete the original file. If the deletion is
+ * successful the copy of the original file is renamed to the original file's
+ * name and if the deletion is not successful the copy of the original file is
+ * deleted.
+ *
+ * @param   aFile
+ *          An nsIFile for the file to be checked if it is in use.
+ * @return  true if the file can't be deleted and false otherwise.
+ */
+function isFileInUse(aFile) {
+  if (!IS_WIN) {
+    do_throw("Windows only function called by a different platform!");
+  }
+
+  if (!aFile.exists()) {
+    debugDump("file does not exist, path: " + aFile.path);
+    return false;
+  }
+
+  let fileBak = aFile.parent;
+  fileBak.append(aFile.leafName + ".bak");
+  try {
+    if (fileBak.exists()) {
+      fileBak.remove(false);
+    }
+    aFile.copyTo(aFile.parent, fileBak.leafName);
+    aFile.remove(false);
+    fileBak.moveTo(aFile.parent, aFile.leafName);
+    debugDump("file is not in use, path: " + aFile.path);
+    return false;
+  } catch (e) {
+    debugDump("file in use, path: " + aFile.path + ", exception: " + e);
+    try {
+      if (fileBak.exists()) {
+        fileBak.remove(false);
+      }
+    } catch (e) {
+      logTestInfo("unable to remove backup file, path: " +
+                  fileBak.path + ", exception: " + e);
+    }
+  }
+  return true;
+}
+
+/**
+ * Waits until files that are in use that break tests are no longer in use and
+ * then calls doTestFinish to end the test.
+ */
 function waitForFilesInUse() {
   if (IS_WIN) {
-    let appBin = getApplyDirFile(FILE_APP_BIN, true);
-    let maintSvcInstaller = getApplyDirFile(FILE_MAINTENANCE_SERVICE_INSTALLER_BIN, true);
-    let helper = getApplyDirFile("uninstall/helper.exe", true);
-    let updater = getApplyDirFile(FILE_UPDATER_BIN, true);
-    let files = [appBin, updater, maintSvcInstaller, helper];
-
-    for (let i = 0; i < files.length; ++i) {
-      let file = files[i];
-      let fileBak = file.parent.clone();
-      if (file.exists()) {
-        fileBak.append(file.leafName + ".bak");
-        try {
-          if (fileBak.exists()) {
-            fileBak.remove(false);
-          }
-          file.copyTo(fileBak.parent, fileBak.leafName);
-          file.remove(false);
-          fileBak.moveTo(file.parent, file.leafName);
-          debugDump("file is not in use, path: " + file.path);
-        } catch (e) {
-          debugDump("will try again to remove file in use, path: " +
-                    file.path + ", exception: " + e);
-          try {
-            if (fileBak.exists()) {
-              fileBak.remove(false);
-            }
-          } catch (e) {
-            logTestInfo("unable to remove backup file, path: " +
-                        fileBak.path + ", exception: " + e);
-          }
-          do_execute_soon(waitForFilesInUse);
-          return;
-        }
+    let fileNames = [FILE_APP_BIN, FILE_UPDATER_BIN,
+                     FILE_MAINTENANCE_SERVICE_INSTALLER_BIN];
+    for (let i = 0; i < fileNames.length; ++i) {
+      let file = getApplyDirFile(fileNames[i], true);
+      if (isFileInUse(file)) {
+        do_timeout(FILE_IN_USE_TIMEOUT_MS, waitForFilesInUse);
+        return;
       }
     }
   }
 
   debugDump("calling doTestFinish");
   doTestFinish();
 }
 
@@ -3964,17 +3972,17 @@ function runUpdateUsingApp(aExpectedStat
       }
       Assert.equal(gProcess.exitValue, 0,
                    "the application process exit value should be 0");
       Assert.equal(aTopic, "process-finished",
                    "the application process observer topic should be " +
                    "process-finished");
 
       if (IS_SERVICE_TEST) {
-        waitForServiceStop();
+        waitForServiceStop(false);
       }
 
       do_execute_soon(afterAppExits);
     },
     QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver])
   };
 
   function afterAppExits() {
@@ -4058,23 +4066,25 @@ const gAppTimerCallback = {
 
 /**
  * The update-staged observer for the call to nsIUpdateProcessor:processUpdate.
  */
 const gUpdateStagedObserver = {
   observe: function(aSubject, aTopic, aData) {
     debugDump("observe called with topic: " + aTopic + ", data: " + aData);
     if (aTopic == "update-staged") {
+      Services.obs.removeObserver(gUpdateStagedObserver, "update-staged");
       // The environment is reset after the update-staged observer topic because
       // processUpdate in nsIUpdateProcessor uses a new thread and clearing the
       // environment immediately after calling processUpdate can clear the
       // environment before the updater is launched.
       resetEnvironment();
-      Services.obs.removeObserver(gUpdateStagedObserver, "update-staged");
-      checkUpdateStagedState(aData);
+      // Use do_execute_soon to prevent any failures from propagating to the
+      // update service.
+      do_execute_soon(checkUpdateStagedState.bind(null, aData));
     }
   },
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver])
 };
 
 /**
  * Sets the environment that will be used by the application process when it is
  * launched.