MFBT tests rely on MOZ_ASSERT for tests, so they don't test anything in non-DEBUG builds

RESOLVED FIXED in mozilla31

Status

()

defect
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

unspecified
mozilla31
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

See TestTypeTraits and TestBloomFilter.

Either use something else than MOZ_ASSERT (like TestCheckedInt) or at least #define DEBUG at the beginning of the files.
Yeah, that's pretty sucky. We should see if we can reuse some of http://mxr.mozilla.org/mozilla-central/source/xpcom/tests/TestHarness.h
I forgot about this bug, then was about to file it again, and Bugzilla's "you might also like" feature unearthed this.
Is it pretty? No.
Is it what we want to keep using forever? I don't know.
But at least it's a straightforward fix, which should be appreciable after 1.5 years :-P
Attachment #8411044 - Flags: review?(jwalden+bmo)
Comment on attachment 8411044 [details] [diff] [review]
MOZ_RELEASE_ASSERT for MFBT tests

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

Cagefight to determine which patch lands.  :-)

::: mfbt/tests/TestTypeTraits.cpp
@@ +161,5 @@
>  
>  static void
>  StandardIsBaseOfTests()
>  {
> +  MOZ_RELEASE_ASSERT((IsBaseOf<B, D>::value) == true);

We could probably make the tests in this file into static_asserts, but it doesn't much matter if it's this way too.  Wonder why they weren't initially static asserts.
Attachment #8411044 - Flags: review?(jwalden+bmo) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cf3ec066bc7

(Made these static_assert's).
Assignee: nobody → bjacob
Duplicate of this bug: 1000758
https://hg.mozilla.org/mozilla-central/rev/4cf3ec066bc7
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.