[expect] introduce Expect.throwsWhen and Expect.throwsTypeErrorWhen

Today, most tests that touch on a behavior variation end up
skipping expectations or the entirety of a test for some
testing configurations.  Moving forward, we'd like skip less
and try to account for the behavior variations if that's
reasonable.

This CL shows an approach to improve our test coverage for
behavior variations. We introduce two new methods to
[Expect] that allow us to conditionally check that a
function throws, depending on variation predicates.

The CL changes expectations for errors that don't occur
when dart2js omits parameter type checks or implicit
downcasts.

Note: originally I had the intention to introduce a name
parameter to `Expect.throws` and `Expect.throwsTypeError` to
avoid introducing a new API. However, because these APIs are
used for testing core language features, such as function
parameters themselves, we decided to keep the use of
features in these APIs as simple as it can be.

CoreLibraryReviewExempt: no public library semantic change - only improving test coverage under variations
Change-Id: I531657622655778491eaca8b37ba69ffaab559fc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/351340
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>

pkg/expect/lib/expect.dart

@@ -4,6 +4,9 @@
 
 /// This library contains an Expect class with static methods that can be used
 /// for simple unit-tests.
+///
+/// This library is deliberately simple and uses very few language features.
+/// This makes it safer to use for writing the language test suite.
 library expect;
 
 /// Whether the program is running without sound null safety.
@@ -606,6 +609,22 @@ class Expect {
     _fail('Expect.throws$msg fails: Did not throw');
   }
 
+  /// Calls [computation] and checks that it throws an [E] when [condition] is
+  /// `true`.
+  ///
+  /// If [condition] is `true`, the test succeeds if an [E] is thrown, and then
+  /// that error is returned. The test fails if nothing is thrown or a different
+  /// error is thrown.
+  /// If [condition] is `false`, the test succeeds if nothing is thrown,
+  /// returning `null`, and fails if anything is thrown.
+  static E? throwsWhen<E extends Object>(
+      bool condition, void Function() computation,
+      [String? reason]) {
+    if (condition) return throws<E>(computation, null, reason);
+    computation();
+    return null;
+  }
+
   static ArgumentError throwsArgumentError(void Function() f,
           [String reason = "ArgumentError"]) =>
       Expect.throws<ArgumentError>(f, _defaultCheck, reason);
@@ -639,6 +658,11 @@ class Expect {
           [String reason = "TypeError"]) =>
       Expect.throws<TypeError>(f, _defaultCheck, reason);
 
+  /// Checks that [f] throws a [TypeError] if an only if [condition] is `true`.
+  static TypeError? throwsTypeErrorWhen(bool condition, void Function() f,
+          [String? reason]) =>
+      Expect.throwsWhen<TypeError>(condition, f, reason);
+
   static UnsupportedError throwsUnsupportedError(void Function() f,
           [String reason = "UnsupportedError"]) =>
       Expect.throws<UnsupportedError>(f, _defaultCheck, reason);

tests/corelib/apply_test.dart

@@ -73,11 +73,8 @@ main() {
 
   // Test that apply works on callable objects when it is passed to a method
   // that expects Function (and not dynamic).
-  if (v.checkedImplicitDowncasts) {
-    Expect.throws(() => testList(42, new Callable(), [13, 29]));
-  } else {
-    testList(42, new Callable(), [13, 29]);
-  }
+  Expect.throwsWhen(
+      v.checkedImplicitDowncasts, () => testList(42, new Callable(), [13, 29]));
 
   testListTyped(42, new Callable(), [13, 29]);
 }

tests/corelib/int_modulo_arith_test.dart

