Closed Bug 1072554 Opened 7 years ago Closed 7 years ago

Remove update directories when running xpcshell tests

Categories

(Toolkit :: Application Update, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
firefox34 --- fixed
firefox35 --- fixed

People

(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)

References

Details

Attachments

(1 file, 3 obsolete files)

General cleanup bug.

With mac v2 signing the update directory path is as follows:
~/Library/Caches/Mozilla/updates/<path to app bundle>

The test's update sub-directories under ~/Library/Caches/Mozilla/updates/ should be removed.

I will also sync the changes to getting the update directory in nsXREDirProvider.cpp to the test head_update.js file in this bug.
Attached patch patch rev1 (obsolete) — Splinter Review
Attachment #8494786 - Flags: review?(spohl.mozilla.bugs)
Attached patch patch rev2 (obsolete) — Splinter Review
Fixed an over 80
Attachment #8494786 - Attachment is obsolete: true
Attachment #8494786 - Flags: review?(spohl.mozilla.bugs)
Attachment #8494788 - Flags: review?(spohl.mozilla.bugs)
Comment on attachment 8494788 [details] [diff] [review]
patch rev2

Review of attachment 8494788 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great! Just one line with extra white space.

::: toolkit/mozapps/update/tests/unit_aus_update/head_update.js
@@ +959,5 @@
>        }
> +      if (IS_MACOSX) {
> +        let updatesRootDir = gUpdatesRootDir.clone();
> +        while (updatesRootDir.path != updatesDir.path) {
> +          if (updatesDir.exists()) {

I did want to point out that this will be false on the first pass, since |updatesDir| was removed via |removeDirRecursive| a few lines above, but I guess we get some additional protection by first checking |updatesDir| against |updatesRootDir| before doing any further removals, so this is fine.

@@ +975,5 @@
> +            }
> +          }
> +          updatesDir = updatesDir.parent;
> +        }
> +      }      

nit: extra white space
Attachment #8494788 - Flags: review?(spohl.mozilla.bugs) → review+
Attached patch patch updated to comments (obsolete) — Splinter Review
Attachment #8494788 - Attachment is obsolete: true
Attachment #8494905 - Flags: review+
Attachment #8494905 - Attachment is obsolete: true
Attachment #8494909 - Flags: review+
If you would like some extra defence-in-depth on this, it would be relatively straight forward to have puppet empty ~/Library/Caches/Mozilla/updates/ on the test slaves.
It isn't actually for defense and there would be no problems if the tests didn't do this... I just like the tests to do their best to not leave a mess and to clean up after themselves.
Oh yes, we like this too. Thanks for thinking of it.
https://hg.mozilla.org/mozilla-central/rev/4f0a10f05200
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Landed on aurora in the Mac V2 signing combined patch in bug 1047584
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.