pkg/oauth2 bringing tests into 2014

pkg/oauth2 bringing tests into 2014

R=nweiz@google.com

Committed: https://code.google.com/p/dart/source/detail?r=33357

[kevmoo, 2014-03-05 22:29:09]

fixing redness

[Jennifer Messerly, 2014-03-05 22:30:56]

looks fine to me but this is Nathan's code AFAIK :)

[nweiz, 2014-03-05 22:38:08]

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/pubspec.yaml
File pkg/oauth2/pubspec.yaml (right):

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/pubspec.yaml#newc...
pkg/oauth2/pubspec.yaml:10: sdk: '>=1.0.0 <2.0.0'
What new 1.0 feature(s) is this using?

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/test/authorizatio...
File pkg/oauth2/test/authorization_code_grant_test.dart (right):

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/test/authorizatio...
pkg/oauth2/test/authorization_code_grant_test.dart:94: throwsStateError);
Indent this correctly.

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/test/authorizatio...
pkg/oauth2/test/authorization_code_grant_test.dart:125: throwsFormatException);
Indent this correctly.

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/test/authorizatio...
pkg/oauth2/test/authorization_code_grant_test.dart:157: });
These days I'd write this as

    expect(grant.handleAuthorizationResponse({'code': 'auth code'})
          .then((client) => client.credentials.accessToken),
        completion(equals('access token')));

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/test/authorizatio...
pkg/oauth2/test/authorization_code_grant_test.dart:176: throwsStateError);
Indent this correctly.

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/test/credentials_...
File pkg/oauth2/test/credentials_test.dart (right):

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/test/credentials_...
pkg/oauth2/test/credentials_test.dart:19: ExpectClient httpClient;
If you're going to move this into [main], get rid of the type annotation.

[Jennifer Messerly, 2014-03-05 22:50:08]

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/pubspec.yaml
File pkg/oauth2/pubspec.yaml (right):

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/pubspec.yaml#newc...
pkg/oauth2/pubspec.yaml:10: sdk: '>=1.0.0 <2.0.0'
On 2014/03/05 22:38:08, nweiz wrote:
> What new 1.0 feature(s) is this using?

fyi -- I've generally been bumping SDK lower bounds to 1.0.0, since there's not
really an excuse to be using a pre-1.0 SDK build. Similarly 1.2.0-dev.4 would be
bumped to 1.2.0 now that it is out the door.

Of course, 1.0 -> 1.1 -> 1.2 changes should be done more cautiously and only if
it is actually necessary.

[kevmoo, 2014-03-06 00:01:42]

PTAL

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/pubspec.yaml
File pkg/oauth2/pubspec.yaml (right):

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/pubspec.yaml#newc...
pkg/oauth2/pubspec.yaml:10: sdk: '>=1.0.0 <2.0.0'
On 2014/03/05 22:50:09, John Messerly wrote:
> On 2014/03/05 22:38:08, nweiz wrote:
> > What new 1.0 feature(s) is this using?
> 
> fyi -- I've generally been bumping SDK lower bounds to 1.0.0, since there's
not
> really an excuse to be using a pre-1.0 SDK build. Similarly 1.2.0-dev.4 would
be
> bumped to 1.2.0 now that it is out the door.
> 
> Of course, 1.0 -> 1.1 -> 1.2 changes should be done more cautiously and only
if
> it is actually necessary.

Agreed

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/test/authorizatio...
File pkg/oauth2/test/authorization_code_grant_test.dart (right):

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/test/authorizatio...
pkg/oauth2/test/authorization_code_grant_test.dart:94: throwsStateError);
On 2014/03/05 22:38:08, nweiz wrote:
> Indent this correctly.

Done.

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/test/authorizatio...
pkg/oauth2/test/authorization_code_grant_test.dart:125: throwsFormatException);
On 2014/03/05 22:38:08, nweiz wrote:
> Indent this correctly.

Done.

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/test/authorizatio...
pkg/oauth2/test/authorization_code_grant_test.dart:157: });
On 2014/03/05 22:38:08, nweiz wrote:
> These days I'd write this as
> 
>     expect(grant.handleAuthorizationResponse({'code': 'auth code'})
>           .then((client) => client.credentials.accessToken),
>         completion(equals('access token')));

Done.

https://codereview.chromium.org/187843006/diff/1/pkg/oauth2/test/authorizatio...
pkg/oauth2/test/authorization_code_grant_test.dart:176: throwsStateError);
On 2014/03/05 22:38:08, nweiz wrote:
> Indent this correctly.

Done.

[nweiz, 2014-03-06 00:12:17]

lgtm

https://codereview.chromium.org/187843006/diff/20001/pkg/oauth2/test/authoriz...
File pkg/oauth2/test/authorization_code_grant_test.dart (right):

https://codereview.chromium.org/187843006/diff/20001/pkg/oauth2/test/authoriz...
pkg/oauth2/test/authorization_code_grant_test.dart:151:
completion(equals('access token')));
Indent this +2 spaces. You can also indent the previous line +2 spaces to help
keep it distinct.

[kevmoo, 2014-03-06 00:13:49]

Committed patchset #3 manually as r33357 (presubmit successful).