Closed Bug 450884 Opened 16 years ago Closed 16 years ago

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

Categories

(Mozilla Messaging Graveyard :: Release Engineering, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: standard8, Assigned: gozer)

References

Details

Attachments

(3 files)

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.
Attached patch [checked in] Possible help (deleted) — Splinter Review
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 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.
Attachment #334110 - Flags: review?(bienvenu)
Depends on: 451372
Attachment #334110 - Flags: review?(bienvenu) → review+
Comment on attachment 334110 [details] [diff] [review] [checked in] Possible help Checked in changeset id 146:aa8cb94fe1ba
Attachment #334110 - Attachment description: Possible help → [checked in] Possible help
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.
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.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Attached patch [checked in] The fix (deleted) — Splinter Review
Follow-up fix to remove the symlinks/files before writing to them so that we don't affect the source directory files.
Attachment #335293 - Flags: review?(bienvenu)
Attachment #335293 - Flags: review?(bienvenu) → review+
Comment on attachment 335293 [details] [diff] [review] [checked in] The fix Checked in, changeset id: 195:e3663bfb9e16
Attachment #335293 - Attachment description: The fix → [checked in] The fix
Attachment #336626 - Flags: review?(bienvenu) → review+
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
Attachment #336626 - Attachment description: Don't change the source file for test_punycodeURIs.js either → [checked in] Don't change the source file for test_punycodeURIs.js either
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: