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

Issue 677544 link

Starred by 1 user
Status: Fixed
Owner:
lukasza@chromium.org
Closed: Feb 2017
Cc:
danakj@chromium.org
dcheng@chromium.org
█ nasko@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 578344



Sign in to add a comment

rewrite_to_chrome_style doesn't rewrite method names used in EXPECT_CALL / in gmock macro invocations

Project Member Reported by lukasza@chromium.org, Dec 29 2016 
For example:
   EXPECT_CALL(mocked_interface, myMethod(123));
should be rewritten to
   EXPECT_CALL(mocked_interface, MyMethod(123));

Comment 1 by lukasza@chromium.org, Dec 29 2016 

The EXPECT_CALL macro looks as follows:
1843 #define GMOCK_EXPECT_CALL_IMPL_(obj, call) \
1844     ((obj).gmock_##call).InternalExpectedAt(__FILE__, __LINE__, #obj, #call)
1845 #define EXPECT_CALL(obj, call) GMOCK_EXPECT_CALL_IMPL_(obj, call)

Therefore are 2 problems that I see:
1. To find places where EXPECT_CALL is used, we need to match gmock_myMethod (rather than myMethod)
2. Once we find an invocation of gmock_myMethod, we need to jump up in macro invocations chain to arrive at the location of the actual argument for EXPECT_CALL.  This is most easily done by going backwards from the invocation of gmock_myMethod until we find the location of the actual argument (i.e. location containing "myMethod" + corresponding to the "call" parameter of the macro).

Comment 3 by lukasza@chromium.org, Jan 9 2017

Status: Fixed (was: Started)

Comment 4 by lukasza@chromium.org, Jan 23 2017

Cc: danakj@chromium.org
Status: Assigned (was: Fixed)
This is still a problem.  Example:

../../third_party/WebKit/Source/platform/graphics/ContiguousContainerTest.cpp:88:5: error: no member named 'gmock_destruct' in 'blink::(anonymous namespace)::MockDestructible'; did you mean 'gmock_Destruct'?

Comment 5 by lukasza@chromium.org, Jan 23 2017

Status: Started (was: Assigned)
Fix for the issue in #c4 is proposed at https://crrev.com/2654553003

Comment 7 by lukasza@chromium.org, Jan 25 2017 

Still some remaining trouble:

$ cat ~/scratch/ninja.err | grep 'error:.*did you mean' | sed -e "s/^.*\\(did you mean '[a-zA-Z0-9_]*'\\).*$/\\1/g" | sort | uniq -c | sort -n | grep gmock
      1 did you mean 'gmock_BeginRead'
      1 did you mean 'gmock_Complete'
      1 did you mean 'gmock_DidAddRemoteStream'
      1 did you mean 'gmock_DidConsumeBufferedAmount'
      1 did you mean 'gmock_DidGenerateICECandidate'
      1 did you mean 'gmock_DidRemoveRemoteStream'
      1 did you mean 'gmock_DrainAsBlobDataHandle'
      1 did you mean 'gmock_DrainAsFormData'
      1 did you mean 'gmock_EndRead'
      1 did you mean 'gmock_GetPublicState'
      1 did you mean 'gmock_HandleContextDestroyed'
      1 did you mean 'gmock_UserAgentOverride'
      2 did you mean 'gmock_Call'
      3 did you mean 'gmock_Run'
     19 did you mean 'gmock_WillOverwriteCanvas'

Comment 8 by lukasza@chromium.org, Jan 25 2017 

We only handle EXPECT_CALL and missed ON_CALL macro... doh...

Comment 9 by lukasza@chromium.org, Jan 25 2017 

Although there are also some other remaining issues (maybe sufficiently few to just tackle them manually):

- EXPECT_CALL nested in TEST_OVERDRAW_SETUP doesn't get its |willOverwriteCanvas| call rewritten to |WillOverwriteCanvas|.

Comment 11 by lukasza@chromium.org, Jan 26 2017

Status: Assigned (was: Started)
Hopefully this is fixed, but let's keep this open until we verify via a dry-run of the Great Blink Rename.

Comment 12 by lukasza@chromium.org, Feb 3 2017

Status: Fixed (was: Assigned)
There are no more gmock problems after the latest dry-run of the rename.
►

Sign in to add a comment