New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 630705 link

Starred by 12 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Roll testing/gtest and testing/gmock

Project Member Reported by gab@chromium.org, Jul 22 2016

Issue description

So... I'd like to roll gtest because I'm about to make a change upstream that I want in Chromium... (ref. https://codereview.chromium.org/2162053006/)

But they both haven't been rolled since a year ago (July 28th 2015 is the last commit).

Two major things to consider:
 1) As of https://github.com/google/googletest/commit/1f87a0970dc0880c954ea4066fd7b72fad6fb089 the entire repo moved one level down into googletest/
  i.e. "testing/gtest/include/gtest/gtest.h" would now be "testing/gtest/googletest/include/gtest/gtest.h"

 2) gmock also moved to googlemock/ in the root of this repo (they've coalesced in the same repo as explained here https://github.com/google/googletest)
  (the current repo is still on SVN and is either a mirror or is dead -- haven't checked)

Proposal:
 A) Our existing gtest.gyp and gmock.gyp already specify an include_dirs to "include/gtest" and "include/gmock" so that it's already okay in Chromium to do #include "gtest/gtest.h" instead of "testing/gtest/include/gtest/gtest.h"
   => Add a presubmit against the full path and force people to use #include "gtest/gtest.h" and "gmock/gmock.h" 
 B) Mass rename includes in existing codebase.
 C) Move DEPS on gtest and update gtest include_dirs to "googletest/include/gtest" (hopefully everything else still works..!)
 D) Delete gmock and add "googlemock/include/gmock" to gtest.gyp's include_dirs (effectively "moving" DEPS on gmock).

I'll go ahead and prepare a CL for the PRESUBMIT in (A) -- the other option is to have (B) instead update all the full paths to the new full paths but that'll be harder because (a) we'll need to change over 6500 files in lock step with rolling DEPS :-O and (b) it means we could have to do this over again if the full path somehow changed again...!
 

Comment 1 by gab@chromium.org, Jul 22 2016

Discussion happened on chromium-dev [1], brettw/jam prefer full include paths, jam proposed to replace (A) with : add "src/testing" as a global include directory, and then put a forwarding header in src/testing/testing/gtest/include/gtest/gtest.h temporarily

I'll go with that.

[1] https://groups.google.com/a/chromium.org/d/topic/chromium-dev/CgVbE6tM1qE/discussion

Comment 2 by gab@chromium.org, Jul 22 2016

Summary: Roll testing/gtest and testing/gmock (was: Roll testing/gtest and testing/gmock (and change include paradigm for all gtest/gmock headers))
Cc: mark@chromium.org brettw@chromium.org jam@chromium.org
So, gmock moves into the github.com/googletest.git, and what was former at the top of the googletest.git repo is now in the googletest/ directory of googletest.git, right?

So we'll have in DEPS:

"src/testing/gtest": ".../googletest.git@XYZ"

the includes will be at:

//testing/gtest/googletest/include/gtest/gtest.h
//testing/gtest/googlemock/include/gmock/gmock.h

and currently our code uses:

#include "testing/gtest/include/gtest/gtest.h"
#include "testing/gmock/include/gmock/gmock.h"

and the proposal is to change the code to:

#include "testing/gtest/googletest/include/gtest.h"
#include "testing/gtest/googlemock/include/gmock.h"

and add forwarding headers to get us through the migration, right?

If we have to update everything, does it make sense to move the repo
to src/third_party/googletest to follow the conventions? It does
make the includes 10 characters longer than the proposal and 20
chars longer than today:

#include "third_party/googletest/googletest/include/gtest/gtest.h"

Comment 4 by brettw@chromium.org, Jul 22 2016

I like the third_party suggestion.

The only reason they're in src/testing was to mirror the structure of the internal Google repository. We're pretty far away from that at this point.

Our rules for repos like this say they should be in third_party. If we're messing with all of the include paths, it seems nice that this might follow the same third_party rules.

The path "third_party/googletest/googletest/include/gtest/gtest.h" is indeed unfortunate. Since we mirror this anyway, can we make the mirror such that we don't have the duplicate "googletest" in there?
> can we make the mirror such that we don't have the duplicate "googletest" in there?

It looks like they merged gmock and gtest into a single repo, so the top level directory of the repo now has "googletest" and "googlemock" subdirectories.

We might be able to mirror just the subtrees of the repos, like we do with //build, //base, //net, etc., but that's probably more trouble than it's worth.

Comment 6 by gab@chromium.org, Jul 25 2016

Cc: gab@chromium.org
Owner: amistry@chromium.org
@amistry who happens to have a CL in progress to do this (though it currently uses a different approach) : https://codereview.chromium.org/2169193002/

Copying from my reply on chromium-dev:

