Proposal to upgrade googletest (gtest+gmock) to 1.8.0

[Victor Costan]

I recently looked into what it would take to bump googletest to 1.8.0, and I came up with a solution that I'd like feedback on.

If you prefer reading code, here's my draft CL: http://crrev.com/2779193002

The main challenge to the upgrade is that gtest and gmock, which were hosted in separate repositories, are now bundled together in the googletest repository. My solution was to host the repository in third_party/googletest, and create forwarding infrastructure in testing/gmock and testing/gtest.

The forwarding has the downside of adding one more click to the process of chasing some definitions in codesearch or in your favorite IDE. Here are some upsides:

* gtest and gmock's build files moved from the secondary tree to //testing/{gtest,gmock}

* (future opportunity) we can simplify the include paths from testing/gtest/include/gtest/gtest.h to testing/gtest.h

* (future opportunity) we have the option to include all the gtest / gmock internal files directly in our forwarding headers, so we can avoid adding gtest and gmock's include directories to the include path for all our tests

In case you're wondering, I first tried symlinking testing/{gtest, gmock} to the right places in third_party/googletest. I was able to build everything locally, but "git cl upload" didn't like the symlinks.

Is this a reasonable approach? Is there a better way?

Thank you,

    Victor

[Nico Weber]

Cool, it'd be great to get a newer gtest. https://bugs.chromium.org/p/chromium/issues/detail?id=630705 has quite a bit of discussion on a prior attempt. I think we should DEPS in gtest from the git repo mentioned in that bug (auto-mirrored from github) instead of adding the files. I think mark@ tried to do this a while ago too, but I couldn't find notes on that.

