404208 - url-classifier tests never actually run on the unit test machines

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Reed Loden [:reed]]

From toolkit/components/url-classifier/Makefile.in :

  48 ifdef ENABLE_TESTS
  49 ifndef MOZ_ENABLE_LIBXUL
  50 TOOL_DIRS     += tests
  51 endif
  52 endif

We aren't running any of the tests? That seems really, really, really bad. What if there's a really bad bug which these tests would catch, but because we never actually run them, we don't know about the bug!

bsmedberg added this ifndef at 2007-05-23 06:24	with log message "Fix url-classifier+libxul warnings and test errors, r=luser (no bug)".

Comment 1

[Benjamin Smedberg]

Is the unit-test machine building with libxul? That's probably not good, because there are a fair number of binary tests that are disabled (i.e. don't link correctly) with libxul.

Comment 2

[Reed Loden [:reed]]

(In reply to comment #1)
> Is the unit-test machine building with libxul? That's probably not good,
> because there are a fair number of binary tests that are disabled (i.e. don't
> link correctly) with libxul.

http://mxr.mozilla.org/mozilla/source/tools/buildbot-configs/testing/unittest/

I don't see --disable-libxul anywhere in those files, so most likely, they are building with libxul.

Comment 3

[Mike Connor [:mconnor]]

We should do this, but its more of a "we should do this today" than a blocker for release.

Comment 4

[Marria Nazif]

Any update on this?

Comment 5

[Marria Nazif]

I am nominating this for blocking since I think it's important to run these tests.  If someone can confirm that the tests are in fact still running, even with the ifndef, then I'm ok not blocking on this.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

The tests aren't running on the machines which do our nightly builds, as the tests are incompatible with the build configuration that we ship - we don't want to modify the build configuration (as then we wouldn't be testing what we're shipping) but at the same time we don't want to not run these tests.

This bug calls for the tests to be runnable on nightly builds, which I don't think blocks.

OTOH, we need to make sure that we *do* run the tests on the bits we use for a release, so I'm cc'ing tony to ensure that the todo of "run url classifier tests" gets added to his testplan. Dave should be able to help QA figure out what they need to do there.

Comment 8

[Dave Camp (:dcamp)]

Attachment: enable xpcshell tests

We can at least enable the xpcshell tests.

Comment 9

[Tony Chang (Google)]

Comment on attachment 360985 [details] [diff] [review]
enable xpcshell tests

Yes.

Comment 10

[Dave Camp (:dcamp)]

Looks like this was done elsewhere.