GMock matchers for net::Error

GMock matchers for net::Error

Provides more informative test failure messages.

Committed: https://crrev.com/32886411988498854a43468fe709f634051a1f84
Cr-Commit-Position: refs/heads/master@{#403426}

[Rob Percival, 2016-06-30 08:50:07]

The CQ bit was checked by robpercival@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-30 08:50:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Rob Percival, 2016-06-30 08:52:32]

robpercival@chromium.org changed reviewers:
+ eroman@chromium.org

[Rob Percival, 2016-06-30 08:52:33]

PTAL

[commit-bot: I haz the power, 2016-06-30 09:50:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-30 09:50:39]

Dry run: This issue passed the CQ dry run.

[Rob Percival, 2016-06-30 17:22:29]

If you decide that this is worth having, I can also, optionally, create a CL
that refactors all the existing ASSERT_EQ(net::ERR_*, ...) calls into
ASSERT_THAT(..., IsError(net::ERR_*)). That way, all of the net tests benefit
(should they ever break).

Alternatively, I'm quite happy to just declare these matchers at the top of my
own tests.

[eroman, 2016-06-30 19:09:23]

LGTM.

Sure, I am good with such a refactor!

(Ideally I think net::Error would be a distinct type from int making it possible
to provide this with a pretty printer instead, but that is a different bug...)

[Rob Percival, 2016-07-01 08:22:17]

The CQ bit was checked by robpercival@chromium.org

[commit-bot: I haz the power, 2016-07-01 08:22:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-01 08:26:19]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-01 08:26:21]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-01 08:28:04]

Description was changed from

==========
GMock matchers for net::Error

Provides more informative test failure messages.
==========

to

==========
GMock matchers for net::Error

Provides more informative test failure messages.

Committed: https://crrev.com/32886411988498854a43468fe709f634051a1f84
Cr-Commit-Position: refs/heads/master@{#403426}
==========

[commit-bot: I haz the power, 2016-07-01 08:28:05]

Patchset 1 (id:??) landed as
https://crrev.com/32886411988498854a43468fe709f634051a1f84
Cr-Commit-Position: refs/heads/master@{#403426}

[mmenke, 2016-07-11 15:02:27]

On 2016/07/01 08:28:05, commit-bot: I haz the power wrote:
> Patchset 1 (id:??) landed as
> https://crrev.com/32886411988498854a43468fe709f634051a1f84
> Cr-Commit-Position: refs/heads/master@{#403426}

Hrm...Eric:  Sure this is a good idea?  I would have pushed back on this,
because of obscurity of the syntax, and because I feel that clear test code is
more important than clearer test error messages.

[eroman, 2016-07-12 19:38:00]

On Mon, Jul 11, 2016 at 8:02 AM, <mmenke@chromium.org> wrote:

> On 2016/07/01 08:28:05, commit-bot: I haz the power wrote:
> > Patchset 1 (id:??) landed as
> > https://crrev.com/32886411988498854a43468fe709f634051a1f84
> > Cr-Commit-Position: refs/heads/master@{#403426}
>
> Hrm...Eric: Sure this is a good idea? I would have pushed back on this,
> because of obscurity of the syntax, and because I feel that clear test
> code is
> more important than clearer test error messages.
>

I can certainly sympathize with that argument.

My thinking when approving it is that gmock is a dependency elsewhere so
like it or not developers need to be familiar with it. The IsError() /
IsOk() still felt readable, although admittedly less writable.


> https://chromiumcodereview-hr.appspot.com/2111093002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Rob Percival, 2016-07-14 17:50:06]

On 2016/07/12 19:38:00, eroman wrote:
> On Mon, Jul 11, 2016 at 8:02 AM, <mailto:mmenke@chromium.org> wrote:
> 
> > On 2016/07/01 08:28:05, commit-bot: I haz the power wrote:
> > > Patchset 1 (id:??) landed as
> > > https://crrev.com/32886411988498854a43468fe709f634051a1f84
> > > Cr-Commit-Position: refs/heads/master@{#403426}
> >
> > Hrm...Eric: Sure this is a good idea? I would have pushed back on this,
> > because of obscurity of the syntax, and because I feel that clear test
> > code is
> > more important than clearer test error messages.
> >
> 
> I can certainly sympathize with that argument.
> 
> My thinking when approving it is that gmock is a dependency elsewhere so
> like it or not developers need to be familiar with it. The IsError() /
> IsOk() still felt readable, although admittedly less writable.
> 
> 
> > https://chromiumcodereview-hr.appspot.com/2111093002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

There is precedence for this in google3, which has testing::status::StatusIs and
testing::status::IsOk. Plus, gently introducing GMock matchers in this way has
the benefit of making developers aware of their existence and also a bit more
comfortable with more advanced uses of GMock, e.g. the built-in set of matchers
(https://github.com/google/googletest/blob/master/googlemock/docs/CheatSheet.m...)
and mock objects.