Switch to 'test', add different support for 'solo'.

Switch to 'test', add different support for 'solo'.

R=brianwilkerson@google.com, paulberry@google.com
BUG=

Committed: https://github.com/dart-lang/test_reflective_loader/commit/6a5faf3d5cabda480cb679ffd228f6931b7cc6ca

[Brian Wilkerson, 2016-10-04 19:14:16]

lgtm, but I'm interested to see what changes have to be made to our code because
of the need to wrap everything in a new method call.

[Paul Berry, 2016-10-04 19:19:12]

https://codereview.chromium.org/2388073004/diff/1/CHANGELOG.md
File CHANGELOG.md (right):

https://codereview.chromium.org/2388073004/diff/1/CHANGELOG.md#newcode6
CHANGELOG.md:6: - Because 'package:test' does not define 'solo_test', in order
to keep
s/Because/Since/

https://codereview.chromium.org/2388073004/diff/1/lib/test_reflective_loader....
File lib/test_reflective_loader.dart (right):

https://codereview.chromium.org/2388073004/diff/1/lib/test_reflective_loader....
lib/test_reflective_loader.dart:96: * If there is a "solo" test method in the
top-level suite, it only "solo"
Drop the word "it"

https://codereview.chromium.org/2388073004/diff/1/lib/test_reflective_loader....
lib/test_reflective_loader.dart:171: }
AFAICT, if defineReflectiveTests() is called without an enclosing
defineReflectiveSuite(), then there will be no effect.  That seems undesirable.

How about if we extract the bottom half of defineReflectiveSuite() (everything
from "if (_currentSuiteLevel == 0) {" to the end), and put it in a new private
function.  Then we can call it from here also, so callers who just call
defineReflectiveTests() will get the behavior they want.

https://codereview.chromium.org/2388073004/diff/1/lib/test_reflective_loader....
lib/test_reflective_loader.dart:233: typedef void _TestFunction();
Return type needs to be `dynamic`, since some tests are async and return a
Future that needs to be waited for.

[scheglov, 2016-10-04 19:30:11]

PTAL

https://codereview.chromium.org/2388073004/diff/1/CHANGELOG.md
File CHANGELOG.md (right):

https://codereview.chromium.org/2388073004/diff/1/CHANGELOG.md#newcode6
CHANGELOG.md:6: - Because 'package:test' does not define 'solo_test', in order
to keep
On 2016/10/04 19:19:12, Paul Berry wrote:
> s/Because/Since/

Done.

https://codereview.chromium.org/2388073004/diff/1/lib/test_reflective_loader....
File lib/test_reflective_loader.dart (right):

https://codereview.chromium.org/2388073004/diff/1/lib/test_reflective_loader....
lib/test_reflective_loader.dart:96: * If there is a "solo" test method in the
top-level suite, it only "solo"
On 2016/10/04 19:19:12, Paul Berry wrote:
> Drop the word "it"

Done.

https://codereview.chromium.org/2388073004/diff/1/lib/test_reflective_loader....
lib/test_reflective_loader.dart:171: }
On 2016/10/04 19:19:12, Paul Berry wrote:
> AFAICT, if defineReflectiveTests() is called without an enclosing
> defineReflectiveSuite(), then there will be no effect.  That seems
undesirable.
> 
> How about if we extract the bottom half of defineReflectiveSuite() (everything
> from "if (_currentSuiteLevel == 0) {" to the end), and put it in a new private
> function.  Then we can call it from here also, so callers who just call
> defineReflectiveTests() will get the behavior they want.

Good idea.
Done.
Thanks.

https://codereview.chromium.org/2388073004/diff/1/lib/test_reflective_loader....
lib/test_reflective_loader.dart:233: typedef void _TestFunction();
On 2016/10/04 19:19:12, Paul Berry wrote:
> Return type needs to be `dynamic`, since some tests are async and return a
> Future that needs to be waited for.

Done.

[Paul Berry, 2016-10-04 19:35:28]

lgtm

[scheglov, 2016-10-04 19:48:19]

Description was changed from

==========
Switch to 'test', add different support for 'solo'.

R=brianwilkerson@google.com, paulberry@google.com
BUG=
==========

to

==========
Switch to 'test', add different support for 'solo'.

R=brianwilkerson@google.com, paulberry@google.com
BUG=

Committed:
https://github.com/dart-lang/test_reflective_loader/commit/6a5faf3d5cabda480c...
==========

[scheglov, 2016-10-04 19:48:20]

Committed patchset #2 (id:20001) manually as
6a5faf3d5cabda480cb679ffd228f6931b7cc6ca (presubmit successful).

[Brian Wilkerson, 2016-10-04 19:48:48]

lgtm