Closed Bug 1110956 Opened 6 years ago Closed 6 years ago
Strings .cpp to gtest and enable it
The attached patch is a diff -w, because I also fixed the indentation in this test.
Attachment #8535769 - Flags: review?(nfroyd)
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...
Attachment #8535769 - Flags: review?(nfroyd) → review+
(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
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
Looks like this also broke gtests on ASAN builds: https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4611580&repo=mozilla-inbound
(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. :(
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?
https://hg.mozilla.org/mozilla-central/rev/b648cb645fb3 https://hg.mozilla.org/mozilla-central/rev/40d720f4c1f2 https://hg.mozilla.org/mozilla-central/rev/96f1307908bf
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
No I don't remember.
You need to log in before you can comment on or make changes to this bug.