Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix TypeError on YAML test execution with trace option #1299

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion openfisca_core/tools/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ class YamlItem(pytest.Item):
def __init__(self, *, baseline_tax_benefit_system, test, options, **kwargs) -> None:
super().__init__(**kwargs)
self.baseline_tax_benefit_system = baseline_tax_benefit_system
self.options = options
self.test = build_test(test)
self.options = options
self.simulation = None
self.tax_benefit_system = None

Expand Down Expand Up @@ -258,6 +258,8 @@ def runtest(self) -> None:
finally:
tracer = self.simulation.tracer
if verbose:
if max_depth is None:
max_depth = sys.maxsize # Print trace full depth
self.print_computation_log(tracer, aggregate, max_depth)
if performance_graph:
self.generate_performance_graph(tracer)
Expand Down
6 changes: 3 additions & 3 deletions openfisca_core/tracers/computation_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ def print_log(self, aggregate: bool = False, max_depth: int = sys.maxsize) -> No
If ``max_depth`` is set, for example to ``3``, only print computed
vectors up to a depth of ``max_depth``.
"""
for _ in self.lines(aggregate, max_depth):
pass
for line in self.lines(aggregate, max_depth):
print(line) # noqa T201

def _get_node_log(
self,
node: t.TraceNode,
depth: int,
aggregate: bool,
max_depth: int = sys.maxsize,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default value removed as _get_node_log shouldn't be called outside openfisca_core (as its name starting with _ suggests). So wouldn't a change of the max_depth option value here produce unexpected behaviors for code calling the public openfisca_core?

max_depth: int,
) -> list[str]:
if depth > max_depth:
return []
Expand Down
88 changes: 77 additions & 11 deletions tests/core/tools/test_runner/test_yaml_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,30 @@ def __init__(self) -> None:


class TestItem(YamlItem):
def __init__(self, test) -> None:
super().__init__("", TestFile(), TaxBenefitSystem(), test, {})

self.tax_benefit_system = self.baseline_tax_benefit_system
self.simulation = Simulation()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed to call the default constructor. Simulation() used once only in test_variable_not_found.

"""
Mock a YamlItem for tests.

Usage example:
testFile = TestFile.from_parent(parent=None)
test = {
"input": {...},
"output": {...}, # noqa RST201
}
test_item = TestItem.from_parent(parent=testFile, test=test)

where 'from_parent' is inherited from pytest.Item through YamlItem
"""

def __init__(self, test, **kwargs) -> None:
# get expected 'parent' from kwargs (comes from 'from_parent')
super().__init__(
name="",
path="",
baseline_tax_benefit_system=TaxBenefitSystem(),
test=test,
options={},
**kwargs,
)


class TestVariable(Variable):
Expand All @@ -82,11 +101,13 @@ def __init__(self) -> None:
self.dtype = numpy.float32


@pytest.mark.skip(reason="Deprecated node constructor")
def test_variable_not_found() -> None:
test_file = TestFile.from_parent(parent=None)
test = {"output": {"unknown_variable": 0}}
with pytest.raises(errors.VariableNotFoundError) as excinfo:
test_item = TestItem(test)
test_item = TestItem.from_parent(parent=test_file, test=test)
test_item.tax_benefit_system = test_item.baseline_tax_benefit_system
test_item.simulation = Simulation()
test_item.check_output()
assert excinfo.value.variable_name == "unknown_variable"

Expand Down Expand Up @@ -147,13 +168,31 @@ def test_extensions_order() -> None:
) # extensions order is ignored in cache


@pytest.mark.skip(reason="Deprecated node constructor")
def test_runtest() -> None:
test_file = TestFile.from_parent(parent=None)
test = {
"input": {"salary": {"2017-01": 2000}},
"output": {"salary": {"2017-01": 2000}},
}
test_item = TestItem.from_parent(parent=test_file, test=test)

# TestItem init should instantiate the baseline TaxBenefitSystem
assert test_item.baseline_tax_benefit_system.get_variable("salary") is not None

test_item.runtest()

# TestItem.runtest(...) should instantiate the TaxBenefitSystem and the Simulation
assert test_item.tax_benefit_system.get_variable("salary") is not None
assert test_item.simulation is not None


def test_performance_graph_option_output() -> None:
test_file = TestFile.from_parent(parent=None)
test = {
"input": {"salary": {"2017-01": 2000}},
"output": {"salary": {"2017-01": 2000}},
}
test_item = TestItem(test)
test_item = TestItem.from_parent(parent=test_file, test=test)
test_item.options = {"performance_graph": True}

paths = ["./performance_graph.html"]
Expand All @@ -169,13 +208,13 @@ def test_performance_graph_option_output() -> None:
clean_performance_files(paths)


@pytest.mark.skip(reason="Deprecated node constructor")
def test_performance_tables_option_output() -> None:
test_file = TestFile.from_parent(parent=None)
test = {
"input": {"salary": {"2017-01": 2000}},
"output": {"salary": {"2017-01": 2000}},
}
test_item = TestItem(test)
test_item = TestItem.from_parent(parent=test_file, test=test)
test_item.options = {"performance_tables": True}

paths = ["performance_table.csv", "aggregated_performance_table.csv"]
Expand All @@ -191,6 +230,33 @@ def test_performance_tables_option_output() -> None:
clean_performance_files(paths)


def test_verbose_option_output(capsys) -> None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test used to debug the TypeError fixed by this PR.

test_file = TestFile.from_parent(parent=None)

expected_output_variable = "salary"
expected_output_date = "2017-01"
expected_output_value = 2000
test = {
"input": {"salary": {"2017-01": 2000}},
"output": {expected_output_variable: {expected_output_date: expected_output_value}},
}

test_item = TestItem.from_parent(parent=test_file, test=test)

test_item.options = {"verbose": True}
test_item.runtest()
captured = capsys.readouterr()

# TestItem.runtest should set the trace attribute from the 'verbose' option
assert test_item.simulation.trace is True

# TestItem.runtest should run print_computation_log
assert captured.out != ""
assert expected_output_variable in captured.out
assert expected_output_date in captured.out
assert str(expected_output_value) in captured.out


def clean_performance_files(paths: list[str]) -> None:
for path in paths:
if os.path.isfile(path):
Expand Down
Loading