Closed Bug 1343557 Opened 3 years ago Closed 3 years ago

Update version of gtest in tree to 1.7.0

Categories

(Testing :: General, enhancement, P1)

Version 3
enhancement

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: dminor, Assigned: dminor)

References

Details

Attachments

(5 files)

The current version in tree is 1.6.0. The latest version is 1.7.0.

In Bug 964133 we want to start building the webrtc.org gtests, which use 1.7.0. If we want to avoid having multiple versions of gtest in tree, we need to update the current version in tree to 1.7.0.
So it looks like the worst part of this is going to be that that the INSTANTIATE_TEST_CASE_P macro has been changed to accept an "optional" last argument like this:

#define INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator, ...)

whereas before it was:

# define INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator)

Unfortunately, leaving the variable arguments empty is not legal C++ and gcc will issue errors and warnings, so you can't make a parameter optional in the way they are trying. I was able to disable the warning I saw on my linux box using -Wno-gnu-zero-variadic-macro-arguments, but I'm getting build errors on Try with linux32 due to the use of "-pedantic-error" while building test files.

I guess one solution would be to disable -pedantic-error just for the affected tests. Another would be to change the macro back to the way it was. Neither solution makes me all that happy.

Limited try run here: https://treeherder.mozilla.org/#/jobs?repo=try&author=dminor@mozilla.com&selectedJob=81223936

The tests also build and run on my local machine.
Actually, looks like I might need to set -no-pedantic-errors in just one place, and there's already a precedent for disabling errors/warnings due to gtest there, so maybe this isn't so bad after all.
Comment on attachment 8843968 [details]
Bug 1343557 - Update gtest and gmock to 1.7.0; .mielczarek

https://reviewboard.mozilla.org/r/117554/#review119244
Attachment #8843968 - Flags: review?(ted) → review+
Comment on attachment 8843969 [details]
Bug 1343557 - Update build for gtest and gmock 1.7.0; .mielczarek

https://reviewboard.mozilla.org/r/117556/#review119246

Nice! I remember running into issues when I was changing the gyp_reader code due to having the WebRTC copy of gtest + the testing/gtest copy. Building fewer copies of gtest definitely sounds nice. :)
Attachment #8843969 - Flags: review?(ted) → review+
Comment on attachment 8843971 [details]
Bug 1343557 - Make operator AutoCreateAndDestroyReentrantMonitor::ReentrantMonitor* const;

https://reviewboard.mozilla.org/r/117560/#review119362
Attachment #8843971 - Flags: review?(benjamin) → review+
Comment on attachment 8843972 [details]
Bug 1343557 - Disable -pedantic-errors for pkix gtests;

https://reviewboard.mozilla.org/r/117562/#review119762

Ok, this seems reasonable.
Attachment #8843972 - Flags: review?(dkeeler) → review+
Comment on attachment 8843970 [details]
Bug 1343557 - Make PfxInstr::operator== const;

https://reviewboard.mozilla.org/r/117558/#review120496
Attachment #8843970 - Flags: review?(jseward) → review+
Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6f0451a83b92
Update gtest and gmock to 1.7.0; r=ted.mielczarek
https://hg.mozilla.org/integration/autoland/rev/b5daeb2d408d
Update build for gtest and gmock 1.7.0; r=ted.mielczarek
https://hg.mozilla.org/integration/autoland/rev/37b0f7cc68d5
Make PfxInstr::operator== const; r=jseward
https://hg.mozilla.org/integration/autoland/rev/66e004568ab2
Make operator AutoCreateAndDestroyReentrantMonitor::ReentrantMonitor* const; r=bsmedberg
https://hg.mozilla.org/integration/autoland/rev/bfd89f8fb93a
Disable -pedantic-errors for pkix gtests; r=keeler
Depends on: 1346305
You need to log in before you can comment on or make changes to this bug.