From 07f092e7133e811dcda9d6eac11115ee1fbbee6b Mon Sep 17 00:00:00 2001 From: adonovan Date: Wed, 21 Oct 2020 07:28:26 -0700 Subject: [PATCH] starlark: improve expectation hygiene in ScriptTest 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 --- .../net/starlark/java/eval/ScriptTest.java | 46 ++++++++++++------- .../net/starlark/java/eval/testdata/int.sky | 2 +- .../java/eval/testdata/int_constructor.sky | 4 +- .../net/starlark/java/eval/testdata/json.sky | 16 +++---- .../java/eval/testdata/list_mutation.sky | 2 +- .../java/eval/testdata/list_slices.sky | 10 ++-- .../net/starlark/java/eval/testdata/loop.sky | 28 +++++------ .../java/eval/testdata/string_format.sky | 4 +- 8 files changed, 63 insertions(+), 49 deletions(-) diff --git a/src/test/java/net/starlark/java/eval/ScriptTest.java b/src/test/java/net/starlark/java/eval/ScriptTest.java index bd8578e749450e..f2223273889640 100644 --- 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 expectations = new HashMap<>(); - for (int i = 0; true; i += "###".length()) { - i = chunk.indexOf("###", i); - if (i < 0) { - break; - } + // extract expectations: ### "regular expression" + Map 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 e : expectations.entrySet()) { + for (Map.Entry 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 expectations, String message) { - for (String pattern : expectations.keySet()) { - if (message.contains(pattern) || message.matches(".*" + pattern + ".*")) { + private static boolean expected(Map 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 index 59e517bc59ea5e..91affcde6e6b57 100644 --- 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 index fd64fc66000444..0851fe4480a968 100644 --- 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 index c287af27b72cc4..7638a3984bddda 100644 --- 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 index d9e5865d12bc2b..04b53cf9f5daa4 100644 --- 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 index 711e6a3ea5f741..2ccc84d00c68ee 100644 --- 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 index 1ef38c104de8dc..4746c3fbbe7a86 100644 --- 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 index e626a56e09c0a2..af91f7924d2a0b 100644 --- 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 ---