Closed
Bug 981251
Opened 10 years ago
Closed 10 years ago
Test app uninstallation
Categories
(Firefox Graveyard :: Webapp Runtime, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 33
People
(Reporter: marco, Assigned: marco)
References
Details
Attachments
(1 file, 1 obsolete file)
21.52 KB,
patch
|
marco
:
review+
|
Details | Diff | Splinter Review |
It should be easy on Linux, I'm not sure how to do it on Windows or Mac.
Updated•10 years ago
|
Priority: -- → P1
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → mar.castelluccio
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•10 years ago
|
||
Depends on the patches in the other bugs. The test is a bit more complex for Mac, because Mac has a weird behavior (PathForAppWithIdentifier returns the app path until you try to launch it. The launch fails and then PathForAppWithIdentifier returns false). Sent to try with the other patches, and it's green: https://tbpl.mozilla.org/?tree=Try&rev=60cf9c4b6525
Attachment #8405862 -
Flags: review?(myk)
Comment 2•10 years ago
|
||
Comment on attachment 8405862 [details] [diff] [review] Patch Review of attachment 8405862 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/webapps/tests/head.js @@ +43,5 @@ > return true; > }); > } > > +function dirContainsOnly(dir, files) { Nit: this would be simpler if you first slurped the list of files in the directory: let filesInDir = [file for each (file in iterator)]; Then you would be able to use use Array.filter to generate arrays of unexpectedly found and missing items: let unexpectedlyFoundInDir = filesInDir.filter(v => files.indexOf(v) == -1); let unexpectedlyMissingFromDir = files.filter(v => filesInDir.indexOf(v) == -1); @@ +69,5 @@ > + } > + > + // If one or more files aren't present in the directory, return false. > + if (files.length > 0) { > + return false; Nit: since you report files that were unexpectedly found in the directory via the info() call in the iterator loop above, it'd make sense to also report here files that are unexpectedly missing from the directory. ::: toolkit/webapps/tests/test_hosted_uninstall.xul @@ +221,3 @@ > } > + // On Mac, the app is moved to the trash, it is still considered launchable > + // (because it does have a install path). Nit: that's not quite right, as you can't actually launch an app in the trash (which you test below). So we should fix WebappOSUtils.isLaunchable to report trashed apps as unlaunchable, even before a failure to launch it. But we can do that in a followup bug. @@ +224,5 @@ > + if (!isMac) { > + while (WebappOSUtils.isLaunchable(app)) { > + yield wait(1000); > + } > + ok(true, "App not launchable"); Isn't Mac the only platform on which isLaunchable is asynchronous (and therefore potentially out-of-date)? I thought Windows and Linux both immediately reflect changes to an app's installation status. @@ +229,5 @@ > > + is(WebappOSUtils.getInstallPath(app), null, "getInstallPath == null"); > + } else { > + trashDir = WebappOSUtils.getInstallPath(app); > + ok(trashDir.contains(".Trash"), "App moved to Trash"); Nit: in a followup, we should add the user's trash dir to OS.Constants.Path, so we can test it more precisely here.
Attachment #8405862 -
Flags: review?(myk) → review+
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8405862 -
Attachment is obsolete: true
Attachment #8410972 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7289f22cafd
Flags: in-testsuite+
Keywords: checkin-needed
Comment 5•10 years ago
|
||
Backed out for making bug 763894 perma-fail. https://hg.mozilla.org/integration/mozilla-inbound/rev/621bb79845e3 https://tbpl.mozilla.org/php/getParsedLog.php?id=38328630&tree=Mozilla-Inbound Some observations: 1) These new tests run right before the now-perma-failing test. Are these tests somehow still doing work after they finish? 2) Can't help but notice that every other test in this dir is skipped on ASAN builds. Should these too?
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #5) > 1) These new tests run right before the now-perma-failing test. Are these > tests somehow still doing work after they finish? Yes, this is most probably the issue, the cleanup function is asynchronous. > 2) Can't help but notice that every other test in this dir is skipped on > ASAN builds. Should these too? Only the tests that actually launch an app need to be skipped on ASAN (test_hosted_launch and test_packaged_launch).
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Marco Castelluccio [:marco] from comment #6) > (In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #5) > > 1) These new tests run right before the now-perma-failing test. Are these > > tests somehow still doing work after they finish? > > Yes, this is most probably the issue, the cleanup function is asynchronous. I've tried making the cleanup function synchronous, but the other test is still failing (only on Windows XP).
Assignee | ||
Comment 8•10 years ago
|
||
Myk, I'd like to land the tests disabled only on Windows XP (since it's the only problematic platform). Do you think it's OK?
Flags: needinfo?(myk)
Comment 9•10 years ago
|
||
(In reply to Marco Castelluccio [:marco] from comment #8) > Myk, I'd like to land the tests disabled only on Windows XP (since it's the > only problematic platform). Do you think it's OK? Yes, provided we open a bug about the issue with the tests on Windows XP.
Flags: needinfo?(myk)
Assignee | ||
Comment 10•10 years ago
|
||
Landed disabled on Windows XP: https://hg.mozilla.org/integration/mozilla-inbound/rev/3b49cfc3c734 Opened bug 1029925.
Comment 11•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3b49cfc3c734
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Updated•8 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•