Closed Bug 1045920 Opened 5 years ago Closed 5 years ago

Add a unit test for mfbt/RefPtr.h

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: njn, Assigned: njn)

Details

Attachments

(2 files)

mfbt/RefPtr.h has a unit test embedded in comments. This bug is about extracting that into a test file so it actually gets run.
The unit test system apparently can't handle files with the same names in
different directories.
Attachment #8464330 - Flags: review?(nfroyd)
Attachment #8464330 - Flags: review?(nfroyd) → review+
Comment on attachment 8464340 [details] [diff] [review]
(part 2) - Add mfbt/tests/TestRefPtr.cpp

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

WFM, just a few small nits.

::: mfbt/tests/TestRefPtr.cpp
@@ +5,5 @@
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#include "mozilla/RefPtr.h"
> +
> +using namespace mozilla;

MFBT test style is to avoid blanket using statements, instead declaring exactly what you need:

using mozilla::RefCounted;
using mozilla::RefPtr;
using mozilla::TemporaryRef;

@@ +13,5 @@
> +  MOZ_DECLARE_REFCOUNTED_TYPENAME(Foo)
> +
> +  Foo() : mDead(false) {}
> +
> +  ~Foo()

Maybe make this and mDead private in the interests of code hygiene?

@@ +74,5 @@
> +int
> +main()
> +{
> +  // This should blow up.
> +//Foo* f = new Foo(); delete f;

Remove this?

@@ +76,5 @@
> +{
> +  // This should blow up.
> +//Foo* f = new Foo(); delete f;
> +
> +  MOZ_ASSERT(0 == Foo::sNumDestroyed);

Please use MOZ_RELEASE_ASSERT here and throughout, so these tests are exercised in non-debug builds, too.
Attachment #8464340 - Flags: review?(nfroyd) → review+
> Maybe make this and mDead private in the interests of code hygiene?

I made mDead private. Making the destructor private cause problems with the sub-class and maybe making it protected would have worked but a public vanilla destructor doesn't seem so bad.
https://hg.mozilla.org/mozilla-central/rev/c58acbd01f32
https://hg.mozilla.org/mozilla-central/rev/7c2de383cfef
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.