Move test(), group(), and expect() to top level.

Move test(), group(), and expect() to top level.

Instead of being methods on UnitTestSuite, they will automatically create
one if there isn't already one, then run it. This lets you define tests just
like:

main() {
  test('not crazy', () {
    expect(sky) == blue;
  });
}

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

[Bob Nystrom, 2011-10-14 22:07:27]

This is one step away from having to put all tests inside UnitTestSuite classes.

[Siggi Cherem (dart-lang), 2011-10-14 23:26:29]

lgtm

http://codereview.chromium.org/8302012/diff/3001/client/testing/unittest/unit...
File client/testing/unittest/unittestsuite.dart (right):

http://codereview.chromium.org/8302012/diff/3001/client/testing/unittest/unit...
client/testing/unittest/unittestsuite.dart:110: void addAsyncTest(TestFunction
body, int callbacks) {
I'm not sure if this is currently used by any of the dom tests, but this might
break there. We might have to keep returning 'this' until we migrate everything
to the new format.

[Siggi Cherem (dart-lang), 2011-10-14 23:38:34]

just to put here what we discussed: it seems that this might not work if we have
2 top-level tests, and at least the latter one being asynchronous.

We could still support the current model that top-level tests are run directly,
as long as there is only one (when the declaration of the second tests gets
there we raise an error and suggest using a [group]).

Alternatively - add a top-level method [runSuite] and call it at the end of
[main].

[Bob Nystrom, 2011-10-15 01:28:09]

On 2011/10/14 23:38:34, sigmund wrote:
> just to put here what we discussed: it seems that this might not work if we
have
> 2 top-level tests, and at least the latter one being asynchronous.
> 
> We could still support the current model that top-level tests are run
directly,
> as long as there is only one (when the declaration of the second tests gets
> there we raise an error and suggest using a [group]).
> 
> Alternatively - add a top-level method [runSuite] and call it at the end of
> [main].

Since all tests are queued until after main() returns, this will handle even
multiple top level async test() calls. The only trick is making sure we don't
queue up multiple setTimeouts. Uploaded a change that should handle that.

PTAL!

[Siggi Cherem (dart-lang), 2011-10-17 23:23:50]

lgtm

nice!

http://codereview.chromium.org/8302012/diff/7001/client/testing/unittest/unit...
File client/testing/unittest/unittestsuite.dart (right):

http://codereview.chromium.org/8302012/diff/7001/client/testing/unittest/unit...
client/testing/unittest/unittestsuite.dart:58: // Immediately queue the suite
up. It will run after a timeout (i.e. after
to make this easier to understand, maybe add a reference to 'runTests' where the
actual timeout was added.

http://codereview.chromium.org/8302012/diff/7001/client/testing/unittest/unit...
client/testing/unittest/unittestsuite.dart:81:
window.dynamic.on.contentLoaded.add(listener);
not related to your change - but worth pointing out. 

I just noticed that this line only works because we run the scripts while
loading the html (before this event). And we'll need to do things differently
when we change to use htmlconverter in the future.

[Anton Muhin, 2011-10-18 06:41:53]

Sorry for late response---we had offsite and I was OOO.

http://codereview.chromium.org/8302012/diff/7001/client/testing/unittest/unit...
File client/testing/unittest/unittestsuite.dart (right):

http://codereview.chromium.org/8302012/diff/7001/client/testing/unittest/unit...
client/testing/unittest/unittestsuite.dart:93: test(null, body);
if you don't want to allow chaining, you probably should modify return type as
well.

http://codereview.chromium.org/8302012/diff/7001/client/testing/unittest/unit...
client/testing/unittest/unittestsuite.dart:293: _currentSuite._tests.length + 1,
_fullSpec(spec), body, callbacks);
nit: two more spaces?

http://codereview.chromium.org/8302012/diff/7001/client/testing/unittest/unit...
client/testing/unittest/unittestsuite.dart:345: return false;
apparently we never check return value.  Maybe remove those true/false and
introduce them then we really need them?

http://codereview.chromium.org/8302012/diff/7001/client/tests/client/view/vie...
File client/tests/client/view/view_tests.dart (right):

http://codereview.chromium.org/8302012/diff/7001/client/tests/client/view/vie...
client/tests/client/view/view_tests.dart:21: group('addToDocument()', () {
nice!

[Bob Nystrom, 2011-10-18 17:37:51]

Thanks for this. I'll send these changes out in a separate patch once I've got
it rebased and all.

http://codereview.chromium.org/8302012/diff/7001/client/testing/unittest/unit...
File client/testing/unittest/unittestsuite.dart (right):

http://codereview.chromium.org/8302012/diff/7001/client/testing/unittest/unit...
client/testing/unittest/unittestsuite.dart:93: test(null, body);
On 2011/10/18 06:41:53, antonmuhin wrote:
> if you don't want to allow chaining, you probably should modify return type as
> well.

Good catch. Fixed!

http://codereview.chromium.org/8302012/diff/7001/client/testing/unittest/unit...
client/testing/unittest/unittestsuite.dart:293: _currentSuite._tests.length + 1,
_fullSpec(spec), body, callbacks);
On 2011/10/18 06:41:53, antonmuhin wrote:
> nit: two more spaces?

Done.

http://codereview.chromium.org/8302012/diff/7001/client/testing/unittest/unit...
client/testing/unittest/unittestsuite.dart:345: return false;
On 2011/10/18 06:41:53, antonmuhin wrote:
> apparently we never check return value.  Maybe remove those true/false and
> introduce them then we really need them?

Done. That was some old code from a different style that I was trying. Not
needed any more. :)