@@ -125,11 +125,7 @@ testGcd() {
   // Test that gcd of value and other (non-negative) throws.
   testThrows(value, other) {
     callCombos(value, other, (a, b) {
-      if (v.checkedParameters || a is! int) {
-        Expect.throws(() => a.gcd(b));
-      } else {
-        a.gcd(b);
-      }
+      Expect.throwsWhen(v.checkedParameters || a is! int, () => a.gcd(b));
     });
   }
 

tests/corelib/list_removeat_test.dart

@@ -26,14 +26,6 @@ void testModifiableList(l1) {
   // Index must be integer and in range.
   Expect.throwsRangeError(() => l1.removeAt(-1), "negative");
   Expect.throwsRangeError(() => l1.removeAt(5), "too large");
-  if (v.checkedParameters) {
-    Expect.throws(
-        () => l1.removeAt(null),
-        // With sound null safety a TypeError is thrown.
-        // Without sound null safety an ArgumentError is thrown.
-        (e) => e is TypeError || e is ArgumentError,
-        "is null");
-  }
 
   Expect.equals(2, l1.removeAt(2), "l1-remove2");
   Expect.equals(1, l1[1], "l1-1[1]");
@@ -47,6 +39,17 @@ void testModifiableList(l1) {
   Expect.equals(3, l1[1], "l1-2[1]");
   Expect.equals(4, l1[2], "l1-2[2]");
   Expect.equals(3, l1.length, "length-2");
+
+  // Note: this is the last expectation because, when `!v.checkedParameters`
+  // this ends up modifying [l1].
+  final e = Expect.throwsWhen(
+      v.checkedParameters, () => l1.removeAt(null), "index is null");
+  if (e != null) {
+    Expect.equals(!v.unsoundNullSafety, e is TypeError,
+        "TypeError expected in sound null safety");
+    Expect.equals(v.unsoundNullSafety, e is ArgumentError,
+        "ArgumentError expected in unsound null safety");
+  }
 }
 
 void main() {

tests/corelib/string_split_test.dart

@@ -25,13 +25,6 @@ testSplit(List<String> expect, String string, Pattern pattern) {
   // Ensure that the correct type is reified.
   actual = actual as List<String>;
 
-  // Check that store of the wrong type throws. Don't test on configurations
-  // that don't perform type checks.
-  if (v.checkedParameters) {
-    Expect.throwsTypeError(
-        () => actual.add(42), 'List<String>.add should not accept an int');
-  }
-
   Expect.listEquals(expect, actual, '"$string".split($patternString)');
 }
 

tests/language/covariant/field_test.dart

@@ -38,25 +38,22 @@ class C extends B<A> {
 }
 
 main() {
-  // TODO(sigmund): replace with a Requirement comment when available.
-  if (!v.checkedParameters) return;
-
   // Dynamic method calls should always have their arguments type checked.
   dynamic d = new C();
-  Expect.throwsTypeError(() => d.s1 = new Object());
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => d.s1 = new Object());
 
   // Interface calls should have any arguments marked "genericCovariantImpl"
   // type checked provided that the corresponding argument on the interface
   // target is marked "genericCovariantInterface".
   B<Object> b = new C();
-  Expect.throwsTypeError(() => b.s2 = new Object());
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => b.s2 = new Object());
 
   // Interface calls should have any arguments marked "covariant" type checked,
   // regardless of whether the corresponding argument on the interface target is
   // marked "genericCovariantInterface".
-  Expect.throwsTypeError(() => b.s3 = new Object());
-  Expect.throwsTypeError(() => b.s4 = new Object());
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => b.s3 = new Object());
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => b.s4 = new Object());
 
   // This calls should have any arguments marked "covariant" type checked.
-  Expect.throwsTypeError(() => b.s5 = new Object());
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => b.s5 = new Object());
 }

tests/language/covariant/method_test.dart

@@ -38,30 +38,27 @@ class C extends B<A> {
 }
 
 main() {
-  // TODO(sigmund): replace with a Requirement comment when available.
-  if (!v.checkedParameters) return;
-
   // Dynamic method calls should always have their arguments type checked.
   dynamic d = new C();
-  Expect.throwsTypeError(() => d.f1(new Object()));
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => d.f1(new Object()));
 
   // Closure calls should have any arguments marked "genericCovariantImpl" type
   // checked.
   B<Object> b = new C();
   void Function(Object) f = b.f2;
-  Expect.throwsTypeError(() => f(new Object()));
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => f(new Object()));
 
   // Interface calls should have any arguments marked "genericCovariantImpl"
   // type checked provided that the corresponding argument on the interface
   // target is marked "genericCovariantInterface".
-  Expect.throwsTypeError(() => b.f2(new Object()));
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => b.f2(new Object()));
 
   // Interface calls should have any arguments marked "covariant" type checked,
   // regardless of whether the corresponding argument on the interface target is
   // marked "genericCovariantInterface".
-  Expect.throwsTypeError(() => b.f3(new Object()));
-  Expect.throwsTypeError(() => b.f4(new Object()));
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => b.f3(new Object()));
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => b.f4(new Object()));
 
   // This calls should have any arguments marked "covariant" type checked.
-  Expect.throwsTypeError(() => b.f5(new Object()));
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => b.f5(new Object()));
 }

tests/language/function/apply_generic2_test.dart

