Skip to content

Commit

Permalink
Improve error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
fpagnoux committed Apr 19, 2019
1 parent c551fac commit 79ef47b
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 19 deletions.
2 changes: 1 addition & 1 deletion openfisca_core/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def __init__(self, path, message, code = None):
message = str(message).strip(os.linesep).replace(os.linesep, ' ')
dpath.util.new(self.error, dpath_path, message)
self.code = code
Exception.__init__(self, str(self.error).encode('utf-8'))
Exception.__init__(self, str(self.error))

def __str__(self):
return str(self.error)
Expand Down
8 changes: 4 additions & 4 deletions openfisca_core/simulations.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def calculate(self, variable_name, period, **parameters):
"""
population = self.get_variable_population(variable_name)
holder = population.get_holder(variable_name)
variable = self.tax_benefit_system.get_variable(variable_name)
variable = self.tax_benefit_system.get_variable(variable_name, check_existence = True)

if period is not None and not isinstance(period, periods.Period):
period = periods.period(period)
Expand Down Expand Up @@ -164,7 +164,7 @@ def purge_cache_of_invalid_values(self):
holder.delete_arrays(_period)

def calculate_add(self, variable_name, period, **parameters):
variable = self.tax_benefit_system.get_variable(variable_name)
variable = self.tax_benefit_system.get_variable(variable_name, check_existence = True)

if period is not None and not isinstance(period, periods.Period):
period = periods.period(period)
Expand All @@ -188,7 +188,7 @@ def calculate_add(self, variable_name, period, **parameters):
)

def calculate_divide(self, variable_name, period, **parameters):
variable = self.tax_benefit_system.get_variable(variable_name)
variable = self.tax_benefit_system.get_variable(variable_name, check_existence = True)

if period is not None and not isinstance(period, periods.Period):
period = periods.period(period)
Expand Down Expand Up @@ -450,7 +450,7 @@ def set_input(self, variable_name, period, value):
If a ``set_input`` property has been set for the variable, this method may accept inputs for periods not matching the ``definition_period`` of the variable. To read more about this, check the `documentation <https://openfisca.org/doc/coding-the-legislation/35_periods.html#automatically-process-variable-inputs-defined-for-periods-not-matching-the-definitionperiod>`_.
"""
variable = self.tax_benefit_system.get_variable(variable_name)
variable = self.tax_benefit_system.get_variable(variable_name, check_existence = True)
period = periods.period(period)
if ((variable.end is not None) and (period.start.date > variable.end)):
return
Expand Down
37 changes: 23 additions & 14 deletions openfisca_core/tools/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import sys
import os
import traceback
import textwrap

import pytest

Expand Down Expand Up @@ -86,7 +87,7 @@ def __init__(self, path, parent, tax_benefit_system, options):
def collect(self):
try:
tests = yaml.load(self.fspath.open(), Loader = Loader)
except (yaml.scanner.ScannerError, TypeError):
except (yaml.scanner.ScannerError, yaml.parser.ParserError, TypeError):
message = os.linesep.join([
traceback.format_exc(),
f"'{self.fspath}' is not a valid YAML file. Check the stack trace above for more details.",
Expand All @@ -97,7 +98,7 @@ def collect(self):
tests = [tests]
for test in tests:
if not self.should_ignore(test):
yield YamlItem(self.fspath.basename, self, self.tax_benefit_system, test, self.options)
yield YamlItem('', self, self.tax_benefit_system, test, self.options)

def should_ignore(self, test):
name_filter = self.options.get('name_filter')
Expand All @@ -119,13 +120,13 @@ def __init__(self, name, parent, baseline_tax_benefit_system, test, options):
self.tax_benefit_system = None

def parse_test(self):
name = self.test.get('name', '')
self.name = self.test.get('name', '')
if not self.test.get('output'):
raise ValueError("Missing key 'output' in test '{}' in file '{}'".format(name, self.fspath))
raise ValueError("Missing key 'output' in test '{}' in file '{}'".format(self.name, self.fspath))

if not TEST_KEYWORDS.issuperset(self.test.keys()):
unexpected_keys = set(self.test.keys()).difference(TEST_KEYWORDS)
raise ValueError("Unexpected keys {} in test '{}' in file '{}'".format(unexpected_keys, name, self.fspath))
raise ValueError("Unexpected keys {} in test '{}' in file '{}'".format(unexpected_keys, self.name, self.fspath))

self.tax_benefit_system = _get_tax_benefit_system(self.baseline_tax_benefit_system, self.test.get('reforms', []), self.test.get('extensions', []))

Expand All @@ -137,14 +138,8 @@ def parse_test(self):
builder.set_default_period(period)
self.simulation = builder.build_from_dict(self.tax_benefit_system, input)
self.simulation.trace = verbose
except SituationParsingError as error:
message = os.linesep.join([
traceback.format_exc(),
str(error.error),
os.linesep,
"Could not parse situation described in test '{}' in YAML file '{}'. Check the stack trace above for more details.".format(name, self.fspath),
])
raise ValueError(message)
except (VariableNotFound, SituationParsingError):
raise
except Exception as e:
error_message = os.linesep.join([str(e), '', f"Unexpected error raised while parsing '{self.fspath}'"])
raise ValueError(error_message).with_traceback(sys.exc_info()[2]) from e # Keep the stack trace from the root error
Expand Down Expand Up @@ -195,7 +190,7 @@ def check_variable(self, variable_name, expected_value, period, entity_index = N
actual_value,
expected_value,
absolute_error_margin = self.test.get('absolute_error_margin'),
message = f"In test '{self.test.get('name')}', in file '{self.fspath}', {variable_name}@{period}: ",
message = f"{variable_name}@{period}: ",
relative_error_margin = self.test.get('relative_error_margin'),
)

Expand All @@ -207,6 +202,20 @@ def should_ignore_variable(self, variable_name):

return variable_ignored or variable_not_tested

def repr_failure(self, excinfo):
if not isinstance(excinfo.value, (AssertionError, VariableNotFound, SituationParsingError)):
return super(YamlItem, self).repr_failure(excinfo)

message = excinfo.value.args[0]
if isinstance(excinfo.value, SituationParsingError):
message = f"Could not parse situation described: {message}"

return os.linesep.join([
f"{str(self.fspath)}:",
f" Test '{str(self.name)}':",
textwrap.indent(message, ' ')
])


class OpenFiscaPlugin(object):

Expand Down

0 comments on commit 79ef47b

Please sign in to comment.