304162 - Integrate necko unit tests into tinderbox

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

Necko unit tests should be integrated into tinderbox so that the tree turns
orange when one of these tests fails.

bsmedberg recently added another test (TestVersionComparator, iirc); but that
one is run during make libs. I'm not sure this is great. I suggest adding a
treewide "make check" target that runs the various automated tests that are
available. (automake provides the same target).

Tinderbox could automatically run "make check" on the tree if tests are enabled.

What do you think?

Comment 1

[Darin Fisher]

If someone has time to work on 'make check' then I suppose that would be ideal,
but if we're short of time, then hacking an additional tinderbox test would seem
the best choice.  It wouldn't take much time :-)

Comment 2

[Benjamin Smedberg]

I think "make check" sounds fine. I do think it should be part of the default
build process if we have --enable-tests and aren't cross compiling (the tiers
would then be built export/libs/check).

What do you think about stuff like the compare-locales tests? I currently am
doing those libs as well (see extensions/reporter/locales/Makefile.in). Is that
something that should be part of "check" or "libs"? It's a source-code checker,
not a runtime checker, so perhaps it should stay where it is.

Comment 3

[Darin Fisher]

The necko unit tests make network requests (or at least the test created by
biesi does).  We could and should probably convert those tests over to using a
loopback server socket created by the testcase (via nsIServerSocket), but until
that happens, it probably doesn't make sense for the build to block on running
these tests.  I'm also not sure that the build should break completely just
because some necko test fails.  An orange tinderbox seems better to me.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: the easy way: FileBasedTest

this uses FileBasedTest to just run the necko tests.  it probably needs a
tinder-config setting. however, I think I'd rather implement the make check as
suggested above...

Comment 5

[Darin Fisher]

It's up to you, but how about we land the easy way first? ;-)

Comment 6

[cls]

I don't think the runtime tests should be enabled by default in the normal
build.  It isn't that much more effort for a developer to add 'make check' to
their local build script.  For the tinderboxes, maybe a 'RunRegressionTests'
function could be added that just calls 'make check'.

See also bug 63228 & bug 63368 .

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

ah, I weren't aware of those bugs. ok. I'll use this bug to get the easy fix in,
and make patches for the other stuff in those two bugs.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: FileBasedTest with setting (defaults to true)

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Checked in; what does it take to get tbox machines updated? Should I file a new
bug about them?

Checking in tools/tinderbox/build-seamonkey-util.pl;
/cvsroot/mozilla/tools/tinderbox/build-seamonkey-util.pl,v  <-- 
build-seamonkey-util.pl
new revision: 1.300; previous revision: 1.299
done
Checking in tools/tinderbox/tinder-defaults.pl;
/cvsroot/mozilla/tools/tinderbox/tinder-defaults.pl,v  <--  tinder-defaults.pl
new revision: 1.79; previous revision: 1.78
done

Comment 10

[Chase Phillips]

(In reply to comment #9)
> Checked in; what does it take to get tbox machines updated?

Time from somebody on the build team.

> Should I file a new bug about them?

Yes, a bug will help us prioritize the task while placing it in our queue. 
Please list the systems you would like this enabled on in the order of
importance as we may only have time to eat through the list in pieces.

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

thanks, filed Bug 304234