Page MenuHomeFreeBSD

Do initial googlemock/googletest integration into the build/FreeBSD test suite
ClosedPublic

Authored by ngie on Mar 11 2019, 11:54 PM.

Details

Summary

This initial integration takes googlemock/googletest release 1.8.1, integrates
the library, tests, and sample unit tests into the build.

googlemock/googletest’s inclusion is optionally available via MK_GOOGLETEST. MK_GOOGLETEST is dependent on
MK_TESTS and is enabled by default when built with a C++11 capable toolchain.

Google tests can be specified via the GTESTS variable, which, in comparison
with the other test drivers, is more simplified/streamlined, as Googletest only supports C++ tests; not raw C or shell tests (C tests can be written in C++ using the standard embedding methods).

No dependent libraries are assumed for the tests. One must specify gmock,
gmock_main, gtest, or gtest_main, via LIBADD for the program.

More information about googlemock and googletest can be found on the
Googletest project page and the
GoogleMock
and
GoogleTest
docs.

These tests are originally integrated into the build as plain driver tests, but
will be natively integrated into Kyua in a later version.

Known issues/Errata:

Test Plan

I have run all of the integration tests successfully, minus
WhenDynamicCastToTest.AmbiguousCast. I have marked the testcase as an
expected failure, so it does not affect the overall test suite result.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 23021
Build 22093: arc lint + arc unit

Event Timeline

ngie created this revision.Mar 11 2019, 11:54 PM
asomers added inline comments.Mar 12 2019, 12:43 AM
share/mk/Makefile
76

Should these files be conditionalized on MK_GOOGLETEST?

share/mk/googletest.test.inc.mk
7

Why do you have to specify -std=c++11? What happens if you don't? Clang defaults to the gnu++14. With this in there, I have to manually add -std=c++14 to my own makefile.

tools/build/options/WITHOUT_GOOGLETEST
2

s/not/neither/g

ngie marked 2 inline comments as done.Mar 12 2019, 1:16 AM
ngie added a subscriber: dim.
ngie added inline comments.
share/mk/Makefile
76

If I do, it would introduce unnecessary complexity and dependencies on src.opts.mk, and make it impossible for others to use bsd.test.mk in ports, etc :/...

share/mk/googletest.test.inc.mk
7

googletest compiles with -std=c++11 right now. I will made this optional via CXXSTD, like I suggested to @dim a while (2 years or so) ago, and will work on shuffling the value/flag setting over to bsd.sys.mk, etc.

ngie marked an inline comment as done.Mar 12 2019, 1:16 AM
ngie updated this revision to Diff 54954.

Update based on comments from @asomers

asomers added inline comments.Mar 12 2019, 2:53 PM
share/mk/Makefile
76

Makes sense.

share/mk/googletest.test.inc.mk
7

Are you saying that googletest itself requires -std=c++11? or that any googletest client needs c++11 at a minimum?

ngie added inline comments.Mar 12 2019, 3:01 PM
share/mk/googletest.test.inc.mk
7

Mostly the latter, however, newer versions of the spec might break functionality developed and tested with c++11 (very unlikely, but possible).

asomers added inline comments.Mar 12 2019, 3:08 PM
share/mk/googletest.test.inc.mk
7

Ok, but why put it in this file instead of bsd.sys.mk ?

ngie edited the summary of this revision. (Show Details)Mar 12 2019, 3:10 PM
ngie marked an inline comment as done.Mar 12 2019, 3:13 PM
ngie added inline comments.
share/mk/googletest.test.inc.mk
7

I hadn’t gotten around to figuring out the appropriate format for putting it in there. I’ll work on it in parallel with this though.

ngie marked an inline comment as done.Mar 12 2019, 3:26 PM
ngie added inline comments.
share/mk/googletest.test.inc.mk
7

The reason why this matters is that PowerPC and mips are both still built with gcc 4.2.1, which will require a lower language spec version than clang and not all versions of clang support the C++11 spec.

asomers accepted this revision.Mar 12 2019, 3:28 PM
asomers added inline comments.
share/mk/googletest.test.inc.mk
7

Ahh, that would explain why you put it in this file, because PowerPC and mips don't use this file.

This revision is now accepted and ready to land.Mar 12 2019, 3:28 PM

Mentor approved.

Why is this review still open? Was the Phabricator hook not working on the day that you committed?

ngie closed this revision.Mar 21 2019, 2:47 PM

Why is this review still open? Was the Phabricator hook not working on the day that you committed?

I think it choked on the commit import *sigh*.