Added better reporting on recompute failure (#207)

If the recomputation of a failed condition raised an exception, the user
was left in the dark which condition triggered the exception.

This patch adds a more informative message, so that the user can at
least inspect the problematic condition or send us a better bug report.

icontract/_checkers.py

@@ -182,7 +182,20 @@ def _create_violation_error(contract: Contract, resolved_kwargs: Mapping[str, An
     exception = None  # type: Optional[BaseException]
 
     if contract.error is None:
-        msg = icontract._represent.generate_message(contract=contract, condition_kwargs=condition_kwargs)
+        try:
+            msg = icontract._represent.generate_message(contract=contract, condition_kwargs=condition_kwargs)
+        except Exception as err:
+            parts = ["Failed to recompute the values of the contract condition:\n"]
+            if contract.location is not None:
+                parts.append("{}:\n".format(contract.location))
+
+            if contract.description is not None:
+                parts.append("{}: ".format(contract.description))
+
+            parts.append(icontract._represent.represent_condition(condition=contract.condition))
+
+            raise RuntimeError(''.join(parts)) from err
+
         exception = ViolationError(msg)
     elif inspect.ismethod(contract.error) or inspect.isfunction(contract.error):
         assert contract.error_arg_set is not None, ("Expected error_arg_set non-None if contract.error a function.")

icontract/_recompute.py

@@ -311,6 +311,9 @@ def visit_Call(self, node: ast.Call) -> Any:
         """Visit the function and the arguments and finally make the function call with them."""
         func = self.visit(node=node.func)
 
+        if not callable(func):
+            raise ValueError(("Unexpected call to a non-callle during the re-computation: {}").format(func))
+
         if inspect.iscoroutinefunction(func):
             raise ValueError(
                 ("Unexpected coroutine function {} as a condition of a contract. "

icontract/_represent.py

@@ -6,14 +6,14 @@
 import sys
 import textwrap
 import uuid
-from typing import Any, Mapping, MutableMapping, Callable, List, Dict, Iterable, cast  # pylint: disable=unused-import
-from typing import Optional, Tuple  # pylint: disable=unused-import
+from typing import Any, Mapping, MutableMapping, Callable, List, Dict, cast  # pylint: disable=unused-import
+from typing import Optional  # pylint: disable=unused-import
 
 import asttokens
 
 import icontract._recompute
-
 from icontract._types import Contract
+from icontract._globals import CallableT
 
 # pylint does not play with typing.Mapping.
 # pylint: disable=unsubscriptable-object
@@ -156,7 +156,7 @@ def visit_DictComp(self, node: ast.DictComp) -> None:
         self.generic_visit(node=node)
 
 
-def is_lambda(a_function: Callable[..., Any]) -> bool:
+def is_lambda(a_function: CallableT) -> bool:
     """
     Check whether the function is a lambda function.
 
@@ -458,6 +458,21 @@ def repr_values(condition: Callable[..., bool], lambda_inspection: Optional[Cond
     return parts
 
 
+def represent_condition(condition: CallableT) -> str:
+    """Represent the condition as a string."""
+    lambda_inspection = None  # type: Optional[ConditionLambdaInspection]
+    if not is_lambda(a_function=condition):
+        condition_repr = condition.__name__
+    else:
+        # We need to extract the source code corresponding to the decorator since inspect.getsource() is broken with
+        # lambdas.
+        lambda_inspection = inspect_lambda_condition(condition=condition)
+        assert lambda_inspection is not None, "Unexpected no lambda inspection for condition: {}".format(condition)
+        condition_repr = lambda_inspection.atok.get_text(lambda_inspection.node)
+
+    return condition_repr
+
+
 def generate_message(contract: Contract, condition_kwargs: Mapping[str, Any]) -> str:
     """Generate the message upon contract violation."""
     # pylint: disable=protected-access
@@ -475,18 +490,9 @@ def generate_message(contract: Contract, condition_kwargs: Mapping[str, Any]) ->
     else:
         # We need to extract the source code corresponding to the decorator since inspect.getsource() is broken with
         # lambdas.
-
-        # Find the line corresponding to the condition lambda
-        lines, condition_lineno = inspect.findsource(contract.condition)
-        filename = inspect.getsourcefile(contract.condition)
-        assert filename is not None
-
-        decorator_inspection = inspect_decorator(lines=lines, lineno=condition_lineno, filename=filename)
-        lambda_inspection = find_lambda_condition(decorator_inspection=decorator_inspection)
-
+        lambda_inspection = inspect_lambda_condition(condition=contract.condition)
         assert lambda_inspection is not None, \
-            "Expected lambda_inspection to be non-None if is_lambda is True on: {}".format(contract.condition)
-
+            "Unexpected no lambda inspection for condition: {}".format(contract.condition)
         condition_text = lambda_inspection.text
 
     parts.append(condition_text)

tests/test_for_integrators.py

@@ -223,3 +223,18 @@ def test_condition_text(self) -> None:
         self.assertEqual('OLD.last is not None or x == cumulative[-1]', lambda_inspection.text)
 
         assert isinstance(lambda_inspection.node, ast.Lambda)
+
+    def test_condition_representation(self) -> None:
+        checker = icontract._checkers.find_checker(func=func_with_contracts)
+        assert checker is not None
+
+        # Retrieve postconditions
+        contract = checker.__postconditions__[0]  # type: ignore
+        assert isinstance(contract, icontract._types.Contract)
+
+        text = icontract._represent.represent_condition(contract.condition)
+        self.assertEqual('lambda x, cumulative, OLD: OLD.last is not None or x == cumulative[-1]', text)
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_represent.py

@@ -1,8 +1,10 @@
 #!/usr/bin/env python3
 # pylint: disable=missing-docstring,invalid-name,too-many-public-methods,no-self-use
 # pylint: disable=unused-argument
+# pylint: disable=unnecessary-lambda
 
 import pathlib
+import re
 import reprlib
 import textwrap
 import unittest
@@ -362,13 +364,22 @@ def test_lambda(self) -> None:
         def func(x: int) -> int:
             return x
 
-        not_implemented_err = None  # type: Optional[NotImplementedError]
+        runtime_error = None  # type: Optional[RuntimeError]
         try:
             func(x=1)
-        except NotImplementedError as err:
-            not_implemented_err = err
+        except RuntimeError as err:
+            runtime_error = err
 
-        self.assertIsNotNone(not_implemented_err)
+        assert runtime_error is not None
+        assert runtime_error.__cause__ is not None
+        assert isinstance(runtime_error.__cause__, NotImplementedError)
+
+        not_implemented_error = runtime_error.__cause__
+
+        self.assertEqual(
+            'Recomputation of in-line lambda functions is not supported since it is quite tricky to implement and '
+            'we decided to implement it only once there is a real need for it. '
+            'Please make a feature request on https://github.com/Parquery/icontract', str(not_implemented_error))
 
     def test_generator_expression_with_attr_on_element(self) -> None:
         @icontract.ensure(lambda result: all(single_res[1].is_absolute() for single_res in result))
@@ -804,5 +815,45 @@ def some_func(arr: tests.mock.NumpyArray) -> None:
         self.assertEqual('The truth value of an array with more than one element is ambiguous.', str(value_err))
 
 
+class TestRecomputationFailure(unittest.TestCase):
+    def test_that_the_error_is_informative(self) -> None:
+        counter = 0
+
+        def some_condition() -> bool:
+            nonlocal counter
+            if counter == 0:
+                # The first time we return False, so that the pre-condition fails.
+                counter += 1
+                return False
+
+            # Every next time we raise the exception, so that the re-computation fails.
+            raise RuntimeError("Recomputation shall fail.")
+
+        @icontract.require(lambda: some_condition())
+        def some_func() -> None:
+            pass
+
+        runtime_error = None  # type: Optional[RuntimeError]
+        try:
+            some_func()
+        except RuntimeError as err:
+            runtime_error = err
+
+        assert runtime_error is not None
+
+        lines = [
+            re.sub(r'^File (.*), line ([0-9]+) in (.*):$',
+                   'File <erased path>, line <erased line> in <erased function>:', line)
+            for line in str(runtime_error).splitlines()
+        ]
+        text = '\n'.join(lines)
+
+        self.assertEqual(
+            textwrap.dedent('''\
+            Failed to recompute the values of the contract condition:
+            File <erased path>, line <erased line> in <erased function>:
+            lambda: some_condition()'''), text)
+
+
 if __name__ == '__main__':
     unittest.main()

tests_3_8/async/test_postcondition.py

@@ -119,21 +119,26 @@ async def some_func() -> None:
         self.assertIsNotNone(violation_error)
         self.assertEqual("hihi", str(violation_error))
 
-    async def test_reported_if_without_error(self) -> None:
+    async def test_reported_if_no_error_is_specified_as_we_can_not_recompute_coroutine_functions(self) -> None:
         async def some_condition() -> bool:
             return False
 
         @icontract.ensure(lambda: some_condition())
         async def some_func() -> None:
             pass
 
-        value_error = None  # type: Optional[ValueError]
+        runtime_error = None  # type: Optional[RuntimeError]
         try:
             await some_func()
-        except ValueError as err:
-            value_error = err
+        except RuntimeError as err:
+            runtime_error = err
+
+        assert runtime_error is not None
+        assert runtime_error.__cause__ is not None
+        assert isinstance(runtime_error.__cause__, ValueError)
+
+        value_error = runtime_error.__cause__
 
-        self.assertIsNotNone(value_error)
         self.assertRegex(
             str(value_error), r"^Unexpected coroutine function <function .*> as a condition of a contract\. "
             r"You must specify your own error if the condition of your contract is a coroutine function\.")

tests_3_8/async/test_precondition.py

@@ -93,21 +93,30 @@ async def some_func() -> None:
         self.assertIsNotNone(violation_error)
         self.assertEqual("hihi", str(violation_error))
 
-    async def test_reported_if_without_error(self) -> None:
+    async def test_reported_if_no_error_is_specified_as_we_can_not_recompute_coroutine_functions(self) -> None:
         async def some_condition() -> bool:
             return False
 
         @icontract.require(lambda: some_condition())
         async def some_func() -> None:
             pass
 
-        value_error = None  # type: Optional[ValueError]
+        runtime_error = None  # type: Optional[RuntimeError]
         try:
             await some_func()
-        except ValueError as err:
-            value_error = err
+        except RuntimeError as err:
+            runtime_error = err
+
+        assert runtime_error is not None
+        assert runtime_error.__cause__ is not None
+        assert isinstance(runtime_error.__cause__, ValueError)
+
+        value_error = runtime_error.__cause__
 
-        self.assertIsNotNone(value_error)
         self.assertRegex(
             str(value_error), r"^Unexpected coroutine function <function .*> as a condition of a contract\. "
             r"You must specify your own error if the condition of your contract is a coroutine function\.")
+
+
+if __name__ == "__main__":
+    unittest.main()