This will hopefully fix the current set of xpcshell failures

This should fix the remaining two app update xpcshell failures.

Since I don't have all of the xpchsell harness changes I split out a separate patch for the xpchsell harness changes.

lol s/xpchsell/xpcshell/g

All tests pass except for one on Mac

Note: I am going to have to create new mar files and then change how the precomplete file is read in updater.cpp from bug 1059467. Fun times!

This gets the tests most of the way there so I'm requesting review. I still need to investigate why the service tests fail and I will need to modify the tests for new mars after I create them which I will do in separate patches. I've also noticed that marStageSuccessComplete.js times out when it runs in parallel but succeeds on the subsequent sequential run which appears to be due to the link file not being created before it is checked.

This fixes the timeout due to the link file test

Note: upgrading the old service worked for me locally so sending to try.

Upgrading the service worked on try https://tbpl.mozilla.org/?tree=Try&rev=761d2d5050cd I think I know why marAppApplyUpdateStageSuccess.js failed on Linux There are almost permanent errors on Windows for the following tests where the post update process was launched after staging an update that I need to look into. marStageSuccessCompleteSvc.js marStageSuccessPartialSvc.js All in all things looks pretty good!

Comment on attachment 8488534 [details] [diff] [review] updated tests - patch Review of attachment 8488534 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks! ::: toolkit/mozapps/update/tests/unit_service_updater/marAppApplyDirLockedStageFailureSvc_win.js @@ +25,5 @@ > > createUpdaterINI(false); > > + if (IS_WIN) { > + Services.prefs.setBoolPref(PREF_APP_UPDATE_SERVICE_ENABLED, true); Is this the default already? If you're not sure I'm fine with just leaving it.

(In reply to Brian R. Bondy [:bbondy] from comment #15) > Comment on attachment 8488534 [details] [diff] [review] > updated tests - patch > > Review of attachment 8488534 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks good, thanks! > > ::: > toolkit/mozapps/update/tests/unit_service_updater/ > marAppApplyDirLockedStageFailureSvc_win.js > @@ +25,5 @@ > > > > createUpdaterINI(false); > > > > + if (IS_WIN) { > > + Services.prefs.setBoolPref(PREF_APP_UPDATE_SERVICE_ENABLED, true); > > Is this the default already? If you're not sure I'm fine with just leaving > it. Thanks for pointing this out. I wanted to protect the service tests in case an app has that set to disabled.

forgot to update the "patch - test changes part 2" patch

Pushed to try https://tbpl.mozilla.org/?tree=Try&rev=b4542505e600 I expect that marStageSuccessCompleteSvc.js and marStageSuccessPartialSvc.js will fail and I'm looking into why.

Comment on attachment 8489188 [details] [diff] [review] patch new binary test support files unify is failing on the new mac binaries so clearing review request

Comment on attachment 8489189 [details] [diff] [review] patch new test support files This patch will need to be changed too

Comment on attachment 8489193 [details] [diff] [review] patch - test changes part 2 Also needs changes :(

Pushed to try https://tbpl.mozilla.org/?tree=Try&rev=f8d3ba1d5cff I expect that marStageSuccessCompleteSvc.js and marStageSuccessPartialSvc.js will fail and I'm looking into why.

missed some changes

Pushed to try... again https://tbpl.mozilla.org/?tree=Try&rev=52b2042aa7aa

marStageSuccessCompleteSvc.js and marStageSuccessPartialSvc.js is failing intermittently and I'm looking into it

Comment on attachment 8489230 [details] [diff] [review] patch - test changes part 2 Removed the following unused code locally. + // Replace %DIR_RESOURCES% with the platform specific value so the same log + // can be used with Mac OS X. + compareLogContents = compareLogContents.replace(/%DIR_RESOURCES%/g, DIR_RESOURCES);

Also removed the following debugging code locally + logTestInfo("compareLogContents\n" + compareLogContents); + logTestInfo("updateLogContents\n" + updateLogContents);

Added a couple of more checks for launching the post update process.

Comment on attachment 8489228 [details] [diff] [review] patch - test support files Review of attachment 8489228 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for separating out this stuff :)

Carrying forward r+ I moved the client changes over to the patch in bug 1064523

Comment on attachment 8490311 [details] [diff] [review] Combined patch Review of attachment 8490311 [details] [diff] [review]: ----------------------------------------------------------------- Just some drive-by feedback. Stumbled over this during the review for bug 1072554. Would be great if we could include this in the final patch that's going to land on m-c. ::: toolkit/mozapps/update/tests/unit_aus_update/head_update.js @@ +9,2 @@ > // MOZ_APP_VENDOR is optional. > // On Windows, if MOZ_APP_VENDOR is not defined the updates directory will be With the removal of the ifdef we should probably update the comment to no longer reference Windows only. @@ +911,5 @@ > // adjustGeneralPaths registers a cleanup function that calls end_test when > // it is defined as a function. > adjustGeneralPaths(); > > // Remove the updates directory on Windows which is located outside of the Ideally, this comment would mention "Windows and Mac", similar to the new comment in |cleanupTestCommon|.

Thanks and I'll update those comments locally

Comment on attachment 8490311 [details] [diff] [review] Combined patch Review of attachment 8490311 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/mozapps/update/tests/unit_aus_update/head_update.js @@ +913,5 @@ > adjustGeneralPaths(); > > // Remove the updates directory on Windows which is located outside of the > // application directory after the call to adjustGeneralPaths has set it up. > // Since the test hasn't ran yet and the directory shouldn't exist finished sorry, one more nit since we're already updating this comment: s/ran/run/ and if I'm understanding the comment correctly, I believe the word "finished" should be removed in this sentence.

Pushed to fx-team https://hg.mozilla.org/integration/fx-team/rev/b1d8f769952a

Landed on aurora in the Mac V2 signing combined patch in bug 1047584