Supervised Users: Add tests for PermissionRequestCreatorApiary.

Supervised Users: Add tests for PermissionRequestCreatorApiary.

BUG=

Committed: https://crrev.com/b33bca099896ecff865de57ba9ffd50e310ce042
Cr-Commit-Position: refs/heads/master@{#297759}

[Marc Treib, 2014-09-30 14:30:59]

treib@chromium.org changed reviewers:
+ bauerb@chromium.org

[Marc Treib, 2014-09-30 14:31:00]

I've started writing some tests, but there's really not too much point while the
PermissionRequestCreator doesn't even tell us if it succeeded.

Since you mentioned you're also working on the PermissionRequestCreators: What's
the correct order of operations here? Commit as-is and add some "success?"
feedback in a follow-up CL? Or wait for yours first and rebase on top of that?

[Bernhard Bauer, 2014-09-30 15:01:08]

LGTM with nits below.

On 2014/09/30 14:31:00, Marc Treib wrote:
> I've started writing some tests, but there's really not too much point while
the
> PermissionRequestCreator doesn't even tell us if it succeeded.
> 
> Since you mentioned you're also working on the PermissionRequestCreators:
What's
> the correct order of operations here? Commit as-is and add some "success?"
> feedback in a follow-up CL? Or wait for yours first and rebase on top of that?

Well, I have a CL that adds a success flag to the callback. I'm going to upload
it for review soon (and guess who gets to review it ;-) ), then you can decide
whether you want to patch it in locally and rebase on top of it, or wait for it
to land.

https://codereview.chromium.org/614053002/diff/1/chrome/browser/supervised_us...
File
chrome/browser/supervised_user/permission_request_creator_apiary_unittest.cc
(right):

https://codereview.chromium.org/614053002/diff/1/chrome/browser/supervised_us...
chrome/browser/supervised_user/permission_request_creator_apiary_unittest.cc:127:
EXPECT_LT(0U, token_service_.GetPendingRequests().size());
Non-equality operators use the "natural" parameter order, so the value under
test comes first.

Also, is there a reason you don't check the exact number?

https://codereview.chromium.org/614053002/diff/1/chrome/browser/supervised_us...
File chrome/browser/supervised_user/supervised_user_service.cc (right):

https://codereview.chromium.org/614053002/diff/1/chrome/browser/supervised_us...
chrome/browser/supervised_user/supervised_user_service.cc:758: GURL url(value);
You could probably inline |value|.

[Marc Treib, 2014-09-30 15:28:46]

https://codereview.chromium.org/614053002/diff/1/chrome/browser/supervised_us...
File
chrome/browser/supervised_user/permission_request_creator_apiary_unittest.cc
(right):

https://codereview.chromium.org/614053002/diff/1/chrome/browser/supervised_us...
chrome/browser/supervised_user/permission_request_creator_apiary_unittest.cc:127:
EXPECT_LT(0U, token_service_.GetPendingRequests().size());
On 2014/09/30 15:01:07, Bernhard Bauer wrote:
> Non-equality operators use the "natural" parameter order, so the value under
> test comes first.

Done.

> Also, is there a reason you don't check the exact number?

Yes: I don't want to test whether the PRCA creates separate access token
requests or reuses the same one.

https://codereview.chromium.org/614053002/diff/1/chrome/browser/supervised_us...
File chrome/browser/supervised_user/supervised_user_service.cc (right):

https://codereview.chromium.org/614053002/diff/1/chrome/browser/supervised_us...
chrome/browser/supervised_user/supervised_user_service.cc:758: GURL url(value);
On 2014/09/30 15:01:08, Bernhard Bauer wrote:
> You could probably inline |value|.

Done.

[Marc Treib, 2014-10-01 21:43:00]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2014-10-01 21:44:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/614053002/60001

[commit-bot: I haz the power, 2014-10-02 01:39:40]

Committed patchset #4 (id:60001) as ea4e584b9d648d5ee44a95f87c6e14712ad94592

[commit-bot: I haz the power, 2014-10-02 01:40:46]

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