Skip to content

Commit

Permalink
Strip ### from tests (#260)
Browse files Browse the repository at this point in the history
So we actually test error messages, not source code: both rust and
java includes snippet in error message, so before this commit error
messages were not tested on java and rust.

This commit is probably the most wasteful time I spent in the last
year. Starting to doubt these tests do more good than harm.
  • Loading branch information
stepancheg committed Aug 8, 2023
1 parent e5bfa95 commit 5704ba4
Show file tree
Hide file tree
Showing 16 changed files with 261 additions and 92 deletions.
35 changes: 24 additions & 11 deletions test_suite/starlark_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import sys
import typing
import unittest
import tempfile
import subprocess
Expand All @@ -8,11 +9,14 @@

import testenv

def indent(s: str) -> str:
return "".join(" " + line.rstrip("\n") + "\n" for line in s.splitlines())

class StarlarkTest(unittest.TestCase):
CHUNK_SEP = "---"
seen_error = False

def chunks(self, path):
def chunks(self, path) -> typing.Iterable[typing.Tuple[typing.List[str], typing.List[str], int]]:
code = []
expected_errors = []
# Current line no
Expand All @@ -22,20 +26,23 @@ def chunks(self, path):
with open(path, mode="rb") as f:
for line in f:
line_no += 1
line = line.decode("utf-8")
if line.strip() == self.CHUNK_SEP:
line = line.decode("utf-8").rstrip()
if line == self.CHUNK_SEP:
yield code, expected_errors, test_line_no
expected_errors = []
code = []
test_line_no = line_no + 1
else:
m = re.search("### *((go|java|rust):)? *(.*)", line)
m = re.fullmatch("(.*?) *### *((go|java|rust):)? *(.*)", line)
if m:
error_impl = m.group(2)
error_impl = m.group(3)
assert error_impl is None or error_impl in ["go", "java", "rust"]
if (not error_impl) or error_impl == impl:
expected_errors.append(m.group(3))
code.append(line)
expected_errors.append(m.group(4))
code.append(m.group(1))
else:
code.append(line)
code.append("\n")
assert len(expected_errors) <= 1
yield code, expected_errors, test_line_no

Expand All @@ -49,13 +56,18 @@ def evaluate(self, f):
else:
return stdout + stderr

def mark_error(self):
if self.seen_error:
print()
self.seen_error = True

def check_output(self, output, expected, line_no):
if expected and not output:
self.seen_error = True
self.mark_error()
print("Test L{}: expected error: {}".format(line_no, expected))

if output and not expected:
self.seen_error = True
self.mark_error()
print("Test L{}: unexpected error: {}".format(line_no, output))

output_ = output.lower()
Expand All @@ -66,9 +78,10 @@ def check_output(self, output, expected, line_no):
# because error message contains source snippet.
# Fix it by removing `###` before evaluating.
if exp not in output_ and not re.search(exp, output_):
self.seen_error = True
self.mark_error()
print("Test L{}: error not found: `{}`".format(line_no, exp.encode('utf-8')))
print("Got: `{}`".format(output.encode('utf-8')))
print("Got:")
print(indent(output), end="")

