starlark: improve expectation hygiene in ScriptTest

adonovan · copybara-github · commit 07f092e7133e · 2020-10-21T07:29:46.000-07:00
Prior to this change, ScriptTest inherited a fuzzy syntax for
expectations from its Python-based predecessor: the content
of a ### comment was interpreted either as a substring, or
as a regular expression.

This change causes it to interpret the content of the comment
as a regular expression, always. Some expectations were updated.

PiperOrigin-RevId: 338258986
diff --git a/src/test/java/net/starlark/java/eval/ScriptTest.java b/src/test/java/net/starlark/java/eval/ScriptTest.java
@@ -24,6 +24,8 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
 import net.starlark.java.annot.Param;
 import net.starlark.java.annot.StarlarkBuiltin;
 import net.starlark.java.annot.StarlarkMethod;
@@ -39,7 +41,11 @@ public final class ScriptTest {
   //
   // In each test file, chunks are separated by "\n---\n".
   // Each chunk is evaluated separately.
-  // Use "###" to specify the expected error.
+  // A comment containing
+  //     ### regular expression
+  // specifies an expected error on that line.
+  // The part after '###', with leading/trailing spaces removed,
+  // must be a valid regular expression matching the error.
   // If there is no "###", the test will succeed iff there is no error.
   //
   // Within the file, the assert_ and assert_eq functions may be used to
@@ -48,11 +54,8 @@ public final class ScriptTest {
 
   // TODO(adonovan): improve this test driver (following go.starlark.net):
   //
-  // - use a proper quotation syntax (Starlark string literals) in '### "foo"' expectations.
   // - extract support for "chunked files" into a library
   //   and reuse it for tests of lexer, parser, resolver.
-  // - don't interpret the pattern as "either a substring or a regexp".
-  //   Be consistent: always use regexp.
   // - require that some frame of each EvalException match the file/line of the expectation.
 
   interface Reporter {
@@ -120,19 +123,28 @@ public static void main(String[] args) throws Exception {
           System.err.printf("%s:%d: <<%s>>\n", file, linenum, buf);
         }
 
-        // extract "### string" expectations
-        Map<String, Integer> expectations = new HashMap<>();
-        for (int i = 0; true; i += "###".length()) {
-          i = chunk.indexOf("###", i);
-          if (i < 0) {
-            break;
-          }
+        // extract expectations: ### "regular expression"
+        Map<Pattern, Integer> expectations = new HashMap<>();
+        for (int i = chunk.indexOf("###"); i >= 0; i = chunk.indexOf("###", i)) {
           int j = chunk.indexOf("\n", i);
           if (j < 0) {
             j = chunk.length();
           }
-          String pattern = chunk.substring(i + 3, j).trim();
+
           int line = linenum + newlines(chunk.substring(0, i));
+          String comment = chunk.substring(i + 3, j);
+          i = j;
+
+          // Compile regular expression in comment.
+          Pattern pattern;
+          try {
+            pattern = Pattern.compile(comment.trim());
+          } catch (PatternSyntaxException ex) {
+            System.err.printf("%s:%d: invalid regexp: %s\n", file, line, ex.getMessage());
+            ok = false;
+            continue;
+          }
+
           if (false) {
             System.err.printf("%s:%d: expectation '%s'\n", file, line, pattern);
           }
@@ -172,6 +184,8 @@ public static void main(String[] args) throws Exception {
           // and expections match exactly. Furthermore, look only at errors
           // whose stack has a frame with a file/line that matches the expectation.
           // This requires inspecting EvalException stack.
+          // (There can be at most one dynamic error per chunk.
+          // Do we even need to allow multiple expectations?)
           if (!expected(expectations, ex.getMessage())) {
             System.err.println(ex.getMessageWithStack());
             ok = false;
@@ -187,7 +201,7 @@ public static void main(String[] args) throws Exception {
         }
 
         // unmatched expectations
-        for (Map.Entry<String, Integer> e : expectations.entrySet()) {
+        for (Map.Entry<Pattern, Integer> e : expectations.entrySet()) {
           System.err.printf("%s:%d: unmatched expectation: %s\n", file, e.getValue(), e.getKey());
           ok = false;
         }
@@ -214,9 +228,9 @@ private static void reportError(StarlarkThread thread, String message) {
     ok = false;
   }
 
-  private static boolean expected(Map<String, Integer> expectations, String message) {
-    for (String pattern : expectations.keySet()) {
-      if (message.contains(pattern) || message.matches(".*" + pattern + ".*")) {
+  private static boolean expected(Map<Pattern, Integer> expectations, String message) {
+    for (Pattern pattern : expectations.keySet()) {
+      if (pattern.matcher(message).find()) {
         expectations.remove(pattern);
         return true;
       }
diff --git a/src/test/java/net/starlark/java/eval/testdata/int.sky b/src/test/java/net/starlark/java/eval/testdata/int.sky
@@ -206,7 +206,7 @@ assert_eq(~((~(0x1010 << 100)) >> 100), 0x1010)
 ---
 1 & False ### unsupported binary operation: int & bool
 ---
-"a" ^ 5 ### unsupported binary operation: string ^ int
+"a" ^ 5 ### unsupported binary operation: string \^ int
 ---
 ~False ### unsupported unary operation: ~bool
 ---
diff --git a/src/test/java/net/starlark/java/eval/testdata/int_constructor.sky b/src/test/java/net/starlark/java/eval/testdata/int_constructor.sky
@@ -72,7 +72,7 @@ int('123', 3) ### invalid base-3 literal: "123"
 ---
 int('FF', 15) ### invalid base-15 literal: "FF"
 ---
-int('123', -1) ### invalid base -1 (want 2 <= base <= 36)
+int('123', -1) ### invalid base -1 \(want 2 <= base <= 36\)
 ---
 int('123', 1) ### invalid base 1 \(want 2 <= base <= 36\)
 ---
@@ -94,7 +94,7 @@ int('  42  ') ### invalid base-10 literal: "  42  "
 ---
 int('-') ### invalid base-10 literal: "-"
 ---
-int('+') ### invalid base-10 literal: "+"
+int('+') ### invalid base-10 literal: "\+"
 ---
 int('0x') ### invalid base-10 literal: "0x"
 ---
diff --git a/src/test/java/net/starlark/java/eval/testdata/json.sky b/src/test/java/net/starlark/java/eval/testdata/json.sky
@@ -101,13 +101,13 @@ json.decode('truefalse') ### at offset 4, unexpected character "f" after value
 ---
 json.decode('"abc') ### unclosed string literal
 ---
-json.decode('"ab\\gc"') ### invalid escape '\g'
+json.decode('"ab\\gc"') ### invalid escape '\\g'
 ---
 json.decode("'abc'") ### unexpected character "'"
 ---
 json.decode("1.2.3") ### invalid number: 1.2.3
 ---
-json.decode("+1") ### unexpected character "+"
+json.decode("+1") ### unexpected character "\+"
 ---
 json.decode("-abc") ### invalid number: -
 ---
@@ -119,7 +119,7 @@ json.decode("00") ### invalid number: 00
 ---
 json.decode("--1") ### invalid number: --1
 ---
-json.decode("-+1") ### invalid number: -+1
+json.decode("-+1") ### invalid number: -\+1
 ---
 json.decode("1e1e1") ### invalid number: 1e1e1
 ---
@@ -157,7 +157,7 @@ json.decode('{"one": 1') ### unexpected end of file
 ---
 json.decode('{"one" 1') ### after object key, got "1", want ':'
 ---
-json.decode('{"one": 1 "two": 2') ### in object, got "\"", want ',' or '}'
+json.decode('{"one": 1 "two": 2') ### in object, got "\\"", want ',' or '}'
 ---
 json.decode('{"x": 1, "x": 2}') ### object has duplicate key: "x"
 ---
@@ -172,13 +172,13 @@ json.decode('{"one": 1]') ### in object, got "]", want ',' or '}'
 json.decode('[' * 10000) ### nesting depth limit exceeded
 ---
 # Unescaped control codes (even tabs) are forbidden in strings.
-json.decode('"\t"') ### invalid character '\x09' in string literal
+json.decode('"\t"') ### invalid character '\\x09' in string literal
 ---
-json.decode('"\\u123"') ### incomplete \uXXXX escape
+json.decode('"\\u123"') ### incomplete \\uXXXX escape
 ---
-json.decode('"\\u123') ### incomplete \uXXXX escape
+json.decode('"\\u123') ### incomplete \\uXXXX escape
 ---
-json.decode('"\\u1') ### incomplete \uXXXX escape
+json.decode('"\\u1') ### incomplete \\uXXXX escape
 ---
 
 def codec(x):
diff --git a/src/test/java/net/starlark/java/eval/testdata/list_mutation.sky b/src/test/java/net/starlark/java/eval/testdata/list_mutation.sky
@@ -91,7 +91,7 @@ assert_eq(li3.pop(1), 3)
 assert_eq(li3, [2, 4])
 
 ---
-[1, 2].pop(3) ### index out of range (index is 3, but sequence has 2 elements)
+[1, 2].pop(3) ### index out of range \(index is 3, but sequence has 2 elements\)
 ---
 (1, 2).pop() ### 'tuple' value has no field or method 'pop'
 ---
diff --git a/src/test/java/net/starlark/java/eval/testdata/list_slices.sky b/src/test/java/net/starlark/java/eval/testdata/list_slices.sky
@@ -106,12 +106,12 @@ bananas.index("d", -1000, 1000) ### not found in list
 ---
 [[1], [2]]['a'] ### got string for sequence index, want int
 ---
-[0, 1, 2][3] ### index out of range (index is 3, but sequence has 3 elements)
+[0, 1, 2][3] ### index out of range \(index is 3, but sequence has 3 elements\)
 ---
-[0, 1, 2][-4] ### index out of range (index is -4, but sequence has 3 elements)
+[0, 1, 2][-4] ### index out of range \(index is -4, but sequence has 3 elements\)
 ---
-[0][-2] ### index out of range (index is -2, but sequence has 1 elements)
+[0][-2] ### index out of range \(index is -2, but sequence has 1 elements\)
 ---
-[0][1] ### index out of range (index is 1, but sequence has 1 elements)
+[0][1] ### index out of range \(index is 1, but sequence has 1 elements\)
 ---
-[][1] ### index out of range (index is 1, but sequence has 0 elements)
+[][1] ### index out of range \(index is 1, but sequence has 0 elements\)
diff --git a/src/test/java/net/starlark/java/eval/testdata/loop.sky b/src/test/java/net/starlark/java/eval/testdata/loop.sky
@@ -18,9 +18,9 @@ x, y = 1  ### got 'int' in sequence assignment
 ---
 
 # too few
-x, y = () ### too few values to unpack (got 0, want 2)
+x, y = () ### too few values to unpack \(got 0, want 2\)
 ---
-[x, y] = () ### too few values to unpack (got 0, want 2)
+[x, y] = () ### too few values to unpack \(got 0, want 2\)
 ---
 
 # just right
@@ -34,11 +34,11 @@ x, y = 1, 2
 # too many
 () = 1 ### got 'int' in sequence assignment
 ---
-() = (1,) ### too many values to unpack (got 1, want 0)
+() = (1,) ### too many values to unpack \(got 1, want 0\)
 ---
-x, y = 1, 2, 3 ### too many values to unpack (got 3, want 2)
+x, y = 1, 2, 3 ### too many values to unpack \(got 3, want 2\)
 ---
-[x, y] = 1, 2, 3 ### too many values to unpack (got 3, want 2)
+[x, y] = 1, 2, 3 ### too many values to unpack \(got 3, want 2\)
 ---
 
 # Assignment to empty tuple is permitted.
@@ -47,11 +47,11 @@ assert_eq([1 for [] in [(), []]], [1, 1])
 
 # Iterating over dict without .items() gives informative error.
 assert_eq([v for v in dict(a = "b")], ["a"])
-[None for () in dict(a = "b")]  ### got 'string' in sequence assignment (want 0-element sequence)
+[None for () in dict(a = "b")]  ### got 'string' in sequence assignment \(want 0-element sequence\)
 ---
-[None for (v1,) in dict(a = "b")] ### got 'string' in sequence assignment (want 1-element sequence)
+[None for (v1,) in dict(a = "b")] ### got 'string' in sequence assignment \(want 1-element sequence\)
 ---
-[None for v1, v2 in dict(a = "b")] ### got 'string' in sequence assignment (want 2-element sequence)
+[None for v1, v2 in dict(a = "b")] ### got 'string' in sequence assignment \(want 2-element sequence\)
 ---
 
 # --- list comprehensions ---
@@ -136,15 +136,15 @@ assert_eq([x + y for x, y in [(1, 2), (3, 4)]], [3, 7])
 assert_eq([z + t for (z, t) in [[1, 2], [3, 4]]], [3, 7])
 
 # multiple variables, fail
-[x + y for x, y, z in [(1, 2), (3, 4)]] ### too few values to unpack (got 2, want 3)
+[x + y for x, y, z in [(1, 2), (3, 4)]] ### too few values to unpack \(got 2, want 3\)
 ---
-[x + y for x, y in (1, 2)] ### got 'int' in sequence assignment (want 2-element sequence)
+[x + y for x, y in (1, 2)] ### got 'int' in sequence assignment \(want 2-element sequence\)
 ---
-[x + y for x, y, z in [(1, 2), (3, 4)]] ### too few values to unpack (got 2, want 3)
+[x + y for x, y, z in [(1, 2), (3, 4)]] ### too few values to unpack \(got 2, want 3\)
 ---
-[x + y for x, y in (1, 2)] ### got 'int' in sequence assignment (want 2-element sequence)
+[x + y for x, y in (1, 2)] ### got 'int' in sequence assignment \(want 2-element sequence\)
 ---
-[x + y for x, y, z in [(1, 2), (3, 4)]] ### too few values to unpack (got 2, want 3)
+[x + y for x, y, z in [(1, 2), (3, 4)]] ### too few values to unpack \(got 2, want 3\)
 ---
-[x2 + y2 for x2, y2 in (1, 2)] ### got 'int' in sequence assignment (want 2-element sequence)
+[x2 + y2 for x2, y2 in (1, 2)] ### got 'int' in sequence assignment \(want 2-element sequence\)
 ---
diff --git a/src/test/java/net/starlark/java/eval/testdata/string_format.sky b/src/test/java/net/starlark/java/eval/testdata/string_format.sky
@@ -54,9 +54,9 @@ assert_eq('{test} and {}'.format(2, test = 1), "1 and 2")
 assert_eq('{test} and {0}'.format(2, test = 1), "1 and 2")
 
 ---
-'{{}'.format(1) ### Found '}' without matching '{'
+'{{}'.format(1) ### Found '}' without matching '\{'
 ---
-'{}}'.format(1) ### Found '}' without matching '{'
+'{}}'.format(1) ### Found '}' without matching '\{'
 ---
 '{0}'.format() ### No replacement found for index 0
 ---
