1072554 - Remove update directories when running xpcshell tests

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Robert Strong (they/them - no direct email)]

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.

Comment 1

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1

Comment 2

[Robert Strong (they/them - no direct email)]

Attachment: patch rev2

Fixed an over 80

Comment 3

[Robert Strong (they/them - no direct email)]

Pushed to oak
https://hg.mozilla.org/projects/oak/rev/65326ac33f25

Comment 4

[Stephen A Pohl [:spohl]]

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

Comment 5

[Robert Strong (they/them - no direct email)]

Attachment: patch updated to comments

Comment 6

[Robert Strong (they/them - no direct email)]

Attachment: patch updated to comments

Comment 7

[Robert Strong (they/them - no direct email)]

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

Comment 8

[Nick Thomas [:nthomas] (UTC+12)]

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.

Comment 9

[Robert Strong (they/them - no direct email)]

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.

Comment 10

[Nick Thomas [:nthomas] (UTC+12)]

Oh yes, we like this too. Thanks for thinking of it.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4f0a10f05200

Comment 12

[Robert Strong (they/them - no direct email)]

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