Last Comment Bug 782658 - Make make -C mailnews xpcshell-tests show failures nicely
: Make make -C mailnews xpcshell-tests show failures nicely
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 17.0
Assigned To: Mark Banner (:standard8)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-14 08:14 PDT by Mark Banner (:standard8)
Modified: 2012-08-27 10:57 PDT (History)
7 users (show)
standard8: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
The fix (1.46 KB, patch)
2012-08-14 08:14 PDT, Mark Banner (:standard8)
no flags Details | Diff | Splinter Review
The fix v2 (1.48 KB, patch)
2012-08-17 02:10 PDT, Mark Banner (:standard8)
bugspam.Callek: review+
Details | Diff | Splinter Review

Description Mark Banner (:standard8) 2012-08-14 08:14:51 PDT
Created attachment 651776 [details] [diff] [review]
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) 2012-08-17 02:10:39 PDT
Created attachment 652703 [details] [diff] [review]
The fix v2

Don't use TEST_DIRS so that we don't conflict when we port bug 701822.
Comment 2 Robert Kaiser 2012-08-22 17:06:05 PDT
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) 2012-08-23 13:29:52 PDT
(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 4 Justin Wood (:Callek) 2012-08-26 22:26:44 PDT
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) 2012-08-27 10:57:27 PDT
Checked in: https://hg.mozilla.org/comm-central/rev/7c9b7ca2be40

Note You need to log in before you can comment on or make changes to this bug.