So it seems like what he's doing is:
1) Move mapping of DEPS to testing/third_party/googletest.
2) Add redirect headers in place of the old ones (under testing/gtest/include/* which is no longer controlled by DEPS).

Does that work as an incremental patch? (gclient knows to delete the old DEPS before syncing the new files in the same place?) I'm worried that people will end up with multiple copies of the same files (typically when DEPS are moved, the old path is at least removed from .gitignore so that if you're not using gclient sync -D you see cruft behind and can manually clean it. I feel like I've seen re-usage of an old DEPS path cause issues in the past...

Also, this feels pretty permanent (and even hard to undo if we decide we don't like it per having to unroll the steps I'm not sure will even roll forward properly above..), do you not intend to change the include paths over the codebase?
If not this feels like it semi goes against the discussion here of keeping full include paths (they're still full paths but to permanent redirect headers..)

Lastly: googletest now includes gmock, so we should probably also strip gmock.

In that light, I prefer the version proposed by John (keeping the DEPS in place, add temporary testing/testing/gtest/include/* redirect headers, do a massive search/replace for existing includes, remove redirect headers). And, bonus, this also allows to replace SVN gmock DEPS in one swoop.

Or even, move DEPS to third_party/ and have testing/testing/* redirect headers for the migration phase as proposed above?

Comment 7 by bauerb@chromium.org, Jul 25 2016

Cc: bauerb@chromium.org aga...@chromium.org
 Issue 584570  has been merged into this issue.

Comment 8 by brettw@chromium.org, Jul 25 2016

I think src/third_party is better than testing/third_party. As I said above, the only reason that src/testing exists was to hold gtest and such.

I would slightly prefer to move to src/third_party, but I'm also OK keeping in place if that's simpler.

Comment 9 by jam@chromium.org, Jul 25 2016

It's more consistent with the other DEPS'd directories to have them be in src/third_party/, so I vote for that.

Comment 10 by gab@chromium.org, Jul 29 2016

ping amistry: how does this sound? Do you still intend to go through with rolling gtest?

Comment 11 by jam@chromium.org, Jul 29 2016

Owner: gab@chromium.org
amistry left Google a week ago. So assigning to gab instead..
Sorry, I can't do this anymore. No z620 or goma => no ability to build in a reasonable time frame.

Comment 13 by gab@chromium.org, Aug 9 2016

Owner: phajdan.jr@chromium.org
Status: Assigned (was: Started)
I do not actively need gtest DEPS to be rolled anymore, I really think this should be done as being a year behind on DEPS has slowed us down (as shown by 3+ people attempting to roll DEPS in the last year).

I think the plan to move gtest to third_party/ and introduce temporary redirect headers in testing/testing/... with an include rule for testing/testing so that existing includes still work (while being mass redirected) is sound.

I think this should be done by testing/OWNERS.

Here's my WIP if it helps: https://codereview.chromium.org/2224323003/ (still missing gmock redirect headers and probably more things -- ran out of time while writing it and haven't tried it...).

Comment 14 by gab@chromium.org, Dec 7 2016

Our DEPS for gtest are now ~1.5 years behind.

@phajdan.jr: can you find an owner for this if not yourself? This should be much easier now that the build is GN only.
Cc: phajdan@google.com
Owner: ----
Status: Available (was: Assigned)
Cc: pwnall@chromium.org
I would like to help out with the work (assuming it can be done in small steps), because my team has bumped into this as well. I was pointed to this bug from chromium-dev, and I think that the approach I proposed there happens to match the comments above pretty well.

I uploaded the following two CLs as a starting point. The Chromium-side CL moves googletest to third_party and introduces forwarding headers. The v8-side CL silences some "gn check" errors that show up because testing/gtest and testing/gmock are now in the repository. If this looks like a reasonable start, I can take ownership of the bug and try to run it through. Thoughts / guidance?

Chromium: http://crrev.com/2779193002
v8: http://crrev.com/2807353002
One thing I'd check is how the #include lines for gtest look like internal to google, and try to come up with a setup where ours look the same (if that's feasible; maybe it isn't -- but maybe it's just about picking the right folder name and layout below third_party). But generally that approach looks good, thanks for working on this!
Owner: pwnall@chromium.org
Status: Started (was: Available)
The google3 discussion will happen off this bug, at least initially. Apologies to external contributors. I'll report back with concrete plans.
The conclusion was that we will proceed as planned. We'll consider Google Test's file/directory layout to be in flux, and we'll keep forwarding files in Chromium's tree until Google Test stabilizes.

While reviewing the V8 CL above, jochen@ taught me that PDFium and V8 rely on Chromium's //build/secondary/{gtest,gmock}, and should be updated at roughly the same time.

V8-side CL: http://crrev.com/2847693002
Project Member

Comment 20 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b539fa1123a19e6499f58582da55ca5af9df94a4

commit b539fa1123a19e6499f58582da55ca5af9df94a4
Author: pwnall <pwnall@chromium.org>
Date: Fri Apr 28 02:07:31 2017

Roll googletest to 1.8.0.

GoogleTest (gtest) and GoogleMock (gmock) are now hosted into the same
googletest repository. In order to cope with this, the googletest
repository is now sourced at third_party/googletest.

The file/directory layout of Google Test is not yet considered stable.
To minimize disruption while Google Test stabilizes, Chromium code will
be insulated from third_party/googletest.

* testing/gtest/include/gtest/ and testing/gmock/include/gmock have
  been populated with headers that forward into the appropriate locations of
  third_party/googletest

* testing/BUILD.gn has been populated with the targets
  //testing/gtest(:gtest_main) and //testing/gmock(:gmock_main), which
  depend on the appropriate //third_party/googletest targets.

All Chromium code should keep depending on the targets and headers
in testing/{gtest,gmock} for now.

BUG= 630705 
TBR=rkc

Review-Url: https://codereview.chromium.org/2779193002
Cr-Commit-Position: refs/heads/master@{#467833}

[modify] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/.gitignore
[modify] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/DEPS
[modify] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/base/gtest_prod_util.h
[modify] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/build/secondary/testing/gmock/BUILD.gn
[modify] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/build/secondary/testing/gtest/BUILD.gn
[modify] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/device/bluetooth/dbus/fake_bluetooth_media_endpoint_service_provider.h
[modify] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/net/socket/sequenced_socket_data_unittest.cc
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gmock/BUILD.gn
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gmock/OWNERS
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gmock/include/gmock/gmock-actions.h
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gmock/include/gmock/gmock-generated-function-mockers.h
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gmock/include/gmock/gmock-matchers.h
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gmock/include/gmock/gmock.h
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gtest/BUILD.gn
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gtest/OWNERS
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gtest/include/gtest/gtest-death-test.h
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gtest/include/gtest/gtest-message.h
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gtest/include/gtest/gtest-param-test.h
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gtest/include/gtest/gtest-spi.h
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gtest/include/gtest/gtest.h
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gtest/include/gtest/gtest_prod.h
[modify] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gtest_mac.h
[modify] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gtest_mac.mm
[modify] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/testing/gtest_mac_unittest.mm
[modify] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/third_party/.gitignore
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/third_party/googletest/BUILD.gn
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/third_party/googletest/OWNERS
[add] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/third_party/googletest/README.chromium
[rename] https://crrev.com/b539fa1123a19e6499f58582da55ca5af9df94a4/third_party/googletest/gmock_custom/gmock/internal/custom/gmock-port.h

Project Member

Comment 21 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed7b62615d138c4d629540bab4a2757da5551134

commit ed7b62615d138c4d629540bab4a2757da5551134
Author: findit-for-me <findit-for-me@appspot.gserviceaccount.com>
Date: Fri Apr 28 02:49:24 2017

Revert of Roll googletest to 1.8.0. (patchset #7 id:340001 of https://codereview.chromium.org/2779193002/ )

Reason for revert:

Findit(https://goo.gl/kROfz5) identified CL at revision 467833 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2I1MzlmYTExMjNhMTllNjQ5OWY1ODU4MmRhNTVjYTVhZjlkZjk0YTQM

Original issue's description:
> Roll googletest to 1.8.0.
>
> GoogleTest (gtest) and GoogleMock (gmock) are now hosted into the same
> googletest repository. In order to cope with this, the googletest
> repository is now sourced at third_party/googletest.
>
> The file/directory layout of Google Test is not yet considered stable.
> To minimize disruption while Google Test stabilizes, Chromium code will
> be insulated from third_party/googletest.
>
> * testing/gtest/include/gtest/ and testing/gmock/include/gmock have
>   been populated with headers that forward into the appropriate locations of
>   third_party/googletest
>
> * testing/BUILD.gn has been populated with the targets
>   //testing/gtest(:gtest_main) and //testing/gmock(:gmock_main), which
>   depend on the appropriate //third_party/googletest targets.
>
> All Chromium code should keep depending on the targets and headers
> in testing/{gtest,gmock} for now.
>
> BUG= 630705 
> TBR=rkc
>
> Review-Url: https://codereview.chromium.org/2779193002
> Cr-Commit-Position: refs/heads/master@{#467833}
> Committed: https://chromium.googlesource.com/chromium/src/+/b539fa1123a19e6499f58582da55ca5af9df94a4

TBR=thakis@chromium.org,dpranke@chromium.org,agl@chromium.org,rkc@chromium.org,skobes@chromium.org,pwnall@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 630705 

Review-Url: https://codereview.chromium.org/2847043002
Cr-Commit-Position: refs/heads/master@{#467852}

[modify] https://crrev.com/ed7b62615d138c4d629540bab4a2757da5551134/.gitignore
[modify] https://crrev.com/ed7b62615d138c4d629540bab4a2757da5551134/DEPS
[modify] https://crrev.com/ed7b62615d138c4d629540bab4a2757da5551134/base/gtest_prod_util.h
[modify] https://crrev.com/ed7b62615d138c4d629540bab4a2757da5551134/build/secondary/testing/gmock/BUILD.gn
[modify] https://crrev.com/ed7b62615d138c4d629540bab4a2757da5551134/build/secondary/testing/gtest/BUILD.gn
[modify] https://crrev.com/ed7b62615d138c4d629540bab4a2757da5551134/device/bluetooth/dbus/fake_bluetooth_media_endpoint_service_provider.h
[modify] https://crrev.com/ed7b62615d138c4d629540bab4a2757da5551134/net/socket/sequenced_socket_data_unittest.cc
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gmock/BUILD.gn
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gmock/OWNERS
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gmock/include/gmock/gmock-actions.h
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gmock/include/gmock/gmock-generated-function-mockers.h
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gmock/include/gmock/gmock-matchers.h
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gmock/include/gmock/gmock.h
[rename] https://crrev.com/ed7b62615d138c4d629540bab4a2757da5551134/testing/gmock_custom/gmock/internal/custom/gmock-port.h
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gtest/BUILD.gn
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gtest/OWNERS
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gtest/include/gtest/gtest-death-test.h
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gtest/include/gtest/gtest-message.h
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gtest/include/gtest/gtest-param-test.h
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gtest/include/gtest/gtest-spi.h
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gtest/include/gtest/gtest.h
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/testing/gtest/include/gtest/gtest_prod.h
[modify] https://crrev.com/ed7b62615d138c4d629540bab4a2757da5551134/testing/gtest_mac.h
[modify] https://crrev.com/ed7b62615d138c4d629540bab4a2757da5551134/testing/gtest_mac.mm
[modify] https://crrev.com/ed7b62615d138c4d629540bab4a2757da5551134/testing/gtest_mac_unittest.mm
[modify] https://crrev.com/ed7b62615d138c4d629540bab4a2757da5551134/third_party/.gitignore
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/third_party/googletest/BUILD.gn
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/third_party/googletest/OWNERS
[delete] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/third_party/googletest/README.chromium

Project Member

Comment 22 by bugdroid1@chromium.org, May 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da

commit dbcef9bdc1edd32d62c6de0bd382cdc78ce385da
Author: pwnall <pwnall@chromium.org>
Date: Wed May 03 01:50:21 2017

Roll googletest to 1.8.0.

This is a re-land of http://crrev.com/2779193002 which was reverted due to
build errors on Mac.

GoogleTest (gtest) and GoogleMock (gmock) are now hosted into the same
googletest repository. In order to cope with this, the googletest
repository is now sourced at third_party/googletest.

The file/directory layout of Google Test is not yet considered stable.
To minimize disruption while Google Test stabilizes, Chromium code will
be insulated from third_party/googletest.

* testing/gtest/include/gtest/ and testing/gmock/include/gmock have
  been populated with headers that forward into the appropriate locations of
  third_party/googletest

* testing/BUILD.gn has been populated with the targets
  //testing/gtest(:gtest_main) and //testing/gmock(:gmock_main), which
  depend on the appropriate //third_party/googletest targets.

All Chromium code should keep depending on the targets and headers
in testing/{gtest,gmock} for now.

BUG= 630705 
TESTED=ninja -C out/Default/ ced_unittests && ninja -C out/Default
TBR=rkc,dpranke,agl

Review-Url: https://codereview.chromium.org/2852613002
Cr-Commit-Position: refs/heads/master@{#468860}

[modify] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/.gitignore
[modify] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/DEPS
[modify] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/base/gtest_prod_util.h
[modify] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/build/secondary/testing/gmock/BUILD.gn
[modify] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/build/secondary/testing/gtest/BUILD.gn
[modify] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/device/bluetooth/dbus/fake_bluetooth_media_endpoint_service_provider.h
[modify] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/net/socket/sequenced_socket_data_unittest.cc
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gmock/BUILD.gn
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gmock/OWNERS
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gmock/include/gmock/gmock-actions.h
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gmock/include/gmock/gmock-generated-function-mockers.h
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gmock/include/gmock/gmock-matchers.h
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gmock/include/gmock/gmock.h
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gtest/BUILD.gn
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gtest/OWNERS
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gtest/empty.cc
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gtest/include/gtest/gtest-death-test.h
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gtest/include/gtest/gtest-message.h
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gtest/include/gtest/gtest-param-test.h
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gtest/include/gtest/gtest-spi.h
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gtest/include/gtest/gtest.h
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gtest/include/gtest/gtest_prod.h
[modify] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gtest_mac.h
[modify] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gtest_mac.mm
[modify] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/testing/gtest_mac_unittest.mm
[modify] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/third_party/.gitignore
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/third_party/googletest/BUILD.gn
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/third_party/googletest/OWNERS
[add] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/third_party/googletest/README.chromium
[rename] https://crrev.com/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da/third_party/googletest/gmock_custom/gmock/internal/custom/gmock-port.h

The Chromium-side upgrade seems to have stuck. \o/

Next up: V8 and PDFium.
Project Member

Comment 24 by bugdroid1@chromium.org, May 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a289707ca3726217915fb7cac34dcdeeaba1e3cc

commit a289707ca3726217915fb7cac34dcdeeaba1e3cc
Author: msramek <msramek@chromium.org>
Date: Wed May 03 10:55:36 2017

Revert of Roll googletest to 1.8.0. (patchset #3 id:40001 of https://codereview.chromium.org/2852613002/ )

Reason for revert:
Seems to have caused all content_browsertests on ChromiumOS to crash.

https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/25582

I'll run the revert through trybots so that it doesn't break anything else.

Original issue's description:
> Roll googletest to 1.8.0.
>
> This is a re-land of http://crrev.com/2779193002 which was reverted due to
> build errors on Mac.
>
> GoogleTest (gtest) and GoogleMock (gmock) are now hosted into the same
> googletest repository. In order to cope with this, the googletest
> repository is now sourced at third_party/googletest.
>
> The file/directory layout of Google Test is not yet considered stable.
> To minimize disruption while Google Test stabilizes, Chromium code will
> be insulated from third_party/googletest.
>
> * testing/gtest/include/gtest/ and testing/gmock/include/gmock have
>   been populated with headers that forward into the appropriate locations of
>   third_party/googletest
>
> * testing/BUILD.gn has been populated with the targets
>   //testing/gtest(:gtest_main) and //testing/gmock(:gmock_main), which
>   depend on the appropriate //third_party/googletest targets.
>
> All Chromium code should keep depending on the targets and headers
> in testing/{gtest,gmock} for now.
>
> BUG= 630705 
> TESTED=ninja -C out/Default/ ced_unittests && ninja -C out/Default
> TBR=rkc,dpranke,agl
>
> Review-Url: https://codereview.chromium.org/2852613002
> Cr-Commit-Position: refs/heads/master@{#468860}
> Committed: https://chromium.googlesource.com/chromium/src/+/dbcef9bdc1edd32d62c6de0bd382cdc78ce385da

TBR=dpranke@chromium.org,thakis@chromium.org,rkc@chromium.org,agl@chromium.org,pwnall@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 630705 

Review-Url: https://codereview.chromium.org/2856963003
Cr-Commit-Position: refs/heads/master@{#468934}

[modify] https://crrev.com/a289707ca3726217915fb7cac34dcdeeaba1e3cc/.gitignore
[modify] https://crrev.com/a289707ca3726217915fb7cac34dcdeeaba1e3cc/DEPS
[modify] https://crrev.com/a289707ca3726217915fb7cac34dcdeeaba1e3cc/base/gtest_prod_util.h
[modify] https://crrev.com/a289707ca3726217915fb7cac34dcdeeaba1e3cc/build/secondary/testing/gmock/BUILD.gn
[modify] https://crrev.com/a289707ca3726217915fb7cac34dcdeeaba1e3cc/build/secondary/testing/gtest/BUILD.gn
[modify] https://crrev.com/a289707ca3726217915fb7cac34dcdeeaba1e3cc/device/bluetooth/dbus/fake_bluetooth_media_endpoint_service_provider.h
[modify] https://crrev.com/a289707ca3726217915fb7cac34dcdeeaba1e3cc/net/socket/sequenced_socket_data_unittest.cc
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gmock/BUILD.gn
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gmock/OWNERS
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gmock/include/gmock/gmock-actions.h
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gmock/include/gmock/gmock-generated-function-mockers.h
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gmock/include/gmock/gmock-matchers.h
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gmock/include/gmock/gmock.h
[rename] https://crrev.com/a289707ca3726217915fb7cac34dcdeeaba1e3cc/testing/gmock_custom/gmock/internal/custom/gmock-port.h
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gtest/BUILD.gn
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gtest/OWNERS
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gtest/empty.cc
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gtest/include/gtest/gtest-death-test.h
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gtest/include/gtest/gtest-message.h
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gtest/include/gtest/gtest-param-test.h
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gtest/include/gtest/gtest-spi.h
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gtest/include/gtest/gtest.h
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/testing/gtest/include/gtest/gtest_prod.h
[modify] https://crrev.com/a289707ca3726217915fb7cac34dcdeeaba1e3cc/testing/gtest_mac.h
[modify] https://crrev.com/a289707ca3726217915fb7cac34dcdeeaba1e3cc/testing/gtest_mac.mm
[modify] https://crrev.com/a289707ca3726217915fb7cac34dcdeeaba1e3cc/testing/gtest_mac_unittest.mm
[modify] https://crrev.com/a289707ca3726217915fb7cac34dcdeeaba1e3cc/third_party/.gitignore
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/third_party/googletest/BUILD.gn
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/third_party/googletest/OWNERS
[delete] https://crrev.com/8758b479e0b41c9311ded11ef2a2d4ffd4d59d4c/third_party/googletest/README.chromium

Project Member

Comment 25 by bugdroid1@chromium.org, May 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/8acca5a336e00eab761363dfdaa081b7cdce3883

commit 8acca5a336e00eab761363dfdaa081b7cdce3883
Author: Michael Achenbach <machenbach@chromium.org>
Date: Wed May 03 12:01:25 2017

V8: Skip auto-rolling gtest depedency

TBR=pwnall@chromium.org
Bug:  chromium:630705 

Change-Id: I02b5bb3854e188f65e78c490f71fa9608251766e
Reviewed-on: https://chromium-review.googlesource.com/494488
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/8acca5a336e00eab761363dfdaa081b7cdce3883/scripts/slave/recipes/v8/auto_roll_v8_deps.py

Cc: kjellander@chromium.org ehmaldonado@chromium.org
I was able to reproduce the crash causing the last revert. content_browsertests fails on plain Linux (no ChromeOS-flavored build required) when run in a non-release build. 

The crash can be caused by one of the following:
1) The BUILD.gn changes.
2) Changes in gtest/gmock.

Assuming my local reproduction matches the failure on the build bot, I was able to rule out the BUILD.gn changes. It appears that the crash was introduced somewhere between gtest/gmock 1.7.0 (after the gtest/gmock) and gtest/gmock 1.8.0. I'm currently verifying my testing on the trybots in patch sets 3-5 of http://crrev.com/2856383002

If this turns out to be gtest/gmock-related, I suggest that we roll googletest to the first commit that includes both projects, so we can switch to the new directory structure. Rolling googletest forward should be less painful after the switch, so it'll be easier to debug/fix the issue behind the crash.
A local manual bisect points to commit bb5c92f9d1f0b26c79978e92f38a0e5fbcc8c9bf -- https://github.com/google/googletest/commit/bb5c92f9d1f0b26c79978e92f38a0e5fbcc8c9bf

The issue seems to be that changing GTEST_API interacts negatively with g_gmock_implicit_sequence at  https://cs.chromium.org/chromium/src/testing/gmock/src/gmock-spec-builders.cc?q=GTEST_API+ThreadLocal+file:src/testing/+package:%5Echromium$&l=243&dr=C

Removing GTEST_API_ (which is an alias for GTEST_API) makes content_browsertests not crash on bb5c92f9d1f0b26c79978e92f38a0e5fbcc8c9bf (the first bad commit), ec44c6c1675c25b9827aacd08c02433cccde7780 (the 1.8.0 release), and 0ad83afdaa3319dadd9f03299bd62f93e6afe2d3 (the current master).
I was wrong, there's no GTEST_API. The comment above still stands with s/GTEST_API/GTEST_API_/ though.
I created https://github.com/google/googletest/pull/1078 which would give us a way to roll Google Test (head) without the GTEST_API_ change that's crashing content_browsertests.
Rolling to up to the breaking commit sounds like a plan.
Project Member

Comment 32 by bugdroid1@chromium.org, May 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f

commit 52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f
Author: pwnall <pwnall@chromium.org>
Date: Fri May 05 00:44:27 2017

Roll googletest to 1.7.0+.

This is a (less ambitious) re-land of http://crrev.com/2852613002 which
was reverted due to content_browsertests crashes on ChromeOS. That CL is
a re-land of http://crrev.com/2779193002 which was reverted due to build
errors on Mac.

Instead of rolling Google Test to 1.8.0, this CL uses the last commit
before a breaking change was introduced. Details about the breakage are
in  https://crbug.com/630705#c27  and  https://crbug.com/630705#c28 . It is
expected that rolling Google Test from there to the next usable version
will be much less disruptive.

GoogleTest (gtest) and GoogleMock (gmock) are now hosted into the same
googletest repository. In order to cope with this, the googletest
repository is now sourced at third_party/googletest.

The file/directory layout of Google Test is not yet considered stable.
To minimize disruption while Google Test stabilizes, Chromium code will
be insulated from third_party/googletest.

* testing/gtest/include/gtest/ and testing/gmock/include/gmock have
  been populated with headers that forward into the appropriate locations of
  third_party/googletest

* testing/BUILD.gn has been populated with the targets
  //testing/gtest(:gtest_main) and //testing/gmock(:gmock_main), which
  depend on the appropriate //third_party/googletest targets.

All Chromium code should keep depending on the targets and headers
in testing/{gtest,gmock} for now.

BUG= 630705 
TESTED=out/Default/content_browsertests --gtest_filter=MojoTest.Init
TESTED=ninja -C out/Default/ ced_unittests && ninja -C out/Default
TBR=rkc, dpranke

Review-Url: https://codereview.chromium.org/2856383002
Cr-Commit-Position: refs/heads/master@{#469551}

[modify] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/.gitignore
[modify] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/DEPS
[modify] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/base/gtest_prod_util.h
[modify] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/build/secondary/testing/gmock/BUILD.gn
[modify] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/build/secondary/testing/gtest/BUILD.gn
[modify] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/device/bluetooth/dbus/fake_bluetooth_media_endpoint_service_provider.h
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gmock/BUILD.gn
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gmock/OWNERS
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gmock/include/gmock/gmock-actions.h
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gmock/include/gmock/gmock-generated-function-mockers.h
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gmock/include/gmock/gmock-matchers.h
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gmock/include/gmock/gmock.h
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gtest/BUILD.gn
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gtest/OWNERS
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gtest/empty.cc
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gtest/include/gtest/gtest-death-test.h
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gtest/include/gtest/gtest-message.h
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gtest/include/gtest/gtest-param-test.h
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gtest/include/gtest/gtest-spi.h
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gtest/include/gtest/gtest.h
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gtest/include/gtest/gtest_prod.h
[modify] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gtest_mac.h
[modify] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gtest_mac.mm
[modify] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/testing/gtest_mac_unittest.mm
[modify] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/third_party/.gitignore
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/third_party/googletest/BUILD.gn
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/third_party/googletest/OWNERS
[add] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/third_party/googletest/README.chromium
[rename] https://crrev.com/52e27a3d242ac0bbcbdc00683f6489ed4d8e9b3f/third_party/googletest/gmock_custom/gmock/internal/custom/gmock-port.h

BTW, a while ago, I read SetArgumentPointee is deprecated, so I tried to replace it in https://codereview.chromium.org/1302233003/ but the bots failed, I can't remember why it failed, and the bot logs are gone. I've uploaded a fresh patch set and started a try job. Let's see how that works with r469551.
Project Member

Comment 34 by bugdroid1@chromium.org, May 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e147d68dc40b1470b6de6c2d8f9232bd788e897e

commit e147d68dc40b1470b6de6c2d8f9232bd788e897e
Author: thestig <thestig@chromium.org>
Date: Fri May 05 07:31:31 2017

Replace gmock's deprecated SetArgumentPointee with SetArgPointee.

BUG= 630705 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
TBR=blundell@chromium.org,jrummell@chromium.org,kelvinp@chromium.org,piman@chromium.org

Review-Url: https://codereview.chromium.org/1302233003
Cr-Commit-Position: refs/heads/master@{#469611}

[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/base/gmock_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/base/nix/xdg_util_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/chrome/browser/chromeos/attestation/platform_verification_flow_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/chrome/browser/chromeos/customization/customization_document_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_toolbar_view_unittest.mm
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/components/browser_sync/profile_sync_service_autofill_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/components/browser_sync/profile_sync_service_typed_url_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/components/search_engines/search_engine_data_type_controller_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/components/sync/driver/async_directory_type_controller_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/components/sync/driver/frontend_data_type_controller_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/components/sync_bookmarks/bookmark_data_type_controller_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/build_gles2_cmd_buffer.py
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/command_buffer_service_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/command_executor_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/context_group_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/feature_info_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/gles2_cmd_decoder_unittest_1.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/gles2_cmd_decoder_unittest_1_autogen.h
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/gles2_cmd_decoder_unittest_2.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/gles2_cmd_decoder_unittest_3.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/gles2_cmd_decoder_unittest_attribs.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/gles2_cmd_decoder_unittest_context_lost.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/gles2_cmd_decoder_unittest_context_state.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/gles2_cmd_decoder_unittest_drawing.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/gles2_cmd_decoder_unittest_textures.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/query_manager_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/gpu/command_buffer/service/test_helper.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/media/audio/alsa/alsa_output_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/media/audio/cras/cras_unified_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/media/filters/chunk_demuxer_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/media/filters/ffmpeg_glue_unittest.cc
[modify] https://crrev.com/e147d68dc40b1470b6de6c2d8f9232bd788e897e/remoting/protocol/jingle_session_unittest.cc

Did this require any special handling on the builder side? The Chromecast builders are trying to roll in this change and are having issues with:

error: The following untracked working tree files would be overwritten by merge:
	testing/gmock/include/gmock/gmock-actions.h
	testing/gmock/include/gmock/gmock-generated-function-mockers.h
	testing/gmock/include/gmock/gmock-matchers.h
	testing/gmock/include/gmock/gmock.h
	testing/gtest/include/gtest/gtest-death-test.h
	testing/gtest/include/gtest/gtest-message.h
	testing/gtest/include/gtest/gtest-param-test.h
	testing/gtest/include/gtest/gtest-spi.h
	testing/gtest/include/gtest/gtest.h
	testing/gtest/include/gtest/gtest_prod.h
Please move or remove them before you merge.

Because the new testing/* headers are in the same place as the old ones that gclient manages
#35: This went through the bots without any manual handling.

I did have to do some dancing while working on this change with git cl and gclient. I think bot_update uses better magic than I do. Are your bots using the same updating script?
Sadly no, we don't use the same updating scripts. So our current plan is to land the DEPS change by itself (which will break the tests temporarily), clean out the old testing/* headers, and then land the rest of the CL. Not ideal, but shouldn't be too cumbersome.

Comment 38 by nick@chromium.org, May 11 2017

I'm also hitting the error at #35, on my local checkout, when trying to git merge. This is probably going to be a major pain point.

Do we need a 'landmines' or something similar to clobber testing/?
Status: Fixed (was: Started)
Project Member

Comment 41 by bugdroid1@chromium.org, Mar 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/539e907258bf639377483692e0b38694fec753f8

commit 539e907258bf639377483692e0b38694fec753f8
Author: Victor Costan <pwnall@chromium.org>
Date: Fri Mar 23 07:54:27 2018

Roll googletest to 1.8.0+.

This is the V8 equivalent to https://crrev.com/2779193002 and must be landed
before //build/secondary/{gtest,gmock} are removed from Chromium. This started
out as https://crrev.com/2847693002

The changes in tools/ were authored by yangguo@chromium.org and
initially shared in http://crrev.com/2849783003.

GoogleTest (gtest) and GoogleMock (gmock) are now hosted into the same
googletest repository. In order to cope with this, the googletest
repository is now sourced at third_party/googletest.

The file/directory layout of Google Test is not yet considered stable.
To minimize disruption while Google Test stabilizes, Chromium code will
be insulated from third_party/googletest.

* testing/gtest/include/gtest/ and testing/gmock/include/gmock have
  been populated with headers that forward into the appropriate
  locations of third_party/googletest

* testing/BUILD.gn has been populated with the targets
  //testing/gtest(:gtest_main) and //testing/gmock(:gmock_main),
  which depend on the appropriate //third_party/googletest targets.

All Chromium code should keep depending on the targets and
headers in testing/{gtest,gmock} for now.

BUG= chromium:630705 

Change-Id: I12b07ae78c8039aeff6ada7a3335e4e2b5d308ab
Reviewed-on: https://chromium-review.googlesource.com/639953
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52170}
[modify] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/.gitignore
[modify] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/DEPS
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gmock/BUILD.gn
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gmock/OWNERS
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gmock/include/DEPS
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gmock/include/gmock/gmock-actions.h
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gmock/include/gmock/gmock-generated-function-mockers.h
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gmock/include/gmock/gmock-matchers.h
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gmock/include/gmock/gmock.h
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gtest/BUILD.gn
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gtest/OWNERS
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gtest/empty.cc
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gtest/include/DEPS
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gtest/include/gtest/gtest-death-test.h
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gtest/include/gtest/gtest-message.h
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gtest/include/gtest/gtest-param-test.h
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gtest/include/gtest/gtest-spi.h
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gtest/include/gtest/gtest.h
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/testing/gtest/include/gtest/gtest_prod.h
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/third_party/googletest/BUILD.gn
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/third_party/googletest/OWNERS
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/third_party/googletest/README.chromium
[rename] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/third_party/googletest/gmock_custom/gmock/internal/custom/gmock-port.h
[modify] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/tools/gcmole/run-gcmole.isolate
[modify] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/tools/node/test_update_node.py
[modify] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/tools/node/testdata/v8/.gitignore
[modify] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/tools/node/update_node.py
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/tools/release/testdata/v8/third_party/googletest/src/googletest/include/gtest/baz/gtest_new
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/tools/release/testdata/v8/third_party/googletest/src/googletest/include/gtest/gtest_new
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/tools/release/testdata/v8/third_party/googletest/src/googletest/include/gtest/gtest_prod.h
[add] https://crrev.com/539e907258bf639377483692e0b38694fec753f8/tools/release/testdata/v8/third_party/googletest/src/googletest/include/gtest/new/gtest_new

Project Member

Comment 42 by bugdroid1@chromium.org, Aug 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/123a64cbf84a55ca88e6a77c1f84f999e96c55d4

commit 123a64cbf84a55ca88e6a77c1f84f999e96c55d4
Author: Nico Weber <thakis@chromium.org>
Date: Tue Aug 28 20:52:36 2018

Remove build/secondary/{gtest,gmock}/BUILD.gn

These files were still here to give v8 and pdfium time to move to the gtest 1.7
layout. That has since happened, so remove them.

Bug:  630705 
Change-Id: I0e7aceb478fabcd03678cbfc28df70fa641e99d2
Reviewed-on: https://chromium-review.googlesource.com/1193962
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586846}
[delete] https://crrev.com/ae61c89ca72d9231f5af79261fe1a544f3d219f8/build/secondary/testing/gmock/BUILD.gn
[delete] https://crrev.com/ae61c89ca72d9231f5af79261fe1a544f3d219f8/build/secondary/testing/gtest/BUILD.gn

Sign in to add a comment