Update assert to follow the spec.

tests/language/src/AssertionTest.dart

Index: tests/language/src/AssertionTest.dart
===================================================================
--- tests/language/src/AssertionTest.dart	(revision 409)
+++ tests/language/src/AssertionTest.dart	(working copy)
@@ -1,48 +1,43 @@
 // Copyright (c) 2011, 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.
-// VMOptions=--enable_asserts
+// VMOptions=--enable_type_checks

[ahe, 2011/10/14 10:45:40]

I don't think this option belongs in language tests. This is a VM specific
option which is understood by the dartc VM "simulator", DartRunner.

Furthermore, the assert statement should also be tested in production mode.

[jat, 2011/10/14 14:46:54]

It is also understood by Dartc to do the same thing -- enable asserts and
runtime type checking.
 
Isn't it specified to be totally ignored in production mode?  Sure, we can have
a test to make sure that it is ignored in production mode but that side effects
in the assert statement still happen, etc -- but that would be a separate test.

 //
 // Dart test program testing assert statements.
 
-class AssertTest {
+class AssertionTest {
   static test() {
     int i = 0;
     try {
       assert(false);
-    } catch (AssertError error) {
+    } catch (AssertionError error) {
       i = 1;
-      Expect.equals("false", error.failedAssertion);
-      int pos = error.url.lastIndexOf("/", error.url.length);
-      if (pos == -1) {
-        pos = error.url.lastIndexOf("\\", error.url.length);
-      }
-      String subs = error.url.substring(pos + 1, error.url.length);
-      Expect.equals("AssertTest.dart", subs);
-      Expect.equals(12, error.line);
-      Expect.equals(14, error.column);

[regis, 2011/10/14 20:40:28]

John, please do not remove valid testing of the vm functionality. It is fine to
rename the class, but move this test to the vm tree and add your new test here.

Until you do that, please revert this change, since it broke the vm build.

[jat, 2011/10/14 20:44:46]

The test does not match the spec, so it seems reasonable to make the test match
the spec and mark the test as failing on the VM until the VM's behavior matches
the spec.

Also, there are tests like this where the VM is adding additional information
not in the spec, such as fields on exceptions, and in some cases that means we
cannot have a single test that works on both DartC and the VM.  In such cases, I
suggest that the base test be shared, which would not check
implementation-specific fields, and then there would be implementation-specific
tests which are much smaller and only check those implementation details.

I do not understand the point of keeping tests which encode non-compliant
behavior, as you seem to be suggesting.

[regis, 2011/10/14 21:04:41]

We are in agreement. I am not against renaming the class AssertError to
AssertionError to match the spec, or to remove the vm specific part from the
test in the language directory. However, the vm specific test should be
preserved by moving it (after renaming the class) to the vm test directory.

But I am against renaming the class in the test only and not renaming the
corresponding class in the core lib implementation of the vm, thereby breaking
the vm build. At least, the renamed test should be disabled for the vm and a bug
should be filed against the vm.

     }
     return i;
   }
+
   static testClosure() {
     int i = 0;
     try {
       assert(() => false);
-    } catch (AssertError error) {
+    } catch (AssertionError error) {
       i = 1;
-      Expect.equals("() => false", error.failedAssertion);
-      int pos = error.url.lastIndexOf("/", error.url.length);
-      if (pos == -1) {
-        pos = error.url.lastIndexOf("\\", error.url.length);
-      }
-      String subs = error.url.substring(pos + 1, error.url.length);
-      Expect.equals("AssertTest.dart", subs);
-      Expect.equals(30, error.line);
-      Expect.equals(14, error.column);
     }
     return i;
   }
 
+  static testClosure2() {

[ahe, 2011/10/14 10:45:40]

This method isn't called.

+    int i = 0;
+    try {
+      var x = () => false;
+      assert(x);
+    } catch (AssertionError error) {
+      i = 1;
+    }
+    return i;
+  }
+
+
   static testMain() {
     Expect.equals(1, test());
     Expect.equals(1, testClosure());

[ahe, 2011/10/14 10:45:40]

Could you add a few more test cases, for example:

* test that assert(true) works

* test that assert(unknown) works, where unknown is something that cannot be
statically determined without inlining method calls.

@@ -50,5 +45,5 @@
 }
 
 main() {
-  AssertTest.testMain();
+  AssertionTest.testMain();
 }