Closed
Bug 658666
Opened 13 years ago
Closed 13 years ago
Comm-central xpcshell-tests busted due to bug 616999 / switching xpcshell to manifests
Categories
(MailNews Core :: Build Config, defect)
MailNews Core
Build Config
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 5.0b1
People
(Reporter: standard8, Assigned: standard8)
References
Details
Attachments
(2 files)
(deleted),
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
kairo
:
review+
|
Details | Diff | Splinter Review |
I'm just working on committing the changes for Thunderbird for porting bug 616999, I'll comment with more details of the landing and follow-ups required in a moment.
Comment 1•13 years ago
|
||
Please ping with any additional requirements to manifestparser that you might desire
Assignee | ||
Updated•13 years ago
|
Severity: normal → blocker
Summary: Tidy up port of bug 616999 / switching of xpcshell to manifests → Comm-central xpcshell-tests busted due to bug 616999 / switching xpcshell to manifests
Assignee | ||
Comment 2•13 years ago
|
||
Ok, I've done the hard work, but someone else is going to have to finish fixing this up - I'm going to be moving house for the next few days, and I won't be able to get much time until our next merge point (just 4 days!), by which time we need to have landed some other patches.
This patch:
- defines manifest files for xpcshell tests that affects Thunderbird
- gets it partially working.
Hacks:
- We copy xpcshell.ini via mail/app/Makefile, because overriding testing/xpcshell/Makfile.in is difficult, and mail/app is about the last thing the build does.
- We have to duplicate everything that is mozilla/ because of two reasons. First manifest doesn't seem to support nested includes (bug 658671), second, we probably can't share the core xpcshell.ini anyway as there's tests we don't want to run (e.g. browser).
Outstanding blocker:
- Despite the fact that the mozilla/ tests are listed in the manifest file. None of them get run even though running xpcshell-tests passes.
If we can fix that issue, then I'd be happy for this to land with a green run on try (with approximately correct xpcshell-test totals) and rs=me, and we can do the rest in follow-ups.
Other issues (can do in follow-ups):
- SeaMonkey and maybe Calendar need to patch their part of the build so that it has the relevant manifest files.
- We should fix doing make xpcshell-tests in mailnews/. This would probably be easier if nested includes worked as we could share the manifest.
- We should sort out some of our tests which are platform specific and are currently testing via what interfaces are available. Currently I believe the manifest format doesn't support doing run-if.os = xxx or skip-if.os = xxx on individual tests, but directories. I'm not sure if that's a bug or not. It would probably be annoying to have to create lots of extra directories just for OS specific tests.
Comment 3•13 years ago
|
||
Comment on attachment 534129 [details] [diff] [review]
Partial fix - checked in.
This gets mailnews and mail (and ldap) xpcshell tests working, which Standard8 and I agreed was sufficient to re-open the tree, especially since we're getting to a branch point in 4 days. Marking r=me, for what that's worth...
Attachment #534129 -
Attachment description: Partial fix → Partial fix - checked in.
Attachment #534129 -
Flags: review+
Comment 4•13 years ago
|
||
this patch didn't turn any of the tinderboxes green (in fact, the errors seem to be the same as before I landed, except for a new error on linux). Maybe I screwed up landing the patch, but the diff looks OK...
Comment 5•13 years ago
|
||
Needless to say, I'm not re-opening the tree anytime soon.
Comment 6•13 years ago
|
||
http://hg.mozilla.org/comm-central/rev/6591bfe0dac0 pushed for windows and linux build bustage (nomac -> notmac)
Comment 7•13 years ago
|
||
I see you edit config/rules.mk, do you add in build/xpccheck.py? I am not too familiar with comm-central, but that is needed for the changes that were added to config/rules.mk.
Comment 8•13 years ago
|
||
(In reply to comment #7)
> I see you edit config/rules.mk, do you add in build/xpccheck.py? I am not
> too familiar with comm-central, but that is needed for the changes that were
> added to config/rules.mk.
If Standard8 changed that file, he didn't include it in his diff. I can look at that. My last change has made tinderbox a bit greener, though.
Assignee | ||
Comment 9•13 years ago
|
||
(In reply to comment #7)
> I see you edit config/rules.mk, do you add in build/xpccheck.py? I am not
> too familiar with comm-central, but that is needed for the changes that were
> added to config/rules.mk.
Our version of config/rules.mk will pick up the build/xpccheck.py from the mozilla/ sub directory, i.e. the mozilla-central version of it.
Comment 10•13 years ago
|
||
I did re-open the tree.
Comment 11•13 years ago
|
||
> - We should sort out some of our tests which are platform specific and are
> currently testing via what interfaces are available. Currently I believe the
> manifest format doesn't support doing run-if.os = xxx or skip-if.os = xxx on
> individual tests, but directories.
Hmm. In Bug 658928 Comment 0:
> xpcshell manifests (bug 616999) allow us to disable tests based on platform
> and other things like whether we're in a debug build or not, e.g.:
>
> [test_foo.js]
> skip-if.os = mac
> skip-if.config = debug
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → dbienvenu
Target Milestone: --- → Thunderbird 3.3a4
Assignee | ||
Comment 12•13 years ago
|
||
This gets the core xpcshell-tests running again. This worked for Thunderbird on try server, although there is one new orange in the core tests, which we can deal with separately (I'd rather get them mostly running again).
Basically, we use the core xpcshell.ini for the core tests, copied to a different location and included from our own. This works because if you're including an xpcshell-test file, then the file locations are relative to the directory you're in. Also, due to the separate build systems, there's no checks for what we're doing, so we get away with it for now at least.
We can't currently do this hack for including a mailnews xpcshell.ini file, as the build system doesn't cope correctly (see bug 658671 comment 3).
Attachment #539156 -
Flags: review?(kairo)
Attachment #539156 -
Flags: review?(bugspam.Callek)
Assignee | ||
Comment 13•13 years ago
|
||
(and I'll take the review of whoever reviews it first).
Assignee | ||
Updated•13 years ago
|
Assignee: dbienvenu → mbanner
Comment 14•13 years ago
|
||
Comment on attachment 539156 [details] [diff] [review]
Actually run core tests - checked in
Looks good to me, given you tested it. Things can only get better using that.
Attachment #539156 -
Flags: review?(kairo) → review+
Assignee | ||
Updated•13 years ago
|
Attachment #539156 -
Flags: review?(bugspam.Callek)
Assignee | ||
Updated•13 years ago
|
Attachment #539156 -
Attachment description: Actually run core tests → Actually run core tests - checked in
Assignee | ||
Comment 15•13 years ago
|
||
I just filed bug 718408 on the central mailnews xpcshell.ini file, which covers the remaining part of this. Hence, resolving this as fixed.
You need to log in
before you can comment on or make changes to this bug.
Description
•