(I'm hoping to sync up google-internal gtest and github gtest since I need a patch I landed internally, and syncing up to 1.8.0 would make that a lot easier.)

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAP_mGKqnYur_5V7x3hgkfvL_ZtVE--BCFcEh%2Bi%3DkhKTd0%2BE%3DSA%40mail.gmail.com.

[Mark Mentovai]

The last time I looked at this, it resulted in https://codereview.chromium.org/1466413002/. The “sedifchange.sh” that I mention in the notes there does what it sounds like it does. I can provide a copy if needed.

The last time we talked about updating gtest on this list (or maybe just a recent-ish time) was https://groups.google.com/a/chromium.org/d/topic/chromium-dev/CgVbE6tM1qE.

On Tue, Apr 4, 2017 at 10:02 PM, Nico Weber <tha...@chromium.org> wrote:

Cool, it'd be great to get a newer gtest. https://bugs.chromium.org/p/chromium/issues/detail?id=630705 has quite a bit of discussion on a prior attempt. I think we should DEPS in gtest from the git repo mentioned in that bug (auto-mirrored from github) instead of adding the files. I think mark@ tried to do this a while ago too, but I couldn't find notes on that.

(I'm hoping to sync up google-internal gtest and github gtest since I need a patch I landed internally, and syncing up to 1.8.0 would make that a lot easier.)

On Tue, Apr 4, 2017 at 9:37 PM, Victor Costan <pwn...@chromium.org> wrote:

I recently looked into what it would take to bump googletest to 1.8.0, and I came up with a solution that I'd like feedback on.

If you prefer reading code, here's my draft CL: http://crrev.com/2779193002

The main challenge to the upgrade is that gtest and gmock, which were hosted in separate repositories, are now bundled together in the googletest repository. My solution was to host the repository in third_party/googletest, and create forwarding infrastructure in testing/gmock and testing/gtest.

The forwarding has the downside of adding one more click to the process of chasing some definitions in codesearch or in your favorite IDE. Here are some upsides:

* gtest and gmock's build files moved from the secondary tree to //testing/{gtest,gmock}

* (future opportunity) we can simplify the include paths from testing/gtest/include/gtest/gtest.h to testing/gtest.h

In the thread I linked to, Brett said that some of our tools don’t work well if the include paths aren’t relative to src.

We also talked about making gtest live in third_party instead of testing.

* (future opportunity) we have the option to include all the gtest / gmock internal files directly in our forwarding headers, so we can avoid adding gtest and gmock's include directories to the include path for all our tests

In case you're wondering, I first tried symlinking testing/{gtest, gmock} to the right places in third_party/googletest. I was able to build everything locally, but "git cl upload" didn't like the symlinks.

Is this a reasonable approach? Is there a better way?

Symlinking won’t work well with our tools. Forwarding headers are a good temporary solution. A big sed should be able to finish the job.

[Victor Costan]

On Tue, Apr 4, 2017 at 7:12 PM, Mark Mentovai <ma...@chromium.org> wrote:

The last time I looked at this, it resulted in https://codereview.chromium.org/1466413002/. The “sedifchange.sh” that I mention in the notes there does what it sounds like it does. I can provide a copy if needed.

The last time we talked about updating gtest on this list (or maybe just a recent-ish time) was https://groups.google.com/a/chromium.org/d/topic/chromium-dev/CgVbE6tM1qE.

On Tue, Apr 4, 2017 at 10:02 PM, Nico Weber <tha...@chromium.org> wrote:

Cool, it'd be great to get a newer gtest. https://bugs.chromium.org/p/chromium/issues/detail?id=630705 has quite a bit of discussion on a prior attempt. I think we should DEPS in gtest from the git repo mentioned in that bug (auto-mirrored from github) instead of adding the files. I think mark@ tried to do this a while ago too, but I couldn't find notes on that.

(I'm hoping to sync up google-internal gtest and github gtest since I need a patch I landed internally, and syncing up to 1.8.0 would make that a lot easier.)

On Tue, Apr 4, 2017 at 9:37 PM, Victor Costan <pwn...@chromium.org> wrote:

I recently looked into what it would take to bump googletest to 1.8.0, and I came up with a solution that I'd like feedback on.

If you prefer reading code, here's my draft CL: http://crrev.com/2779193002

The main challenge to the upgrade is that gtest and gmock, which were hosted in separate repositories, are now bundled together in the googletest repository. My solution was to host the repository in third_party/googletest, and create forwarding infrastructure in testing/gmock and testing/gtest.

The forwarding has the downside of adding one more click to the process of chasing some definitions in codesearch or in your favorite IDE. Here are some upsides:

* gtest and gmock's build files moved from the secondary tree to //testing/{gtest,gmock}

* (future opportunity) we can simplify the include paths from testing/gtest/include/gtest/gtest.h to testing/gtest.h

In the thread I linked to, Brett said that some of our tools don’t work well if the include paths aren’t relative to src.

We also talked about making gtest live in third_party instead of testing.

Sweet! The fact that smart people have thought of this makes me hope that it's not a terribly unreasonable approach.

 

* (future opportunity) we have the option to include all the gtest / gmock internal files directly in our forwarding headers, so we can avoid adding gtest and gmock's include directories to the include path for all our tests

In case you're wondering, I first tried symlinking testing/{gtest, gmock} to the right places in third_party/googletest. I was able to build everything locally, but "git cl upload" didn't like the symlinks.

Is this a reasonable approach? Is there a better way?

Symlinking won’t work well with our tools. Forwarding headers are a good temporary solution. A big sed should be able to finish the job.

Thank you for confirming my hunch :)

    Victor

[Victor Costan]

On Tue, Apr 4, 2017 at 7:02 PM, Nico Weber <tha...@chromium.org> wrote:

Cool, it'd be great to get a newer gtest. https://bugs.chromium.org/p/chromium/issues/detail?id=630705 has quite a bit of discussion on a prior attempt. I think we should DEPS in gtest from the git repo mentioned in that bug (auto-mirrored from github) instead of adding the files. I think mark@ tried to do this a while ago too, but I couldn't find notes on that.

(I'm hoping to sync up google-internal gtest and github gtest since I need a patch I landed internally, and syncing up to 1.8.0 would make that a lot easier.)

Thank you very much for pointing me to the bug! I will continue the discussion there.

     Victor