Add command-line non-client support to unit test lib.

Add command-line non-client support to unit test lib.

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

[Bob Nystrom, 2011-10-28 23:10:20]

This pulls the client-specific code out of the main unit test file. It then
specifies two entrypoints (unittest and unittest_vm) that you can import. The
former is for running tests on the client (i.e. using DOM) and the latter runs
in the command line.

Still pretty rudimentary, but it's getting there.

[sra1, 2011-10-28 23:31:10]

lgtm

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/sample....
File client/testing/unittest/sample.dart (right):

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/sample....
client/testing/unittest/sample.dart:45: }
Can you add an async test?

[Siggi Cherem (dart-lang), 2011-10-28 23:35:13]

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/shared....
File client/testing/unittest/shared.dart (right):

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/shared....
client/testing/unittest/shared.dart:136: _runTestsPlatform();
this feels like an odd name for what it does.

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/unittes...
File client/testing/unittest/unittest.dart (right):

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/unittes...
client/testing/unittest/unittest.dart:17: _initializePlatform() {
suggestion - what if we create an abstraction for each platform instead?

Basically I'm thinking having a top-level symbol [_platform] which implements a
[Platform] interface. Then in shared.dart every platform-specific call is of the
form:

  _platform.foo()

rather than 
  _fooPlatform()

We would still do the trick you use here to combine the code for each entry
point, but I think the code in shared.dart will be easier to
understand/maintain.

[Bob Nystrom, 2011-10-28 23:57:48]

Open for ideas on how to make the entrypoint/platform-specific functions look
better.

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/sample....
File client/testing/unittest/sample.dart (right):

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/sample....
client/testing/unittest/sample.dart:45: }
On 2011/10/28 23:31:10, sra1 wrote:
> Can you add an async test?

Done. Async is still a little flaky (if an async test *fails*, the VM doesn't
know what to do with the exception that percolates up to the Timer callback),
but it works sort of.

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/shared....
File client/testing/unittest/shared.dart (right):

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/shared....
client/testing/unittest/shared.dart:136: _runTestsPlatform();
On 2011/10/28 23:35:13, sigmund wrote:
> this feels like an odd name for what it does.

I was going for a "Platform" suffix for methods that are specific to the
platform/entrypoint. You're right that it does read awkwardly. Ideas?

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/unittes...
File client/testing/unittest/unittest.dart (right):

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/unittes...
client/testing/unittest/unittest.dart:17: _initializePlatform() {
On 2011/10/28 23:35:13, sigmund wrote:
> suggestion - what if we create an abstraction for each platform instead?
> 
> Basically I'm thinking having a top-level symbol [_platform] which implements
a
> [Platform] interface. Then in shared.dart every platform-specific call is of
the
> form:
> 
>   _platform.foo()

When would that field get initialized? Would the Platform just be a const class?
 That feels like more work than I want to do to just swap a couple of functions
out. I do like that your suggestion reads better.

Maybe I should do _platformRunTests() instead of _runTestsPlatform()?


> 
> rather than 
>   _fooPlatform()
> 
> We would still do the trick you use here to combine the code for each entry
> point, but I think the code in shared.dart will be easier to
> understand/maintain.

[Siggi Cherem (dart-lang), 2011-10-29 00:13:19]

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/shared....
File client/testing/unittest/shared.dart (right):

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/shared....
client/testing/unittest/shared.dart:136: _runTestsPlatform();
On 2011/10/28 23:57:48, Bob Nystrom wrote:
> On 2011/10/28 23:35:13, sigmund wrote:
> > this feels like an odd name for what it does.
> 
> I was going for a "Platform" suffix for methods that are specific to the
> platform/entrypoint. You're right that it does read awkwardly. Ideas?

actually in this case I meant the prefix: sending a post message doesn't seem to
relate to 'runTests', maybe 'initTests'?

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/unittes...
File client/testing/unittest/unittest.dart (right):

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/unittes...
client/testing/unittest/unittest.dart:17: _initializePlatform() {
Another idea ---

if you'd like to avoid adding classes, a third option would be to make a library
with visible top-level functions, but import it with a prefix:

--- file browser_platform.dart:
init() {
...
}

--- file shared.dart:
_runTest() {
  platform.init();
  platform.runAsync(() {...});
}

--- file unittest.dart:
#import('browser_platform', prefix: 'platform');
#source('shared.dart');

http://codereview.chromium.org/8413058/diff/1/client/testing/unittest/unittes...
client/testing/unittest/unittest.dart:17: _initializePlatform() {
On 2011/10/28 23:57:48, Bob Nystrom wrote:
> On 2011/10/28 23:35:13, sigmund wrote:
> > suggestion - what if we create an abstraction for each platform instead?
> > 
> > Basically I'm thinking having a top-level symbol [_platform] which
implements
> a
> > [Platform] interface. Then in shared.dart every platform-specific call is of
> the
> > form:
> > 
> >   _platform.foo()
> 
> When would that field get initialized? Would the Platform just be a const
class?
>  That feels like more work than I want to do to just swap a couple of
functions
> out. I do like that your suggestion reads better.
> 
> Maybe I should do _platformRunTests() instead of _runTestsPlatform()?
> 
> 
> > 
> > rather than 
> >   _fooPlatform()
> > 
> > We would still do the trick you use here to combine the code for each entry
> > point, but I think the code in shared.dart will be easier to
> > understand/maintain.
> 

Basically yes, it would be a const class - here is more precisely what I meant.

--- file shared.dart:
interface Platform {
  init(); // == _initializePlatform
  initTests(); // == _runTestPlatform
  runAsync(void callback()); // == _doLaterPlatform
  completeTests(int passed, int failed, int errors);
}

...
_runTest() {
  _platform.initTests();
  _platform.runAsync(() { _nextBatch(); });
}

...
--- file unittest.dart:

#source('shared.dart');
#source('browser_platform.dart');
Platform _platform = new BrowserPlatform();

--- file browser_platform.dart ?:
class BrowserPlatform implements Platform {
 ...
}

[Siggi Cherem (dart-lang), 2011-10-29 00:37:26]

lgtm - with the name changes we talked about ...

[jimhug, 2011-10-30 20:05:53]

LGTM

I'm eager to try this out!  I don't yet have any good suggestions on how to
improve the platform-specific parts, but I agree that this feels like something
that could be cleaner.  We'll just have to keep pushing on this one - I have a
feeling that it won't become fully clear and clean until we also work out our
"package" story.