Closed Bug 807607 Opened 8 years ago Closed 7 years ago

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

Categories

(Core :: MFBT, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: bjacob, Assigned: bjacob)

References

Details

Attachments

(1 file)

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: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.