599477 - test_0110_general.js, test_0111_general.js, test_0112_general.js fail on 10.5 from a i386+x86_64 universal build

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

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

Originally found in bug 598507 comment #19, where there's a log at attachment
477805. We're switching from ppc+i386 universal to i386+x86_64 universal in bug 571367, and are planning to temporarily disable these tests for that. Basically
we need to make sure we launch the i386 arch of any util the test uses, when
running on 10.5.

TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/mozilla-central_leopard_test-xpcshell/build/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0110_general.js | test failed (with xpcshell return code: 0), see following log:
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/mozilla-central_leopard_test-xpcshell/build/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0110_general.js | 256 == 0 - See following stack:
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/mozilla-central_leopard_test-xpcshell/build/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0111_general.js | test failed (with xpcshell return code: 0), see following log:
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/mozilla-central_leopard_test-xpcshell/build/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0111_general.js | 256 == 0 - See following stack:
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/mozilla-central_leopard_test-xpcshell/build/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0112_general.js | test failed (with xpcshell return code: 0), see following log:
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/mozilla-central_leopard_test-xpcshell/build/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0112_general.js | 256 == 0 - See following stack:

Comment 1

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

(In reply to comment #0)
> Originally found in bug 598507 comment #19, where there's a log at attachment
> 477805 [details]. We're switching from ppc+i386 universal to i386+x86_64 universal in bug
> 571367, and are planning to temporarily disable these tests for that. Basically
> we need to make sure we launch the i386 arch of any util the test uses, when
> running on 10.5.
Why is that the case? Is nsIProcess not doing the right thing? If so, is there a bug to get nsIProcess to do the right thing?

Comment 2

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

Not that I know of. Bug 599478 may be the root cause though.

Comment 3

[(not currently active) Ted Mielczarek]

nsIProcess clearly doesn't do the right thing (run the arch you're currently running). Doing it correctly is sort of a pain, since you probably have to verify that the binary contains the architecture you expect to want to run.

Comment 4

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

I wonder if going back to using PR_CreateProcess and PR_CreateProcessDetached as prior to bug 263429 being fixed would fix it. If not that or some other fix then nsIProcess is going to be busted under some usages.

btw: I would hope that nsIProcess were able to launch both x86 / x64 processes no matter what arch the app is running under.

Comment 5

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

note to self... check the code especially if you haven't looked at it in a while.

The updater is a bundle so I wonder if it would work to just pass "arch", "-arch", and "i386" for it and launch the bundle instead?

Comment 6

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

I was going to suggest that updater.app should use the same Info.plist trick as on the main app, and set 10.6 as the minimum requirement to run 64bit. Then I discovered it already does.

Comment 7

[Josh Aas]

Attachment: fix v1.0

First draft, haven't tested it yet.

Comment 8

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

Josh, thanks for working on the bork'd nsIProcess. It would probably be best to file a core -> xpcom bug for fixing this  and make this bug and the other bugs for the disabled tests that are broken due to nsIProcess not working properly depend on it.

Comment 9

[Josh Aas]

Comment on attachment 479565 [details] [diff] [review]
fix v1.0

Moving this work to bug 600711.

Comment 10

[(not currently active) Ted Mielczarek]

We should be able to reenable these tests now.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Looks to me like they were reenabled by http://hg.mozilla.org/mozilla-central/rev/861afa477aba .

Comment 12

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

They were.