1111149 - Move TestArray.cpp to gtest and enable it

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[(no longer active)]

Attachment: Patch (v1)

Comment 1

[Nathan Froyd [:froydnj]]

Comment on attachment 8535957 [details] [diff] [review]
Patch (v1)

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

::: xpcom/glue/tests/gtest/TestArray.cpp
@@ -70,5 @@
> -{
> -  if (aValue1 == aValue2) {
> -    return "OK";
> -  }
> -  if (kExitOnError) {

You should remove the kExitOnError variable, too.

@@ +152,5 @@
>      array->RemoveLastElement(foo);
>      int32_t   removeResult4[9] = {0, 1, 2, 4, 3, 5, 7, 8, 9};
> +    CheckArray(array, 9, removeResult4, 9);
> +
> +    foo = nullptr;

Is this really necessary?  The presence or absence of |foo| shouldn't affect the tests below, so can't we just let it autorelease at the end of the function?

Comment 2

[(no longer active)]

(In reply to Nathan Froyd (:froydnj) from comment #1)
> @@ +152,5 @@
> >      array->RemoveLastElement(foo);
> >      int32_t   removeResult4[9] = {0, 1, 2, 4, 3, 5, 7, 8, 9};
> > +    CheckArray(array, 9, removeResult4, 9);
> > +
> > +    foo = nullptr;
> 
> Is this really necessary?  The presence or absence of |foo| shouldn't affect
> the tests below, so can't we just let it autorelease at the end of the
> function?

The number of refs is explicitly tested, so yes this is necessary.  The old version of the test called Release() manually.

Comment 3

[Nathan Froyd [:froydnj]]

Ah, gCount, right.  Carry on!

Comment 4

[Nigel Babu [:nigelb]]

https://hg.mozilla.org/mozilla-central/rev/7bad83346e58