PRELUDE = """
def assert_eq(x, y):
Expand Down
19 changes: 14 additions & 5 deletions test_suite/testdata/go/builtins.star
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,20 @@ assert_eq(sorted(["two", "three", "four"], key=len, reverse=True),
["three", "four", "two"])

---
sorted(1) ### (for parameter iterable: got int, want iterable|not a collection|not iterable|operation.*not supported)
---
sorted([1, 2, None, 3]) ### (not implemented|cannot compare|operation.*not supported)
---
sorted([1, "one"]) ### (string < int not implemented|cannot compare|operation.*not supported)
### java: Error in sorted
### rust: not supported
### go: Error in sorted
sorted(1)
---
### java: unsupported
### rust: not supported
### go: Error in sorted
sorted([1, 2, None, 3])
---
### java: unsupported
### rust: not supported
### go: Error in sorted
sorted([1, "one"])
---
# _inconsistency_: java accepts key to be None
# sorted([1, 2, 3], key=None) ## (want callable|not supported)
Expand Down
2 changes: 1 addition & 1 deletion test_suite/testdata/go/dict.star
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ setIndex(x9, [], 2) ### (unhashable|not hashable)
---

x9a = {}
x9a[1, 2] = 3 ### rust: TODO(stepancheg): checked incorrectly on rust because error message includes snippet
x9a[1, 2] = 3 ### rust: left-hand-side of assignment must take
assert_eq(x9a.keys()[0], (1, 2))

---
Expand Down
6 changes: 5 additions & 1 deletion test_suite/testdata/go/function.star
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ def f(a, b, c, d, e, f, g, h,
mm):
pass

### rust: more than once
### java: multiple values
### go: multiple values

f(
1, 2, 3, 4, 5, 6, 7, 8,
9, 10, 11, 12, 13, 14, 15, 16,
Expand All @@ -183,7 +187,7 @@ f(
41, 42, 43, 44, 45, 46, 47, 48,
49, 50, 51, 52, 53, 54, 55, 56,
57, 58, 59, 60, 61, 62, 63, 64, 65,
mm = 100) ### (multiple values|argument 'mm' passed both by position and by name|Extraneous parameter|occurs both explicitly and in \*\*kwargs)
mm = 100)

---
# Regression test for github.com/google/starlark-go/issues/21,
Expand Down
12 changes: 6 additions & 6 deletions test_suite/testdata/go/int.star
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,15 @@ assert_eq(int("0b00000", 0), 0)
assert_eq(11111 * 11111, 123454321)

---
int("0x123", 8) ### (invalid literal.*base 8|not a base 8|not a valid number in base 8)
int("0x123", 8) ### base
---
int("-0x123", 8) ### (invalid literal.*base 8|not a base 8|not a valid number in base 8)
int("-0x123", 8) ### base
---
int("0o123", 16) ### (invalid literal.*base 16|not a base 16|not a valid number in base 16)
int("0o123", 16) ### base
---
int("-0o123", 16) ### (invalid literal.*base 16|not a base 16|not a valid number in base 16)
int("-0o123", 16) ### base
---
int("0x110", 2) ### (invalid literal.*base 2|not a base 2|not a valid number in base 2)
int("0x110", 2) ### base
---
# int from string, auto detect base
assert_eq(int("123", 0), 123)
Expand All @@ -194,7 +194,7 @@ assert_eq(int("-0o123", 0), -83)


# github.com/google/starlark-go/issues/108
int("0Oxa", 8) ### (invalid literal|not a base 8|not a valid number in base 8)
int("0Oxa", 8) ### base
---

# _inconsistency_: some implementations allow some of these conversions
Expand Down
40 changes: 32 additions & 8 deletions test_suite/testdata/go/misc.star
Original file line number Diff line number Diff line change
@@ -1,21 +1,45 @@
# Miscellaneous tests of Starlark evaluation.

# Ordered comparisons require values of the same type.
None < False ### (not impl|cannot compare|operation.*not supported)
### java: unsupported
### go: not implemented
### rust: not supported
None < False
---
False < list ### (not impl|cannot compare|operation.*not supported)
### java: unsupported
### go: not implemented
### rust: not supported
False < list
---
list < {} ### (not impl|cannot compare|operation.*not supported)
### java: unsupported
### go: not implemented
### rust: not supported
list < {}
---
{} < None ### (not impl|cannot compare|operation.*not supported)
### java: unsupported
### go: not implemented
### rust: not supported
{} < None
---
None < 0 ### (not impl|cannot compare|operation.*not supported)
### java: unsupported
### go: not implemented
### rust: not supported
None < 0
---
0 < [] ### (not impl|cannot compare|operation.*not supported)
### java: unsupported
### go: not implemented
### rust: not supported
0 < []
---
[] < "" ### (not impl|cannot compare|operation.*not supported)
### java: unsupported
### go: not implemented
### rust: not supported
[] < ""
---
"" < () ### (not impl|cannot compare|operation.*not supported)
### java: unsupported
### go: not implemented
### rust: not supported
"" < ()
---

# _inconsistency_: cyclic data structures not supported in Rust and Java
Expand Down
49 changes: 38 additions & 11 deletions test_suite/testdata/go/string.star
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,10 @@ assert_eq("banana"[4::-2], "nnb")
assert_eq("banana"[::-1], "ananab")
assert_eq("banana"[None:None:-2], "aaa")
---
"banana"[:"":] ### (got.*want|parameters mismatch)
### rust: not supported
### java: want int
### go: want int
"banana"[:"":]
---
"banana"[:"":True] ### (got.*want|parameters mismatch)
---
Expand Down Expand Up @@ -374,7 +377,10 @@ assert_('abc'.startswith(('a', 'A')))
assert_('ABC'.startswith(('a', 'A')))
assert_(not 'ABC'.startswith(('b', 'B')))
---
'123'.startswith((1, 2)) ### (got int, for element 0|type of parameter.*doesn't match)
### rust: Type of parameter
### java: want string
### go: want string
'123'.startswith((1, 2))
---
# _inconsistency_: rust startswith allows a list, not just tuple
# https://github.com/facebookexperimental/starlark-rust/issues/23
Expand All @@ -384,7 +390,10 @@ assert_('abc'.endswith(('c', 'C')))
assert_('ABC'.endswith(('c', 'C')))
assert_(not 'ABC'.endswith(('b', 'B')))
---
'123'.endswith((1, 2)) ### (got int, for element 0|type of parameter.*doesn't match)
### java: want string
### rust: Type of parameter
### go: want string
'123'.endswith((1, 2))
---
# _inconsistency_: rust endswith allows a lists, not just tuple
# https://github.com/facebookexperimental/starlark-rust/issues/23
Expand Down Expand Up @@ -464,16 +473,25 @@ def args(*args): return args
args(*"abc") ### (must be iterable, not string|not iterable|must be an iterable|operation.*not supported)
---
# list(str)
list("abc") ### (got string, want iterable|not iterable|not a collection|operation.*not supported)
### java: got value of type
### rust: not supported
### go: got string, want iterable
list("abc")
---
# tuple(str)
tuple("abc") ### (got string, want iterable|not iterable|not a collection|operation.*not supported)
### java: got value of type
### rust: not supported
### go: got string, want iterable
tuple("abc")
---
# enumerate
enumerate("ab") ### (got string, want iterable|not iterable|expected value of type 'sequence'|operation.*not supported)
---
# sorted
sorted("abc") ### (got string, want iterable|not iterable|not a collection|operation.*not supported)
### java: got value of type
### rust: not supported
### go: got string, want iterable
sorted("abc")
---
# list.extend
[].extend("bc") ### (got string, want iterable|not iterable|expected value of type 'sequence'|operation.*not supported)
Expand Down Expand Up @@ -552,8 +570,17 @@ assert_eq("¿Por qué?".title(), "¿Por Qué?")

---
# method spell check
"".starts_with() ### (no .starts_with field.*did you mean .startswith|not supported|no field or method)
---
"".StartsWith() ### (no .StartsWith field.*did you mean .startswith|not supported|no field or method)
---
"".fin() ### (no .fin field.*.did you mean .find|not supported|no field or method)
### rust: has no attribute
### java: has no field or method
### go: field or method
"".starts_with()
---
### rust: has no attribute
### java: has no field or method
### go: field or method
"".StartsWith()
---
### rust: has no attribute
### java: has no field or method
### go: field or method
"".fin()
5 changes: 4 additions & 1 deletion test_suite/testdata/go/tuple.star
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ assert_eq(tuple(["a", "b", "c"]), ("a", "b", "c"))
assert_eq(tuple(["a", "b", "c"]), ("a", "b", "c"))
assert_eq(tuple([1]), (1,))
---
tuple(1) ### (got int, want iterable|not iterable|not a collection|operation.*not supported)
### go: want iterable
### java: got value of type
### rust: not supported
tuple(1)
---

# tuple * int, int * tuple
Expand Down
2 changes: 1 addition & 1 deletion test_suite/testdata/java/int.star
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,4 @@ compound()
---
1 // 0 ### (division by zero|divide by zero)
---
1 % 0 ### (integer modulo by zero|division by zero|divide by zero)
1 % 0 ### by zero
16 changes: 8 additions & 8 deletions test_suite/testdata/java/int_constructor.star
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,23 @@ assert_eq(int('0XFF', 0), 255)
assert_eq(int('0xFF', 16), 255)

---
int('1.5') ### (invalid literal|not a base 10|not a valid number in base 10)
int('1.5') ### base
---
int('ab') ### (invalid literal|not a base 10|not a valid number in base 10)
int('ab') ### base
---
int(None) ### None
---
int('123', 3) ### (invalid literal|not a base 3|not a valid number in base 3)
int('123', 3) ### base
---
int('FF', 15) ### (invalid literal|not a base 15|not a valid number in base 15)
int('FF', 15) ### base
---
int('123', -1) ### >= 2 (and|&&) <= 36
int('123', -1) ### base
---
int('123', 1) ### >= 2 (and|&&) <= 36
int('123', 1) ### base
---
int('123', 37) ### >= 2 (and|&&) <= 36
int('123', 37) ### base
---
int('0xFF', 8) ### (invalid literal|not a base 8|not a valid number in base 8)
int('0xFF', 8) ### base
---
int(True, 2) ### (can't convert non-string with explicit base|non-string)
---
Expand Down
Loading

0 comments on commit 5704ba4

Please sign in to comment.