782658 - Make make -C mailnews xpcshell-tests show failures nicely

Status: RESOLVED FIXED

(MailNews Core :: Build Config, defect)

Description

[Mark Banner (:standard8)]

Attachment: The fix

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.

Comment 1

[Mark Banner (:standard8)]

Attachment: The fix v2

Don't use TEST_DIRS so that we don't conflict when we port bug 701822.

Comment 2

[Robert Kaiser]

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?

Comment 3

[Mark Banner (:standard8)]

(In reply to Robert Kaiser (:[e-mail]) 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 4

[Justin Wood (:Callek)]

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

Comment 5

[Mark Banner (:standard8)]

Checked in: https://hg.mozilla.org/comm-central/rev/7c9b7ca2be40