Skip to content

Commit

Permalink
Do expected error checking in (Check|Infer)WithErrors.
Browse files Browse the repository at this point in the history
Now that we have comment-based error checking, I really wanted to be able to
insert expected errors into test code and have them checked without needing to
call a separate assert method afterwards. This CL:

* Fixes a couple cases where errors were reported in a non-deterministic order,
  so that a mark can always be associated with the right error.
* Has CheckWithErrors and InferWithErrors call assert_errors_match_expected()
  on the errorlog.
* Forbids adding expected error comments to Check, Infer, and InferFromFile, so
  that we don't accidentally use one of these methods and have our expected
  errors silently ignored.
* Replaces assertErrorLogIs and assertErrorsMatch with an assertErrorRegexes
  method that only has to be called when we want to check marks.
* Makes some simplifications to the line number handling and error matching
  code that are possible because of the removal of assertErrorLogIs.

On its own, this CL does not pass tests; a follow-up will migrate all uses of
assertErrorLogIs over to assertErrorRegexes. I'll wait until both CLs have been
reviewed, then submit them in quick succession.

PiperOrigin-RevId: 303863481
  • Loading branch information
rchen152 committed Apr 2, 2020
1 parent b8fdc56 commit f01905b
Show file tree
Hide file tree
Showing 6 changed files with 202 additions and 117 deletions.
5 changes: 3 additions & 2 deletions pytype/abstract.py
Expand Up @@ -2491,7 +2491,7 @@ def update_sig(method):
if nitem in self.template:
raise abstract_utils.GenericTypeError(
self, ("Generic class [%s] and its nested generic class [%s] "
"cannot use same type variable %s.")
"cannot use the same type variable %s.")
% (self.full_name, cls.full_name, item.name))

