pkg/matcher: refactor with unittest v0.12 work

refactor with unittest

R=nweiz@google.com

Committed: https://github.com/dart-lang/matcher/commit/14747b0658911cd55d140ba966653f9fbc915eab

[kevmoo, 2015-02-17 21:10:59]

kevmoo@google.com changed reviewers:
+ nweiz@google.com

[nweiz, 2015-02-17 23:17:43]

https://codereview.chromium.org/867133002/diff/40001/CHANGELOG.md
File CHANGELOG.md (right):

https://codereview.chromium.org/867133002/diff/40001/CHANGELOG.md#newcode4
CHANGELOG.md:4: [`unittest`](https://pub.dartlang.org/packages/unittest)
package.
Nit: indent +2. Also below.

https://codereview.chromium.org/867133002/diff/40001/CHANGELOG.md#newcode5
CHANGELOG.md:5: * `TestFailure`, `ErrorFormatter`, `expect`, `fail`, and
'wrapAsync'
Nit: add a period.

https://codereview.chromium.org/867133002/diff/40001/CHANGELOG.md#newcode8
CHANGELOG.md:8: * All of the `throws...Error` Matchers.
Have you checked to be sure this nested list renders right? Markdown can be
finnicky with lists and spacing.

https://codereview.chromium.org/867133002/diff/40001/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/867133002/diff/40001/pubspec.yaml#newcode12
pubspec.yaml:12: url: https://github.com/dart-lang/unittest
Add a note here explaining that matcher needs a git dependency only because
there's no released version of unittest that's compatible with matcher 0.12.0.

https://codereview.chromium.org/867133002/diff/40001/test/core_matchers_test....
File test/core_matchers_test.dart (right):

https://codereview.chromium.org/867133002/diff/40001/test/core_matchers_test....
test/core_matchers_test.dart:192: class _PrefixMatcher extends Matcher {
Add at least a short description of this.

https://codereview.chromium.org/867133002/diff/40001/test/operator_matchers_t...
File test/operator_matchers_test.dart (right):

https://codereview.chromium.org/867133002/diff/40001/test/operator_matchers_t...
test/operator_matchers_test.dart:9: show test, group, expect,
throwsArgumentError;
Don't use "show" unless we absolutely have to.

https://codereview.chromium.org/867133002/diff/40001/test/test_utils.dart
File test/test_utils.dart (right):

https://codereview.chromium.org/867133002/diff/40001/test/test_utils.dart#new...
test/test_utils.dart:1: // Copyright (c) 2012, the Dart project authors.  Please
see the AUTHORS file
Can we rename this to just "utils.dart"?

[kevmoo, 2015-02-18 00:33:40]

https://codereview.chromium.org/867133002/diff/40001/CHANGELOG.md
File CHANGELOG.md (right):

https://codereview.chromium.org/867133002/diff/40001/CHANGELOG.md#newcode4
CHANGELOG.md:4: [`unittest`](https://pub.dartlang.org/packages/unittest)
package.
On 2015/02/17 23:17:42, nweiz wrote:
> Nit: indent +2. Also below.

Done.

https://codereview.chromium.org/867133002/diff/40001/CHANGELOG.md#newcode5
CHANGELOG.md:5: * `TestFailure`, `ErrorFormatter`, `expect`, `fail`, and
'wrapAsync'
On 2015/02/17 23:17:42, nweiz wrote:
> Nit: add a period.

Done.

https://codereview.chromium.org/867133002/diff/40001/CHANGELOG.md#newcode8
CHANGELOG.md:8: * All of the `throws...Error` Matchers.
On 2015/02/17 23:17:42, nweiz wrote:
> Have you checked to be sure this nested list renders right? Markdown can be
> finnicky with lists and spacing.


Yup

https://codereview.chromium.org/867133002/diff/40001/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/867133002/diff/40001/pubspec.yaml#newcode12
pubspec.yaml:12: url: https://github.com/dart-lang/unittest
On 2015/02/17 23:17:42, nweiz wrote:
> Add a note here explaining that matcher needs a git dependency only because
> there's no released version of unittest that's compatible with matcher 0.12.0.

Done.

https://codereview.chromium.org/867133002/diff/40001/test/core_matchers_test....
File test/core_matchers_test.dart (right):

https://codereview.chromium.org/867133002/diff/40001/test/core_matchers_test....
test/core_matchers_test.dart:192: class _PrefixMatcher extends Matcher {
On 2015/02/17 23:17:42, nweiz wrote:
> Add at least a short description of this.

Or remove it. Much easier. :-)

https://codereview.chromium.org/867133002/diff/40001/test/operator_matchers_t...
File test/operator_matchers_test.dart (right):

https://codereview.chromium.org/867133002/diff/40001/test/operator_matchers_t...
test/operator_matchers_test.dart:9: show test, group, expect,
throwsArgumentError;
On 2015/02/17 23:17:42, nweiz wrote:
> Don't use "show" unless we absolutely have to.

This is very explicit. Since unittest exports matcher, I want to be clear what
we're getting from the matcher package and what we're getting from unittest.

If I eleminate 'show', I could just remove the matcher import entirely and this
code still works

https://codereview.chromium.org/867133002/diff/40001/test/test_utils.dart
File test/test_utils.dart (right):

https://codereview.chromium.org/867133002/diff/40001/test/test_utils.dart#new...
test/test_utils.dart:1: // Copyright (c) 2012, the Dart project authors.  Please
see the AUTHORS file
On 2015/02/17 23:17:42, nweiz wrote:
> Can we rename this to just "utils.dart"?

Maybe later? Trying to keep the delta relatively clean

[nweiz, 2015-02-18 00:42:07]

lgtm, although I do think you should remove "show".

https://codereview.chromium.org/867133002/diff/40001/test/operator_matchers_t...
File test/operator_matchers_test.dart (right):

https://codereview.chromium.org/867133002/diff/40001/test/operator_matchers_t...
test/operator_matchers_test.dart:9: show test, group, expect,
throwsArgumentError;
On 2015/02/18 00:33:40, kevmoo wrote:
> On 2015/02/17 23:17:42, nweiz wrote:
> > Don't use "show" unless we absolutely have to.
> 
> This is very explicit. Since unittest exports matcher, I want to be clear what
> we're getting from the matcher package and what we're getting from unittest.

There's no distinction between the test getting something "from matcher" or
"from unittest". It's the same member regardless.

> If I eleminate 'show', I could just remove the matcher import entirely and
this
> code still works

I don't necessarily think this is a bad thing.

[kevmoo, 2015-02-18 00:49:42]

Committed patchset #4 (id:60001) manually as
14747b0658911cd55d140ba966653f9fbc915eab (presubmit successful).

[kevmoo, 2015-02-18 00:49:50]

https://codereview.chromium.org/867133002/diff/40001/test/operator_matchers_t...
File test/operator_matchers_test.dart (right):

https://codereview.chromium.org/867133002/diff/40001/test/operator_matchers_t...
test/operator_matchers_test.dart:9: show test, group, expect,
throwsArgumentError;
On 2015/02/18 00:42:07, nweiz wrote:
> On 2015/02/18 00:33:40, kevmoo wrote:
> > On 2015/02/17 23:17:42, nweiz wrote:
> > > Don't use "show" unless we absolutely have to.
> > 
> > This is very explicit. Since unittest exports matcher, I want to be clear
what
> > we're getting from the matcher package and what we're getting from unittest.
> 
> There's no distinction between the test getting something "from matcher" or
> "from unittest". It's the same member regardless.
> 
> > If I eleminate 'show', I could just remove the matcher import entirely and
> this
> > code still works
> 
> I don't necessarily think this is a bad thing.

To put it another way: unittest could change what it exports at any time and it
shouldn't affect the code's ability to run. Let's agree to disagree. :-)