507339 - Test more files in the complete / partial mar files and check file permissions

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

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

I think it would be a good thing to add a couple of more files for these tests especially for test_0112_general.js which tests the restore of the original files when a partial mar can't be applied

Comment 1

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

Attachment: patch in progress rev1

Comment 2

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

Attachment: patch in progress rev2

Comment 3

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

Attachment: patch in progress rev3

Comment 4

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

Attachment: patch in progress rev 4

Comment 5

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

Attachment: patch rev1

Brad, would you take a look at the tests. It adds tests for bug 505120

Comment 6

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

I've pushed the patch to try

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 392891 [details] [diff] [review]
patch rev1

this looks good.  The only minor comment I have is about the naming scheme for your gTestFiles elements.  It might be more clear if you used a consistent prefix, such as origFile, compareFile, origPerms, comparePerms etc.

Comment 8

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

Attachment: patch rev2

Thanks Brad

Comment 9

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

All of these tests passed on the try servers. I'll check these in after the tree is a tad more stable

Comment 10

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

Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/744cffd14567

Comment 11

[Dave Townsend [:mossop]]

Attachment: remove head_update.js

I think we wanted to remove head_update.js here but it is still in the source tree. This patch is just the file removal.

There is something a little awkward going on here though. People who have previously built have a symlink in _tests/xpcshell/test_update/unit/head_update.js pointing to head_update.js in the source dir. So when the preprocessor is invoked it actually writes the processed head_update.js.in to head_update.js in the source dir. Not sure whether we need to solve this, but it might end with people accidentally landing new copies of this file.

Comment 12

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

Comment on attachment 393356 [details] [diff] [review]
remove head_update.js

bah... thought I removed it along with test_0040_general.js.in. Thanks

Comment 13

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

I'll check this in along with the removal of test_0040_general.js.in as soon as the tree reopens

Comment 14

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

Something strange is going on. I just tried to commit these changes and hg stated nothing changed. Previous to this hg status showed these files as removed and after they are no longer listed... all this without doing a push. I suspect I did push the removals and for some odd reason it didn't push properly

Comment 15

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

I was able to successfully push the two removals using a different repo.

Comment 16

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

followup pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/d33ee97a3606

Comment 17

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

It appears that hg got confused with doing a rename and modification to these files.

Comment 18

[Dave Townsend [:mossop]]

(In reply to comment #11)
> There is something a little awkward going on here though. People who have
> previously built have a symlink in
> _tests/xpcshell/test_update/unit/head_update.js pointing to head_update.js in
> the source dir. So when the preprocessor is invoked it actually writes the
> processed head_update.js.in to head_update.js in the source dir. Not sure
> whether we need to solve this, but it might end with people accidentally
> landing new copies of this file.

This actually isn't true. I realised that a full rebuild wipes out the symlinks so everything is ok, it is only because I was running make in the updates dir for testing that I was seeing this issue.

Comment 19

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

(In reply to comment #18)
This can manifest in a slightly different way, here's what happens in scratchbox for our Maemo builds when doing a hg pull -u:
pulling from http://hg.mozilla.org/mozilla-central
searching for changes
adding changesets
adding manifests
adding file changes
added 7 changesets with 6 changes to 6 files
 local changed toolkit/mozapps/update/test/unit/head_update.js which remote deleted
use (c)hanged version or (d)elete? 

I set a clobber to resolve it.

Comment 20

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

Thanks for taking care of that Nick and sorry it happened. Looks like it happened again too. :(
http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1249962906.1249964346.17736.gz&fulltext=1

Comment 21

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

Yeah, there's something odd happening with the clobberer on Maemo.

Comment 22

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

Bug 509652 for the long-term fix for that, and removed <srcdir>/toolkit/mozapps/update/test/unit/head_update.js from all the slaves.