Adding tracing and gtest pretty printing support to TestPendingTask.

Adding tracing and gtest pretty printing support to TestPendingTask.

These functions where invaluable when trying to debug and fix the
cc::OrderedTestRunner as they let me easily inspect the tasks without having to
fire up GDB. They should also be useful in development / testing of other task
runners.

Added a very simple test for ShouldRunBefore which shows this stuff working. If
that test fails, the output looks like the following;
-------------------------------------------------------------------------------
../../base/test/test_pending_task_unittest.cc:49: Failure
Value of: task_after.ShouldRunBefore(task_first)
  Actual: false
Expected: true
TestPendingTask(
  {"delay":2000,"nestability":"NESTABLE","post_time":0,
   "posting_function":"Unknown@Unknown:-1","run_at":2000}
).ShouldRunBefore(TestPendingTask(
  {"delay":1000,"nestability":"NESTABLE","post_time":0,
   "posting_function":"Unknown@Unknown:-1","run_at":1000})
)
-------------------------------------------------------------------------------

BUG=380889
Committed: https://crrev.com/e120a88b850d51b1db67ea693652a8c855610fb6
Cr-Commit-Position: refs/heads/master@{#292370}

[mithro-old, 2014-08-20 06:58:51]

Hi ajwong / phajdan.jr, 
  I'm sending this patch to you for review as suggested by "git cl owners".
Please feel free to reassign if you think there is a better person to do the
review.

Hi akalin, 
  I'm sending this patch to you for review as "git blame" marks you as the
primary author of TestPendingTask.

Tim

[mithro-old, 2014-08-25 05:27:28]

On 2014/08/20 at 06:58:51, mithro wrote:
> Hi ajwong / phajdan.jr, 
>   I'm sending this patch to you for review as suggested by "git cl owners".
Please feel free to reassign if you think there is a better person to do the
review.
> 
> Hi akalin, 
>   I'm sending this patch to you for review as "git blame" marks you as the
primary author of TestPendingTask.
> 
> Tim

Friendly ping?

Tim

[awong, 2014-08-26 00:33:19]

ajwong@chromium.org changed reviewers:
- akalin@chromium.org

[awong, 2014-08-26 00:33:19]

Sorry...missed this review.

Mainly 1 comment.

https://codereview.chromium.org/491743002/diff/1/base/test/test_pending_task.h
File base/test/test_pending_task.h (right):

https://codereview.chromium.org/491743002/diff/1/base/test/test_pending_task....
base/test/test_pending_task.h:68: void PrintTo(const TestPendingTask& task,
::std::ostream* os);
This should be a method in TestPendingTask.

[mithro-old, 2014-08-26 01:34:54]

https://codereview.chromium.org/491743002/diff/1/base/test/test_pending_task.h
File base/test/test_pending_task.h (right):

https://codereview.chromium.org/491743002/diff/1/base/test/test_pending_task....
base/test/test_pending_task.h:68: void PrintTo(const TestPendingTask& task,
::std::ostream* os);
On 2014/08/26 00:33:19, awong wrote:
> This should be a method in TestPendingTask.

gtest requires this method to be in the name space but not on the object.

[awong, 2014-08-27 20:51:21]

https://codereview.chromium.org/491743002/diff/1/base/test/test_pending_task.h
File base/test/test_pending_task.h (right):

https://codereview.chromium.org/491743002/diff/1/base/test/test_pending_task....
base/test/test_pending_task.h:68: void PrintTo(const TestPendingTask& task,
::std::ostream* os);
On 2014/08/26 01:34:54, mithro wrote:
> On 2014/08/26 00:33:19, awong wrote:
> > This should be a method in TestPendingTask.
> 
> gtest requires this method to be in the name space but not on the object.

Ah hah...I just went and read the gtest documentation. I think you only need the
operator<<. Shall we delete the PrintTo?

Also, we don't normally root std namespace off of ::. Change to just std::
instead? Yeah, it's somewhat more ambiguous, but since the whole rest of the
codebase does this w/o much of a problem, we should stay consistent.

[mithro-old, 2014-08-28 06:25:45]

On 2014/08/27 at 20:51:21, ajwong wrote:
> https://codereview.chromium.org/491743002/diff/1/base/test/test_pending_task.h
> File base/test/test_pending_task.h (right):
> 
>
https://codereview.chromium.org/491743002/diff/1/base/test/test_pending_task....
> base/test/test_pending_task.h:68: void PrintTo(const TestPendingTask& task,
::std::ostream* os);
> On 2014/08/26 01:34:54, mithro wrote:
> > On 2014/08/26 00:33:19, awong wrote:
> > > This should be a method in TestPendingTask.
> > 
> > gtest requires this method to be in the name space but not on the object.
> 
> Ah hah...I just went and read the gtest documentation. I think you only need
the operator<<. Shall we delete the PrintTo?

I agree that the gtest documentation seems to indicate that we should only need
the operator<<

However, when I removed the PrintTo function and then gtest seemed inconsistent
about when it would do the pretty printing. I had a dig into the gtest code and
they make heavy use of templating and koenig lookup (Argument-dependent name
lookup) to find operator<< and PrintTo methods and even after looking at it four
hours I still have very little understanding of what is going on.

> Also, we don't normally root std namespace off of ::. Change to just std::
instead? Yeah, it's somewhat more ambiguous, but since the whole rest of the
codebase does this w/o much of a problem, we should stay consistent.

Done.

Tim

[awong, 2014-08-28 06:49:18]

LGTM

On 2014/08/28 06:25:45, mithro wrote:
> On 2014/08/27 at 20:51:21, ajwong wrote:
> >
https://codereview.chromium.org/491743002/diff/1/base/test/test_pending_task.h
> > File base/test/test_pending_task.h (right):
> > 
> >
>
https://codereview.chromium.org/491743002/diff/1/base/test/test_pending_task....
> > base/test/test_pending_task.h:68: void PrintTo(const TestPendingTask& task,
> ::std::ostream* os);
> > On 2014/08/26 01:34:54, mithro wrote:
> > > On 2014/08/26 00:33:19, awong wrote:
> > > > This should be a method in TestPendingTask.
> > > 
> > > gtest requires this method to be in the name space but not on the object.
> > 
> > Ah hah...I just went and read the gtest documentation. I think you only need
> the operator<<. Shall we delete the PrintTo?
> 
> I agree that the gtest documentation seems to indicate that we should only
need
> the operator<<
> 
> However, when I removed the PrintTo function and then gtest seemed
inconsistent
> about when it would do the pretty printing. I had a dig into the gtest code
and
> they make heavy use of templating and koenig lookup (Argument-dependent name
> lookup) to find operator<< and PrintTo methods and even after looking at it
four
> hours I still have very little understanding of what is going on.

:( Sorry have had you spend so much time on it. If gtest is indeed working
strangely without both operator<< and PrintTo defined, then guess we have to
have it defined.

Thanks for being so thorough.
 
> > Also, we don't normally root std namespace off of ::. Change to just std::
> instead? Yeah, it's somewhat more ambiguous, but since the whole rest of the
> codebase does this w/o much of a problem, we should stay consistent.
> 
> Done.
> 
> Tim

[mithro-old, 2014-08-28 07:23:46]

The CQ bit was checked by mithro@mithis.com

[commit-bot: I haz the power, 2014-08-28 07:25:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mithro@mithis.com/491743002/40001

[commit-bot: I haz the power, 2014-08-28 12:38:18]

Committed patchset #3 (id:40001) as 43c4c2a05a81abfe692a8902cea19ec89ec4ff39

[commit-bot: I haz the power, 2014-09-10 02:59:09]

Patchset 3 (id:??) landed as
https://crrev.com/e120a88b850d51b1db67ea693652a8c855610fb6
Cr-Commit-Position: refs/heads/master@{#292370}