Add 'self_check' runtime for running self-checking unit tests

tools/testing/dart/test_suite.dart

Index: tools/testing/dart/test_suite.dart
diff --git a/tools/testing/dart/test_suite.dart b/tools/testing/dart/test_suite.dart
index 4fed64fec78fcd1efe2f3afdad1501e3fed98066..47b8a8791b67dcf9e333f435677375bbb708a3c2 100644
--- a/tools/testing/dart/test_suite.dart
+++ b/tools/testing/dart/test_suite.dart
@@ -123,6 +123,7 @@ abstract class TestSuite {
   // This function is set by subclasses before enqueueing starts.
   Function doTest;
   Map<String, String> _environmentOverrides;
+  RuntimeConfiguration runtimeConfiguration;
 
   TestSuite(this.configuration, this.suiteName) {
     TestUtils.buildDir(configuration); // Sets configuration_directory.
@@ -131,6 +132,7 @@ abstract class TestSuite {
         'DART_CONFIGURATION': configuration['configuration_directory']
       };
     }
+    runtimeConfiguration = new RuntimeConfiguration(configuration);
   }
 
   Map<String, String> get environmentOverrides => _environmentOverrides;
@@ -293,6 +295,9 @@ abstract class TestSuite {
   //  - test if the selector matches
   // and will enqueue the test (if necessary).
   void enqueueNewTestCase(TestCase testCase) {
+    if (testCase.isNegative && runtimeConfiguration.shouldSkipNegativeTests) {
+      return;
+    }

[kustermann, 2016/12/06 10:39:05]

I'm not sure if this is always doing the right thing. There are several
different cases of negative tests, some are "_negative_test.dart" (which can be
negative in compiler or runtime AFAIK) and multi tests annotations and co19 test
annotations, sometimes they are only negative in checked mode.

Could we get rid of this? 

The test_suite.dart file is already very complex and this makes it even worse.

Why is it even needed in the first place?

[asgerf, 2016/12/06 13:42:08]

If the test expects a compile-time error:
There is no data for the self checking unit test to consume, so there is nothing
to do.

If the test expects a runtime error:
The self_check runtime configuration does not execute the program, and the
expectations associated with the test case describe what should happen if the
code is executed, and are thus not relevant this runtime.

This could be handled in a more fine-grained way, but exactly because the code
is so unwieldy I have a preference for simplest solution which is to just ignore
the negative tests altogether. That way, the code for comparing test
expectations against concrete output does not get more complicated.

I don't really like this style either, but I think it's par for the course in
this codebase. Considering the functionality that was added I think it's a very
mild change.

The isNegative flag is the aggregate of many cases, e.g. in checked mode it
returns true for tests that are negative in checked mode. And this point is the
pretty much the first program point where this value has been computed, so it
cannot be filtered out sooner without refactoring/duplication.

     var expectations = testCase.expectedOutcomes;
 
     // Handle sharding based on the original test path (i.e. all multitests
@@ -1095,7 +1100,9 @@ class StandardTestSuite extends TestSuite {
             CommandBuilder.instance,
             compileTimeArguments,
             environmentOverrides);
-    commands.addAll(compilationArtifact.commands);
+    if (!configuration['skip-compilation']) {
+      commands.addAll(compilationArtifact.commands);
+    }
 
     if (expectCompileError(info) && compilerConfiguration.hasCompiler) {
       // Do not attempt to run the compiled result. A compilation
@@ -1103,8 +1110,6 @@ class StandardTestSuite extends TestSuite {
       return commands;
     }
 
-    RuntimeConfiguration runtimeConfiguration =
-        new RuntimeConfiguration(configuration);
     List<String> runtimeArguments =
         compilerConfiguration.computeRuntimeArguments(
             runtimeConfiguration,