Closed Bug 533332 Opened 14 years ago Closed 14 years ago

[regression] "make -C mailnews xpcshell-tests" doesn't run composer tests

Categories

(MailNews Core :: Build Config, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.1a1

People

(Reporter: BenB, Assigned: BenB)

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

Reproduction:
1. Run "make xpcshell-tests" (wait 20 minutes)
2. Run "make -C mailnews xpcshell-tests" (wait 5 minutes)

Actual result:
Step 1 runs the tests in mailnews/composer/test/ (and all Gecko tests and Mailnews test)
Step 2 does not the tests in mailnews/composer/test/ (although it does run the test in imap, mime, AB etc., but not in those for Gecko, as expected.)

Expected result:
- "make -C mailnews" builds everything in mailnews/
- "make -C mailnews xpcshell-tests" runs all xpcshell unit tests in mailnews/

Reason:
This seems to be a regression caused by bug 509367
Attached patch Proposed patch (obsolete) — Splinter Review
Attachment #416457 - Flags: review?(bugzilla)
Attachment #416457 - Flags: review?(bugzilla) → review+
I think the following patch is better than hacking around a hack.
This is the patch from bug 509367 comment 1 plus Standard8's suggestion in the following comment, plus the Seamonkey change.

I think this is better, also because it allows composer to build in parallel.
Attachment #416461 - Flags: review?(neil)
Attached patch Fix v1.1 (obsolete) — Splinter Review
If you must take the first idea, at least do it by including all of $(DIR), in case somebody adds things to the DIR line.
Attachment #416457 - Attachment is obsolete: true
Attachment #416462 - Attachment description: Fix v1.1 (Discouraged) → Fix v1.1
Comment on attachment 416461 [details] [diff] [review]
Fix v2 - original fix for 509367 + standard8's suggestion
[Checkin: See comment 8+9]

I can't really review the interdiff between two of my patches...
Attachment #416461 - Flags: review?(neil) → review?(bugzilla)
Attachment #416461 - Flags: review?(bugzilla) → review+
It looks like patch v2 is ready for check-in, isn't it?
Is this actually affecting c-1.9.1 only?
Status: NEW → ASSIGNED
I think so (not sure about sr).
I just didn't have time to look at it yet.
Component: Build Config → Testing Infrastructure
QA Contact: build-config → testing-infrastructure
Comment on attachment 416461 [details] [diff] [review]
Fix v2 - original fix for 509367 + standard8's suggestion
[Checkin: See comment 8+9]


http://hg.mozilla.org/comm-central/rev/5fdf1bd04492
Attachment #416461 - Attachment description: Fix v2 - original fix for 509367 + standard8's suggestion → Fix v2 - original fix for 509367 + standard8's suggestion [Checkin: Comment 8]
Attachment #416461 - Flags: approval-seamonkey2.0.3?
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1a1
Version: 1.9.1 Branch → Trunk
Assignee: neil → ben.bucksch
Component: Testing Infrastructure → Build Config
QA Contact: testing-infrastructure → build-config
Attachment #416461 - Flags: approval-seamonkey2.0.3? → approval-thunderbird3.0.2?
Comment on attachment 416461 [details] [diff] [review]
Fix v2 - original fix for 509367 + standard8's suggestion
[Checkin: See comment 8+9]


(In reply to comment #8)
> http://hg.mozilla.org/comm-central/rev/5fdf1bd04492

With added formatting and sorting improvements while there.
Attachment #416461 - Attachment description: Fix v2 - original fix for 509367 + standard8's suggestion [Checkin: Comment 8] → Fix v2 - original fix for 509367 + standard8's suggestion [Checkin: See comment 8+9]
Attachment #416461 - Flags: approval-thunderbird3.0.2? → approval-thunderbird3.0.2-
Comment on attachment 416461 [details] [diff] [review]
Fix v2 - original fix for 509367 + standard8's suggestion
[Checkin: See comment 8+9]

Not taking this on 3.0.1. There's no benefit to users, and the full xpcshell-tests do run in mailnews/compose so, worst case if a developer doesn't catch the failure on a post-1.9.1 branch, the tinderboxes will catch it.
Attachment #416462 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.