@@ -30,10 +30,8 @@ main() {
   check('[null, 33, null, 11, 22, null]',
       Function.apply(new CCC().memberFn, [], {#a4: 11, #a5: 22, #a2: 33}));
 
-  if (v.checkedParameters) {
-    Expect.throwsTypeError(
-        () => Function.apply(new CCC().memberFn, [], {#a3: 'hi'}));
-  }
+  Expect.throwsTypeErrorWhen(v.checkedParameters,
+      () => Function.apply(new CCC().memberFn, [], {#a3: 'hi'}));
 
   check('[11, 22, 33, null, null]',
       Function.apply(makeFn(), [], {#a1: 11, #a2: 22, #a3: 33}));
@@ -41,14 +39,12 @@ main() {
   check('[null, 33, null, 11, 22]',
       Function.apply(makeFn(), [], {#a4: 11, #a5: 22, #a2: 33}));
 
-  if (v.checkedParameters) {
-    Expect.throwsTypeError(() => Function.apply(makeFn(), [], {#a3: 'hi'}));
-  }
+  Expect.throwsTypeErrorWhen(
+      v.checkedParameters, () => Function.apply(makeFn(), [], {#a3: 'hi'}));
 
   check('[null, 33, null, 11, 22, null]',
       Function.apply(staticFn, [], {#a4: 11, #a5: 22, #a2: 33}));
 
-  if (v.checkedParameters) {
-    Expect.throwsTypeError(() => Function.apply(staticFn, [], {#a3: 'hi'}));
-  }
+  Expect.throwsTypeErrorWhen(
+      v.checkedParameters, () => Function.apply(staticFn, [], {#a3: 'hi'}));
 }

tests/language/function/type2_test.dart

@@ -16,9 +16,5 @@ class B<T> extends A<T> {
 }
 
 main() {
-  if (v.checkedParameters) {
-    Expect.throwsTypeError(() => new B<String>());
-  } else {
-    new B<String>();
-  }
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => new B<String>());
 }

tests/language/function/type_call_getter2_runtime_test.dart

@@ -24,29 +24,27 @@ main() {
   B b = new B();
   C c = new C();
 
-  if (v.checkedImplicitDowncasts) {
-    Expect.throwsTypeError(() {
-      Function a1 = a as dynamic;
-    });
+  Expect.throwsTypeErrorWhen(v.checkedImplicitDowncasts, () {
+    Function a1 = a as dynamic;
+  });
 
-    Expect.throwsTypeError(() {
-      F a2 = a as dynamic;
-    });
+  Expect.throwsTypeErrorWhen(v.checkedImplicitDowncasts, () {
+    F a2 = a as dynamic;
+  });
 
-    Expect.throwsTypeError(() {
-      Function b1 = b as dynamic;
-    });
+  Expect.throwsTypeErrorWhen(v.checkedImplicitDowncasts, () {
+    Function b1 = b as dynamic;
+  });
 
-    Expect.throwsTypeError(() {
-      F b2 = b as dynamic;
-    });
+  Expect.throwsTypeErrorWhen(v.checkedImplicitDowncasts, () {
+    F b2 = b as dynamic;
+  });
 
-    Expect.throwsTypeError(() {
-      Function c1 = c as dynamic;
-    });
+  Expect.throwsTypeErrorWhen(v.checkedImplicitDowncasts, () {
+    Function c1 = c as dynamic;
+  });
 
-    Expect.throwsTypeError(() {
-      F c2 = c as dynamic;
-    });
-  }
+  Expect.throwsTypeErrorWhen(v.checkedImplicitDowncasts, () {
+    F c2 = c as dynamic;
+  });
 }

tests/language/function/type_test.dart

@@ -9,9 +9,7 @@ import "package:expect/variations.dart" as v;
 typedef FListInt(List<int> l);
 
 main() {
-  // TODO(sigmund): replace with a Requirement comment when available.
-  if (!v.checkedImplicitDowncasts) return;
-  Expect.throwsTypeError(() {
+  Expect.throwsTypeErrorWhen(v.checkedImplicitDowncasts, () {
     // Static result type of f(), i.e. FList, is a subtype of FListInt.
     // However, run time type of returned function is not a subtype of FListInt.
     // Run time type check should not be eliminated.

tests/language/function_subtype/bound_closure7_test.dart

@@ -24,18 +24,14 @@ void main() {
   Expect.isTrue(f is Foo<Foo<int>>);
   Expect.isFalse(f is Foo<int>);
   Expect.isFalse(f is Foo<Object>);
-  if (v.checkedParameters) {
-    Expect.throwsTypeError(() => f(f));
-    Expect.throwsTypeError(() => f(42));
-  }
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => f(f));
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => f(42));
 
   Foo<Foo<int>> bazInt = baz; // implicit instantiation baz<int>
   f = bazInt;
   Expect.isTrue(f(bar));
   Expect.isFalse(f is Foo<int>);
 
-  if (v.checkedParameters) {
-    Expect.throwsTypeError(() => f(f));
-    Expect.throwsTypeError(() => f(42));
-  }
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => f(f));
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => f(42));
 }

tests/language/function_subtype/checked0_test.dart

@@ -30,11 +30,7 @@ class C<T> {
 
   void test(String nameOfT, bool expectedResult) {
     check(bool expectedResult, f()) {
-      if (!expectedResult) {
-        if (v.checkedParameters) Expect.throwsTypeError(f);
-      } else {
-        f();
-      }
+      Expect.throwsTypeErrorWhen(!expectedResult && v.checkedParameters, f);
     }
 
     dynamic foo = fooF, baz = bazF, boz = bozF;

tests/language/this/as_covariant_call_checks_test.dart

@@ -45,14 +45,11 @@ void loop(A<String> obj, bool violateType) {
 }
 
 void main() {
-  // TODO(sigmund): update to use a Requirements comment instead
-  if (!v.checkedParameters) return;
-
   A<num>().field = 10;
   final obj = A<String>();
   loop(obj, false);
   loop(obj, false);
-  Expect.throwsTypeError(() => obj.testMethod(true));
-  Expect.throwsTypeError(() => obj.testSetter(true));
-  Expect.throwsTypeError(() => obj.testField(true));
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => obj.testMethod(true));
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => obj.testSetter(true));
+  Expect.throwsTypeErrorWhen(v.checkedParameters, () => obj.testField(true));
 }