Closed Bug 1317451 Opened 3 years ago Closed 3 years ago

ASSERT_DEATH might not be defined

Categories

(Core :: XPCOM, defect)

x86_64
FreeBSD
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: mds, Assigned: mds)

References

Details

(Keywords: regression)

Attachments

(1 file)

There are few death (g)tests throughout XPCOM; these tests make use of the ASSERT_DEATH macro.

As death tests are not supported on every platform [1], ASSERT_DEATH might not be defined and, hence, lead to build failures.

ASSERT_DEATH instances should be replaced by the non-breaking alternative ASSERT_DEATH_IF_SUPPORTED.

This has been reproduced on FreeBSD 11.

[1] https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md
See Also: → 1013882
Summary: Death tests should be asserted only when supported → ASSERT_DEATH might not be defined
Comment on attachment 8810549 [details]
Bug 1317451 - ASSERT_DEATH replaced by ASSERT_DEATH_IF_SUPPORTED.

https://reviewboard.mozilla.org/r/92820/#review92808

Thanks!  You may also want to file an upstream bug for getting death tests supported on FreeBSD; I can't imagine they'd be too different from the Linux/Solaris/etc. support that's already there.
Attachment #8810549 - Flags: review?(nfroyd) → review+
Pushed by mdesimone@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/76733f2cb30f
ASSERT_DEATH replaced by ASSERT_DEATH_IF_SUPPORTED. r=froydnj
(In reply to Nathan Froyd [:froydnj] from comment #2)

> Thanks!  You may also want to file an upstream bug for getting death tests
> supported on FreeBSD; I can't imagine they'd be too different from the
> Linux/Solaris/etc. support that's already there.

Thank you for the super fast review. The relevant bug has been filed: https://github.com/google/googletest/issues/938
(In reply to Nathan Froyd [:froydnj] from comment #2)
> getting death tests supported on FreeBSD;

Maybe try backporting:
  https://github.com/google/googletest/commit/814a5e9310bb
  https://github.com/google/googletest/commit/b215e30cadd1
https://hg.mozilla.org/mozilla-central/rev/76733f2cb30f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Blocks: 1316437
Keywords: regression
Comment on attachment 8810549 [details]
Bug 1317451 - ASSERT_DEATH replaced by ASSERT_DEATH_IF_SUPPORTED.

http://buildbot.rhaalovely.net/builders/mozilla-aurora-amd64/builds/1866/steps/build/logs/stdio
http://buildbot.rhaalovely.net/builders/mozilla-aurora-freebsd-amd64/builds/1001/steps/build/logs/stdio

Approval Request Comment
[Feature/regressing bug #]: bug 1316437 regression according to our buildbot
[User impact if declined]: broken build on BSDs with empty .mozconfig
[Describe test coverage new/current, TreeHerder]: landed on m-c
[Risks and why]: Low, can only break or make nop tests from bug 1316437
[String/UUID change made/needed]: None
Attachment #8810549 - Flags: approval-mozilla-aurora?
Comment on attachment 8810549 [details]
Bug 1317451 - ASSERT_DEATH replaced by ASSERT_DEATH_IF_SUPPORTED.

build fix for some bsd platforms, take in aurora52
Attachment #8810549 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.