1110956 - Port TestStrings.cpp to gtest and enable it

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[(no longer active)]

Attachment: Patch (v1)

The attached patch is a diff -w, because I also fixed the indentation in this test.

Comment 1

[Nathan Froyd [:froydnj]]

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

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

Fabulous.

::: xpcom/glue/tests/gtest/TestStrings.cpp
@@ +479,1 @@
>    */

I wonder why this test is commented out?  It looks like it should succeed...

Comment 2

[(no longer active)]

(In reply to Nathan Froyd (:froydnj) from comment #1)
> ::: xpcom/glue/tests/gtest/TestStrings.cpp
> @@ +479,1 @@
> >    */
> 
> I wonder why this test is commented out?  It looks like it should succeed...

Who can say?!  But it does succeed, I turned it on.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d468b88bdb23

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/599e1b8d3cbc and https://hg.mozilla.org/integration/mozilla-inbound/rev/463dbe09e4e4 for linux build bustage:

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4611334&repo=mozilla-inbound

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Looks like this also broke gtests on ASAN builds: https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4611580&repo=mozilla-inbound

Comment 5

[(no longer active)]

(In reply to Wes Kocher (:KWierso) from comment #4)
> Looks like this also broke gtests on ASAN builds:
> https://treeherder.mozilla.org/ui/logviewer.
> html#?job_id=4611580&repo=mozilla-inbound

Ah, this is why it's bad to leave tests disabled like this. :(

Comment 6

[(no longer active)]

Actually it was my mistake, I converted a raw pointer to an nsRefPtr while forgetting to remove Release() calls on it. :/

Benjamin, do you remember what happened the last time that we discussed disallowing direct ->AddRef/Release() calls on our smart pointers?

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b648cb645fb3
https://hg.mozilla.org/mozilla-central/rev/40d720f4c1f2
https://hg.mozilla.org/mozilla-central/rev/96f1307908bf

Comment 8

[Benjamin Smedberg]

No I don't remember.