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

pkg/unittest/lib/unittest.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /**
  * Support for writing Dart unit tests.
  *
  * For information on installing and importing this library, see the
  * [unittest package on pub.dartlang.org]
  * (http://pub.dartlang.org/packages/unittest).
@@ skipping 261 matching lines @@
     _testTeardown = parentTeardown;
   }
 }
 
 // We use a 'dummy' context for the top level to eliminate null
 // checks when querying the context. This allows us to easily
 //  support top-level setUp/tearDown functions as well.
 final _rootContext = new _GroupContext();
 _GroupContext _currentContext = _rootContext;
 
-int _currentTestCaseIndex = 0;
+int _currentTestCaseIndex = -1;

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

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]

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:46:33]

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:37]

Done.

 
 /** [TestCase] currently being executed. */
 TestCase get currentTestCase =>
     (_currentTestCaseIndex >= 0 && _currentTestCaseIndex < testCases.length)
         ? testCases[_currentTestCaseIndex]
         : null;
 
 /** Whether the framework is in an initialized state. */
 bool _initialized = false;
 
@@ skipping 13 matching lines @@
 const FAIL = 'fail';
 /// Result string for an test case with an error.
 const ERROR = 'error';
 
 /**
  * Creates a new test case with the given description and body. The
  * description will include the descriptions of any surrounding group()
  * calls.
  */
 void test(String spec, TestFunction body) {
+  _requireNotRunning();
   ensureInitialized();
   if (!_soloTestSeen || _soloNestingLevel > 0) {
     var testcase = new TestCase._internal(testCases.length + 1, _fullSpec(spec),
                                         body);
     _testCases.add(testcase);
   }
 }
 
 /** Convenience function for skipping a test. */
 void skip_test(String spec, TestFunction body){}
 
 /**
  * Creates a new test case with the given description and body. The
  * description will include the descriptions of any surrounding group()
  * calls.
  *
  * If we use [solo_test] (or [solo_group]) instead of test, then all non-solo
  * tests will be disabled. Note that if we use [solo_group], all tests in
  * the group will be enabled, regardless of whether they use [test] or
  * [solo_test], or whether they are in a nested [group] vs [solo_group]. Put
  * another way, if there are any calls to [solo_test] or [solo_group] in a test
  * file, all tests that are not inside a [solo_group] will be disabled unless
  * they are [solo_test]s.
  *
  * [skip_test] and [skip_group] take precedence over soloing, by virtue of the
  * fact that they are effectively no-ops.
  */
 void solo_test(String spec, TestFunction body) {
+  _requireNotRunning();
   ensureInitialized();
   if (!_soloTestSeen) {
     _soloTestSeen = true;
     // This is the first solo-ed test. Discard all tests up to now.
     _testCases.clear();
   }
   ++_soloNestingLevel;
   try {
     test(spec, body);
   } finally {
@@ skipping 223 matching lines @@
 Function protectAsync2(Function callback, {String id}) {
   return new _SpreadArgsHelper(callback, 0, -1, null, id).invoke2;
 }
 
 /**
  * Creates a new named group of tests. Calls to group() or test() within the
  * body of the function passed to this will inherit this group's description.
  */
 void group(String description, void body()) {
   ensureInitialized();
+  _requireNotRunning();
   _currentContext = new _GroupContext(_currentContext, description);
   try {
     body();
   } catch (e, trace) {
     var stack = (trace == null) ? '' : ': ${trace.toString()}';
     _uncaughtErrorMessage = "${e.toString()}$stack";
   } finally {
     // Now that the group is over, restore the previous one.
     _currentContext = _currentContext.parent;
   }
 }
 
 /** Like [skip_test], but for groups. */
 void skip_group(String description, void body()) {}
 
 /** Like [solo_test], but for groups. */
 void solo_group(String description, void body()) {
+  _requireNotRunning();
   ensureInitialized();
   if (!_soloTestSeen) {
     _soloTestSeen = true;
     // This is the first solo-ed group. Discard all tests up to now.
     _testCases.clear();
   }
   ++_soloNestingLevel;
   try {
     group(description, body);
   } finally {
     --_soloNestingLevel;
   }
 }
 
 /**
  * Register a [setUp] function for a test [group]. This function will
  * be called before each test in the group is run.
  * [setUp] and [tearDown] should be called within the [group] before any
  * calls to [test]. The [setupTest] function can be asynchronous; in this
  * case it must return a [Future].
  */
 void setUp(Function setupTest) {
+  _requireNotRunning();
   _currentContext.testSetup = setupTest;
 }
 
 /**
  * Register a [tearDown] function for a test [group]. This function will
  * be called after each test in the group is run. Note that if groups
  * are nested only the most locally scoped [teardownTest] function will be run.
  * [setUp] and [tearDown] should be called within the [group] before any
  * calls to [test]. The [teardownTest] function can be asynchronous; in this
  * case it must return a [Future].
  */
 void tearDown(Function teardownTest) {
+  _requireNotRunning();
   _currentContext.testTeardown = teardownTest;
 }
 
 /** Advance to the next test case. */
 void _nextTestCase() {
   _currentTestCaseIndex++;
   _runTest();
 }
 
 /** Handle errors that happen outside the tests. */
@@ skipping 28 matching lines @@
   } else if (testFilter is RegExp) {
     filterFunction = (t) => testFilter.hasMatch(t.description);
   } else if (testFilter is Function) {
     filterFunction = testFilter;
   }
   _testCases.retainWhere(filterFunction);
 }
 
 /** Runs all queued tests, one at a time. */
 void runTests() {
+  _requireNotRunning();
   _ensureInitialized(false);
   _currentTestCaseIndex = 0;
   _config.onStart();
   _runTest();
 }
 
 /**
  * Run [tryBody] guarded in a try-catch block. If an exception is thrown, it is
  * passed to the corresponding test.
  *
@@ skipping 31 matching lines @@
   } else {
     testCase.error(message, trace);
   }
 }
 
 /**
  * Runs the next test.
  */
 void _runTest() {
   if (_currentTestCaseIndex >= testCases.length) {
+    assert(_currentTestCaseIndex == testCases.length);
     _completeTests();
   } else {
-    final testCase = testCases[_currentTestCaseIndex];
-    var f = _guardAsync(testCase._run, null, testCase);
+    var testCase = testCases[_currentTestCaseIndex];
+    Future f = _guardAsync(testCase._run, null, testCase);

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

Future -> var

[kevmoo, 2014/01/31 02:27:37]

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.

+    var timeout = unittestConfiguration.timeout;
+
     Timer timer;
-    final Duration timeout = unittestConfiguration.timeout;
     if (timeout != null) {
       try {
         timer = new Timer(timeout, () {
           testCase.error("Test timed out after ${timeout.inSeconds} seconds.");
           _nextTestCase();
         });
       } on UnsupportedError catch (e) {
         if (e.message != "Timer greater than 0.") rethrow;
         // Support running on d8 and jsshell which don't support timers.
       }
@@ skipping 22 matching lines @@
     switch (t.result) {
       case PASS:  passed++; break;
       case FAIL:  failed++; break;
       case ERROR: errors++; break;
     }
   }
   _config.onSummary(passed, failed, errors, testCases, _uncaughtErrorMessage);
   _config.onDone(passed > 0 && failed == 0 && errors == 0 &&
       _uncaughtErrorMessage == null);
   _initialized = false;
+  _currentTestCaseIndex = -1;
 }
 
 String _fullSpec(String spec) {
   var group = '${_currentContext.fullName}';
   if (spec == null) return group;
   return group != '' ? '$group$groupSep$spec' : spec;
 }
 
 /**
  * Lazily initializes the test library if not already initialized.
@@ skipping 56 matching lines @@
  * Useful to disable when debugging unittest or matcher customizations.
  */
 bool formatStacks = true;
 
 /**
  * A flag that controls whether we try to filter out irrelevant frames from
  * the stack trace. Requires formatStacks to be set.
  */
 bool filterStacks = true;
 
+void _requireNotRunning() {
+  if(_currentTestCaseIndex != -1) {
+    throw new StateError('Not allowed when tests are running.');
+  }
+}
+
 /**
  * Returns a Trace object from a StackTrace object or a String, or the
  * unchanged input if formatStacks is false;
  */
 Trace _getTrace(stack) {
   Trace trace;
   if (stack == null || !formatStacks) return null;
   if (stack is String) {
     trace = new Trace.parse(stack);
   } else if (stack is StackTrace) {
     trace = new Trace.from(stack);
   } else {
     throw new Exception('Invalid stack type ${stack.runtimeType} for $stack.');
   }
 
   if (!filterStacks) return trace;
 
   // Format the stack trace by removing everything above TestCase._runTest,
   // which is usually going to be irrelevant. Also fold together unittest and
   // core library calls so only the function the user called is visible.
   return new Trace(trace.frames.takeWhile((frame) {
     return frame.package != 'unittest' || frame.member != 'TestCase._runTest';
   })).terse.foldFrames((frame) => frame.package == 'unittest' || frame.isCore);
 }