450884 - Unit check tinderboxes keep on failing with random errors when interfaces are changed

Status: RESOLVED FIXED

(Mozilla Messaging Graveyard :: Release Engineering, defect)

Description

[Mark Banner (:standard8)]

Frequently when we check patches in that contain interface changes in mailnews/ all three Thunderbird unit test boxes go orange. Usually, these go green the next cycle, or a clobber fixes it. However, its a pain to development, and more of a problem if the thunderb* bots go offline.

I've replicated the process on my machine locally:

1) Build Thunderbird
2) make -C mailnews check (passes)
3) Apply (or remove a patch) e.g. bug 413260 or maybe bug 449618 is more up to date
4) Rebuild Thunderbird (dep build)
5) make -C mailnews check

This last step fails. On today's change (bug 449618) it is causing segfaults.

When we checked in bug 413260 it was one of the NS_ERROR_XPC_* error codes (http://mxr.mozilla.org/comm-central/source/mozilla/js/src/xpconnect/idl/nsIXPConnect.idl)

I briefly looked at this a while ago (with the patch from bug and there appeared to be two solutions:

a) running "make check" twice, ignoring the first search results
b) running the full "make check" (including core etc) with a patched set of tests, to avoid the ones that currently fail (which is currently blocked by bug 431125).
c) fix the xcpshell bug

So I think that this is probably a bug with xpcshell, but I haven't got time to look into it further, and we could do with more reliable tests sooner.

Option a) may be worth trying if we can get tinderbox not to fail on the first run, though from today's results it might not work.

Option b) is only an option if we can disable the failing tests for now or find an appropriate subset of directories to run.

Assigning to gozer to take a look at this, but I've copied Benjamin and Ted in as well in case they have any ideas.

Comment 1

[Mark Banner (:standard8)]

Attachment: [checked in] Possible help

I had an idea earlier. As make check only fails on three tests (see bug 431125) we could disable just those three tests.

As the xpcshell tests currently don't allow todos, or equivalent, I realised we could do this in our own TB makefiles whilst not ideal, and a bit hacky, it would at least allow us to run the full make check.

Attaching the patch that would do this - for TestCookie (a compiled test), I had to "borrow" a passing test from the bin directory.

This would give us more testing coverage (although FF does most of this for us already anyway), but hopefully allow this bug to be fixed easily.

Thoughts?

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 334110 [details] [diff] [review]
[checked in] Possible help

Some of us discussed this on irc last night, and thought it was a good idea (so hopefully we'll be able to stabilise tinderboxes a bit).

I've just double checked this on mac and we pass fine.

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 334110 [details] [diff] [review]
[checked in] Possible help

Checked in changeset id 146:aa8cb94fe1ba

Comment 4

[Mark Banner (:standard8)]

Gozer, can you do the necessary now to http://hg.mozilla.org/build/index.cgi/buildbot-configs/file/07cbcf0816ca/thunderbird-unittest/master.cfg to get all three platforms running the full make check? (i.e. running in build/objdir-tb rather than build/objdir-tb/mailnews).

In theory (assuming Firefox tree stays stable) they should all pass and we'll be looking at TUnit numbers in the low 300s.

If Mac fails, then we might need to work out what we want to do there - but lets see how it goes, we've still got 3 weeks to code freeze so I think we can always cope with a day of known orange.

Comment 5

[Philippe M. Chiasson (:gozer)]

http://hg.mozilla.org/build/buildbot-configs/index.cgi/rev/242
http://hg.mozilla.org/build/buildbot-configs/index.cgi/rev/243

Pushed that change to the buildbots and reconfigured.

Comment 6

[Mark Banner (:standard8)]

After adding a couple of checks to see if the files actually existed or not, we got make check working globally - the only problem was on mac, which is already tracked as bug 447999 (looks to be OS version specific).

Based on the tests I did originally I'm going to close this now as fixed, and we'll monitor it to see how it goes.

Comment 7

[Mark Banner (:standard8)]

Attachment: [checked in] The fix

Follow-up fix to remove the symlinks/files before writing to them so that we don't affect the source directory files.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 335293 [details] [diff] [review]
[checked in] The fix

Checked in, changeset id: 195:e3663bfb9e16

Comment 9

[Mark Banner (:standard8)]

Attachment: [checked in] Don't change the source file for test_punycodeURIs.js either

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 336626 [details] [diff] [review]
[checked in] Don't change the source file for test_punycodeURIs.js either

Checked in, changeset id: 251:755089594547