self._load_all_formal_type_parameters() # Throw exception if there is error
Expand Down Expand Up @@ -3010,7 +3010,8 @@ def _inner_cls_check(self, last_frame):
inner_cls_types = value.collect_inner_cls_types()
inner_cls_types.update([(value, item.with_module(None))
for item in value.template])
for cls, item in inner_cls_types:
# Report errors in a deterministic order.
for cls, item in sorted(inner_cls_types, key=lambda typ: typ[1].name):
if item in all_type_parameters:
self.vm.errorlog.invalid_annotation(
self.vm.simple_stack(self.get_first_opcode()), item,
Expand Down
8 changes: 7 additions & 1 deletion pytype/annotations_util.py
@@ -1,5 +1,7 @@
"""Utilities for inline type annotations."""

import sys

from pytype import abstract
from pytype import abstract_utils
from pytype import mixin
Expand Down Expand Up @@ -157,7 +159,11 @@ def convert_annotations_list(self, node, annotations_list):
def convert_class_annotations(self, node, raw_annotations):
"""Convert a name -> raw_annot dict to annotations."""
annotations = {}
for name, t in raw_annotations.items():
raw_items = raw_annotations.items()
if sys.version_info.major == 2:
# Make sure annotation errors are reported in a deterministic order.
raw_items = sorted(raw_items)
for name, t in raw_items:
# Don't use the parameter name, since it's often something unhelpful
# like `0`.
annot = self._process_one_annotation(
Expand Down
1 change: 1 addition & 0 deletions pytype/tests/CMakeLists.txt
Expand Up @@ -18,6 +18,7 @@ py_test(
test_base_test.py
DEPS
.test_base
pytype.utils
)

py_test(
Expand Down
39 changes: 24 additions & 15 deletions pytype/tests/test_base.py
@@ -1,13 +1,13 @@
"""Common methods for tests of analyze.py."""

import logging
import re
import sys
import textwrap

from pytype import analyze
from pytype import config
from pytype import directors
from pytype import errors
from pytype import load_pytd
from pytype import utils
from pytype.pyi import parser
Expand Down Expand Up @@ -52,7 +52,10 @@ def _Wrapper(self, code, *args, **kwargs):
def _IncrementLineNumbersPy2(func):
def _Wrapper(self, errorlog, expected_errors):
if self.options.python_version == (2, 7):
expected_errors = errorlog.increment_line_numbers(expected_errors)
for mark in expected_errors:
expected_errors[mark] = re.sub(
r"line (\d+)",
lambda m: "line %d" % (int(m.group(1)) + 1), expected_errors[mark])
return func(self, errorlog, expected_errors)
return _Wrapper

Expand Down Expand Up @@ -169,20 +172,21 @@ def ConfigureOptions(self, **kwargs):
python_exe=utils.get_python_exe(self.options.python_version))

# For historical reasons (byterun), this method name is snakecase:
# TODO(kramm): Rename this function.
# pylint: disable=invalid-name
def Check(self, code, pythonpath=(), skip_repeat_calls=True,
report_errors=True, filename=None, quick=False, **kwargs):
"""Run an inference smoke test for the given code."""
self.ConfigureOptions(skip_repeat_calls=skip_repeat_calls,
pythonpath=pythonpath, quick=quick)
errorlog = errors.ErrorLog()
try:
src = ""
if six.PY3:
src = textwrap.dedent(code)
else:
src = textwrap.dedent(code.decode("utf-8"))
errorlog = test_utils.TestErrorLog(code)
if errorlog.expected:
self.fail("Cannot assert errors with Check(); use CheckWithErrors()")
analyze.check_types(
src, filename, loader=self.loader,
errorlog=errorlog, options=self.options, **kwargs)
Expand Down Expand Up @@ -211,19 +215,26 @@ def InferWithErrors(self, code, deep=True, pythonpath=(),
unit.Visit(visitors.VerifyVisitor())
unit = optimize.Optimize(unit, builtins_pytd, lossy=False, use_abcs=False,
max_union=7, remove_mutable=False)
return pytd_utils.CanonicalOrdering(unit), kwargs["errorlog"]
errorlog = kwargs["errorlog"]
errorlog.assert_errors_match_expected()
return pytd_utils.CanonicalOrdering(unit), errorlog

def CheckWithErrors(self, code, deep=True, pythonpath=(),
analyze_annotated=True, quick=False, **kwargs):
kwargs.update(
self._SetUpErrorHandling(code, pythonpath, analyze_annotated, quick))
analyze.check_types(filename="<inline>", deep=deep, **kwargs)
return kwargs["errorlog"]
errorlog = kwargs["errorlog"]
errorlog.assert_errors_match_expected()
return errorlog

def InferFromFile(self, filename, pythonpath):
with open(filename, "r") as fi:
code = fi.read()
errorlog = errors.ErrorLog()
errorlog = test_utils.TestErrorLog(code)
if errorlog.expected:
self.fail(
"Cannot assert errors with InferFromFile(); use InferWithErrors()")
self.ConfigureOptions(
module_name=load_pytd.get_module_name(filename, pythonpath),
pythonpath=pythonpath)
Expand Down Expand Up @@ -329,12 +340,8 @@ def assertIsIdentity(self, func):
self.assertEqual(param1.type, sig.return_type,
"Not identity: %r" % pytd_utils.Print(func))

def assertErrorsMatch(self, errorlog, expected_errors):
expected = errorlog.make_expected_errors(expected_errors)
self.assertErrorLogIs(errorlog, expected)

def assertErrorLogIs(self, errorlog, expected_errors):
errorlog.assert_expected_errors(expected_errors)
def assertErrorRegexes(self, errorlog, expected_errors):
errorlog.assert_error_regexes(expected_errors)

def _Pickle(self, ast, module_name):
assert module_name
Expand Down Expand Up @@ -383,7 +390,9 @@ def _InferAndVerify(
module_name=module_name, quick=quick, use_pickled_files=True,
pythonpath=[""] if (not pythonpath and imports_map) else pythonpath,
imports_map=imports_map, analyze_annotated=analyze_annotated)
errorlog = errors.ErrorLog()
errorlog = test_utils.TestErrorLog(src)
if errorlog.expected:
self.fail("Cannot assert errors with Infer(); use InferWithErrors()")
unit, builtins_pytd = analyze.infer_types(
src, errorlog, self.options, loader=self.loader, **kwargs)
unit.Visit(visitors.VerifyVisitor())
Expand Down Expand Up @@ -427,7 +436,7 @@ def assertTypesMatchPytd(self, ty, pytd_src):
CheckWithErrors = _AddAnnotationsImportPy2(CheckWithErrors)
Infer = _AddAnnotationsImportPy2(Infer)
InferWithErrors = _AddAnnotationsImportPy2(InferWithErrors)
assertErrorLogIs = _IncrementLineNumbersPy2(assertErrorLogIs)
assertErrorRegexes = _IncrementLineNumbersPy2(assertErrorRegexes)


class TargetIndependentTest(BaseTest):
Expand Down
112 changes: 102 additions & 10 deletions pytype/tests/test_base_test.py
@@ -1,31 +1,123 @@
"""Tests for our test framework."""

from pytype import file_utils
from pytype import utils
from pytype.tests import test_base
import six
from pytype.tests import test_utils


class ErrorLogTest(test_base.TargetIndependentTest):

def _lineno(self, line):
if self.options.python_version == (2, 7) and utils.USE_ANNOTATIONS_BACKPORT:
return line + 1
return line

def test_error_comments(self):
err = self.CheckWithErrors("""\
a = 10 # a random comment
b = "hello" # .mark
c = a + b # some-error
d = a + b # .another_mark
b = "hello" + 3 # unsupported-operands[.mark]
c = (10).foo # attribute-error
d = int(int) # wrong-arg-types[.another_mark]
""")
self.assertEqual(
{mark: (e.lineno, e.name) for mark, e in err.marks.items()},
{".mark": (self._lineno(2), "unsupported-operands"),
".another_mark": (self._lineno(4), "wrong-arg-types")})
self.assertEqual(err.expected, {
self._lineno(2): [("unsupported-operands", ".mark")],
self._lineno(3): [("attribute-error", None)],
self._lineno(4): [("wrong-arg-types", ".another_mark")]})

def test_multiple_errors_one_line(self):
err = self.CheckWithErrors("""\
x = (10).foo, "hello".foo # attribute-error[e1] # attribute-error[e2]
""")
six.assertCountEqual(self, err.marks.keys(), [".mark", ".another_mark"])
self.assertEqual(err.marks[".mark"], 2)
six.assertCountEqual(self, err.expected.keys(), [3])
six.assertCountEqual(self, err.expected[3], "some-error")
line = self._lineno(1)
self.assertEqual(err.expected, {line: [("attribute-error", "e1"),
("attribute-error", "e2")]})
self.assertCountEqual(err.marks, ["e1", "e2"])
self.assertIn("on int", err.marks["e1"].message)
self.assertIn("on str", err.marks["e2"].message)

def test_populate_marks(self):
# Test that assert_error_regexes populates self.marks if not already done.
errorlog = test_utils.TestErrorLog("x = 0")
self.assertIsNone(errorlog.marks)
self.assertErrorRegexes(errorlog, {})
self.assertIsNotNone(errorlog.marks)

def test_duplicate_mark(self):
with self.assertRaises(AssertionError) as ctx:
self.CheckWithErrors("x = 0 # attribute-error[e] # attribute-error[e]")
self.assertEqual(str(ctx.exception), "Mark e already used")

def test_error_matching(self):
err = self.CheckWithErrors("""\
a = 10
b = "hello"
c = a + b # unsupported-operands
d = a.foo() # .mark
d = a.foo() # attribute-error[.mark]
""")
self.assertErrorsMatch(err, [(".mark", "attribute-error", ".*foo.*")])
self.assertErrorRegexes(err, {".mark": ".*foo.*"})

def test_mismatched_error(self):
with self.assertRaises(AssertionError) as ctx:
self.CheckWithErrors("(10).foo # wrong-arg-types")
self.assertIn("Error does not match", str(ctx.exception))

def test_unexpected_error(self):
with self.assertRaises(AssertionError) as ctx:
self.CheckWithErrors("""\
(10).foo # attribute-error
"hello".foo
""")
self.assertIn("Unexpected error", str(ctx.exception))

def test_leftover_error(self):
with self.assertRaises(AssertionError) as ctx:
self.CheckWithErrors("x = 0 # attribute-error")
self.assertIn("Errors not found", str(ctx.exception))

def test_misspelled_leftover_error(self):
with self.assertRaises(AssertionError) as ctx:
self.CheckWithErrors("x = 0 # misspelled-error")
self.assertIn("Errors not found", str(ctx.exception))

def test_mismatched_regex(self):
err = self.CheckWithErrors("(10).foo # attribute-error[e]")
with self.assertRaises(AssertionError) as ctx:
self.assertErrorRegexes(err, {"e": r"does not match error message"})
self.assertIn("Bad error message", str(ctx.exception))

def test_missing_regex(self):
err = self.CheckWithErrors("(10).foo # attribute-error[e]")
with self.assertRaises(AssertionError) as ctx:
self.assertErrorRegexes(err, {})
self.assertEqual(str(ctx.exception), "No regex for mark e")

def test_leftover_regex(self):
err = self.CheckWithErrors("x = 0")
with self.assertRaises(AssertionError) as ctx:
self.assertErrorRegexes(err, {"e": ""})
self.assertEqual(str(ctx.exception), "Marks not found in code: e")

def test_bad_check(self):
with self.assertRaises(AssertionError) as ctx:
self.Check("name_error # name-error")
self.assertIn("Cannot assert errors", str(ctx.exception))

def test_bad_infer(self):
with self.assertRaises(AssertionError) as ctx:
self.Infer("name_error # name-error")
self.assertIn("Cannot assert errors", str(ctx.exception))

def test_bad_infer_from_file(self):
with file_utils.Tempdir() as d:
d.create_file("some_file.py", "name_error # name-error")
with self.assertRaises(AssertionError) as ctx:
self.InferFromFile(filename=d["some_file.py"], pythonpath=[])
self.assertIn("Cannot assert errors", str(ctx.exception))


test_base.main(globals(), __name__ == "__main__")

0 comments on commit f01905b

Please sign in to comment.