Closed Bug 782658 Opened 8 years ago Closed 8 years ago

Make make -C mailnews xpcshell-tests show failures nicely

Categories

(MailNews Core :: Build Config, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 17.0

People

(Reporter: standard8, Assigned: standard8)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch The fix (obsolete) — Splinter Review
Currently when you run "make -C mailnews xpcshell-tests" if there's a failure in a directory, you don't actually know that unless you scroll back through the logs.

I've been looking at fixing this on and off, and never come up with a good solution until now. I realised today that using runxpcshelltests.py we can select the directories we want to run.

Hence, we can take our existing hack and replace it with a better one - now when we run the mailnews tests they will run all the way through, presenting one result summary at the end.

The code is copied and slightly modified from config/rules.mk, we can't re-use that because of how the directory selection works.
Attachment #651776 - Flags: review?(bugspam.Callek)
Attached patch The fix v2Splinter Review
Don't use TEST_DIRS so that we don't conflict when we port bug 701822.
Attachment #651776 - Attachment is obsolete: true
Attachment #651776 - Flags: review?(bugspam.Callek)
Attachment #652703 - Flags: review?(bugspam.Callek)
Attachment #652703 - Flags: review?(kairo)
Comment on attachment 652703 [details] [diff] [review]
The fix v2

Review of attachment 652703 [details] [diff] [review]:
-----------------------------------------------------------------

Hrm, I don't really like this hack for two reasons:
1) It needs us to have two lists of directories and remember to keep them in sync.
2) It duplicates the call logic for the tests instead of having those calls in one place and only have to change rules.mk if something changes with those calls.

I'm also not sure if this will bode well with the way the build system is being reworked, we should make sure of that.

Callek, what do you think?
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #2)
> Comment on attachment 652703 [details] [diff] [review]
> The fix v2
> 
> Review of attachment 652703 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hrm, I don't really like this hack for two reasons:
> 1) It needs us to have two lists of directories and remember to keep them in
> sync.

We did this previously, this just alters the nature of the previous hack.

> 2) It duplicates the call logic for the tests instead of having those calls
> in one place and only have to change rules.mk if something changes with
> those calls.

I think we'd probably be able to rework this, if the build system correctly supported multi-level xpcshell.ini files correctly (e.g. we could have one in mailnews/ and not just the hack we have at app level).

> I'm also not sure if this will bode well with the way the build system is
> being reworked, we should make sure of that.

At the moment I was just looking at improving the current hack rather than fixing this long term, which I recognize we will need to do at some stage.
Comment on attachment 652703 [details] [diff] [review]
The fix v2

Review of attachment 652703 [details] [diff] [review]:
-----------------------------------------------------------------

I agree with all of KaiRo's comments, but I also agree with Mark's. And afaict this is useable as a "make the last hack work again -- but still hacky" I do agree we *need* a better solution down the road
Attachment #652703 - Flags: review?(kairo)
Attachment #652703 - Flags: review?(bugspam.Callek)
Attachment #652703 - Flags: review+
Checked in: https://hg.mozilla.org/comm-central/rev/7c9b7ca2be40
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 17.0
You need to log in before you can comment on or make changes to this bug.