throw when group or test is called while unittest system is running

throw when group or test is called while unittest system is running

This is not un-common and makes any future refactoring of unittest more difficult

Let's kill it before we rev a v1

R=efortuna@google.com

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

[Emily Fortuna, 2014-01-30 23:32:15]

looks good. 
can you add a negative test to test the throw? :-D

https://codereview.chromium.org/142023005/diff/20001/pkg/unittest/lib/unittes...
File pkg/unittest/lib/unittest.dart (right):

https://codereview.chromium.org/142023005/diff/20001/pkg/unittest/lib/unittes...
pkg/unittest/lib/unittest.dart:747: var timeout = unittestConfiguration.timeout;
any reason why these aren't final anymore? 
NBD, just curious

[Emily Fortuna, 2014-01-30 23:37:19]

lgtm! the test was updated!

[Siggi Cherem (dart-lang), 2014-01-30 23:39:01]

FWIW, this library has grown to a point where it's been really hard to maintain.
Our plan is to stop doing incremental changes here and start designing a new
unittest for v1. For example, there are a lot of language features available now
that can help us improve this code and make it more user friendly (e.g. like
using zones to better support async tests).

Might be easier to chat in person about the details...

https://codereview.chromium.org/142023005/diff/40001/pkg/unittest/lib/unittes...
File pkg/unittest/lib/unittest.dart (right):

https://codereview.chromium.org/142023005/diff/40001/pkg/unittest/lib/unittes...
pkg/unittest/lib/unittest.dart:282: int _currentTestCaseIndex = -1;
There is a lot of implicit state encoded in 'current index', maybe can we encode
whether you are active/inactive in a separate boolean value?

[kevmoo, 2014-01-30 23:42:04]

https://codereview.chromium.org/142023005/diff/20001/pkg/unittest/lib/unittes...
File pkg/unittest/lib/unittest.dart (right):

https://codereview.chromium.org/142023005/diff/20001/pkg/unittest/lib/unittes...
pkg/unittest/lib/unittest.dart:747: var timeout = unittestConfiguration.timeout;
On 2014/01/30 23:32:15, Emily Fortuna wrote:
> any reason why these aren't final anymore? 
> NBD, just curious

it's against our guidelines.

I had a crazy hang-up on this a while ago, but it seems it's fallen out of
favor. Going with convention.

https://codereview.chromium.org/142023005/diff/40001/pkg/unittest/lib/unittes...
File pkg/unittest/lib/unittest.dart (right):

https://codereview.chromium.org/142023005/diff/40001/pkg/unittest/lib/unittes...
pkg/unittest/lib/unittest.dart:282: int _currentTestCaseIndex = -1;
On 2014/01/30 23:39:01, Siggi Cherem (dart-lang) wrote:
> There is a lot of implicit state encoded in 'current index', maybe can we
encode
> whether you are active/inactive in a separate boolean value?

I introduced the currentIndex model a while back.

It tracks not started, running, and done.

I'm trying to minimize the many places where we are tracking things...so that
they still in sync.

[Siggi Cherem (dart-lang), 2014-01-30 23:44:29]

oops forgot one more small nit

https://codereview.chromium.org/142023005/diff/40001/pkg/unittest/lib/unittes...
File pkg/unittest/lib/unittest.dart (right):

https://codereview.chromium.org/142023005/diff/40001/pkg/unittest/lib/unittes...
pkg/unittest/lib/unittest.dart:748: Future f = _guardAsync(testCase._run, null,
testCase);
Future -> var

[Siggi Cherem (dart-lang), 2014-01-30 23:46:32]

https://codereview.chromium.org/142023005/diff/40001/pkg/unittest/lib/unittes...
File pkg/unittest/lib/unittest.dart (right):

https://codereview.chromium.org/142023005/diff/40001/pkg/unittest/lib/unittes...
pkg/unittest/lib/unittest.dart:282: int _currentTestCaseIndex = -1;
On 2014/01/30 23:42:04, kevmoo wrote:
> On 2014/01/30 23:39:01, Siggi Cherem (dart-lang) wrote:
> > There is a lot of implicit state encoded in 'current index', maybe can we
> encode
> > whether you are active/inactive in a separate boolean value?
> 
> I introduced the currentIndex model a while back.
> 
> It tracks not started, running, and done.
> 
> I'm trying to minimize the many places where we are tracking things...so that
> they still in sync.

I'm not convinced :), but lets at least add some dartdoc above this to explain
what are the possible states.

[kevmoo, 2014-01-31 02:27:36]

https://codereview.chromium.org/142023005/diff/40001/pkg/unittest/lib/unittes...
File pkg/unittest/lib/unittest.dart (right):

https://codereview.chromium.org/142023005/diff/40001/pkg/unittest/lib/unittes...
pkg/unittest/lib/unittest.dart:282: int _currentTestCaseIndex = -1;
On 2014/01/30 23:46:33, Siggi Cherem (dart-lang) wrote:
> On 2014/01/30 23:42:04, kevmoo wrote:
> > On 2014/01/30 23:39:01, Siggi Cherem (dart-lang) wrote:
> > > There is a lot of implicit state encoded in 'current index', maybe can we
> > encode
> > > whether you are active/inactive in a separate boolean value?
> > 
> > I introduced the currentIndex model a while back.
> > 
> > It tracks not started, running, and done.
> > 
> > I'm trying to minimize the many places where we are tracking things...so
that
> > they still in sync.
> 
> I'm not convinced :), but lets at least add some dartdoc above this to explain
> what are the possible states.

Done.

https://codereview.chromium.org/142023005/diff/40001/pkg/unittest/lib/unittes...
pkg/unittest/lib/unittest.dart:748: Future f = _guardAsync(testCase._run, null,
testCase);
On 2014/01/30 23:44:30, Siggi Cherem (dart-lang) wrote:
> Future -> var

This one is intentional. _guardAsync return dynamic.

This is just to advertise to users of the code (and checked mode) that we expect
this to be a Future.

[kevmoo, 2014-01-31 04:54:36]

Committed patchset #4 manually as r32188 (presubmit successful).