1126295 - Move TestAtoms.cpp to gtest and enable it

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Comment 1

[(no longer active)]

Attachment: Patch (v1)

Comment 2

[Nathan Froyd [:froydnj]]

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

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

::: xpcom/tests/gtest/UTFStrings.h
@@ +17,5 @@
>        { 'a', 'b', 'c', 'd' } },
>      { { '1', '2', '3', '4' },
>        { '1', '2', '3', '4' } },
>      { { 0x7F, 'A', 0x80, 'B', 0x101, 0x200 },
> +      { 0x7F, 'A', char(0xC2), char(0x80), 'B', char(0xC4), char(0x81), char(0xC8), char(0x80) } },

Ah, compiler warnings and the changes they require to the code, how I love them.

Comment 3

[(no longer active)]

These tests are busted, I backed them out:

http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-win32/1422380541/mozilla-inbound-win32-bm82-build1-build5368.txt.gz


TEST-UNEXPECTED-FAIL | Atoms.Table | SEH exception with code 0xc0000005 thrown in the test body. @ (null):-1
TEST-UNEXPECTED-FAIL | Atoms.Table | test completed (time: 0ms)
TEST-UNEXPECTED-FAIL | Atoms.Permanent | SEH exception with code 0xc0000005 thrown in the test body. @ (null):-1
TEST-UNEXPECTED-FAIL | Atoms.Permanent | test completed (time: 0ms)
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | Value of: err
TEST-UNEXPECTED-FAIL | UTF.Hash16 | test completed (time: 4ms)
TEST-UNEXPECTED-FAIL | GTest unit test: failed
gtest TEST-UNEXPECTED-FAIL | gtest | test failed with return code 1

Comment 4

[(no longer active)]

This reproduces reliably on try: <https://treeherder.mozilla.org/#/jobs?repo=try&revision=3703d3deac4b>

I have been unable to reproduce locally so far.

Comment 5

[(no longer active)]

I have been unable to reproduce this locally.  Bug 1136841 makes it impossible to get a stack trace for this crash on the try server, and because we do not package xul-gtest, running the gtests locally from a build produced on the try server doesn't work either.

Comment 6

[sajitk]

Attachment: 1126295-new.patch

I was able to reproduce the SEH exception on my local non-debug build on Windows 7. I then isolated the exception to the method:

> bool isStaticAtom(nsIAtom* atom)

To isolate the statement in which the crash was happening, I broke up the single statement with & to a series of statements with &=, as in the attached patch.

And the SEH exception went away. I have no idea why the change stopped the crash (related to evaluation order maybe?)

I wanted to run this on the try server, but I was not sure which tests include the gTests. Could you please let me know and I can schedule a run to try.

Comment 7

[sajitk]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0589dd5c586d

Comment 8

[Nathan Froyd [:froydnj]]

Comment on attachment 8673829 [details] [diff] [review]
1126295-new.patch

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

Cool!  Thanks for tracking that down!  The 'B' step on try currently runs gtests, so you're all set as far as confirming your changes are OK.  I'd say that your existing try push is sufficient for checkin-needed.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7cf5a24145ac

Comment 10

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

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

Comment 11

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

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