Skip to content

Commit

Permalink
Error if a layer is missing
Browse files Browse the repository at this point in the history
  • Loading branch information
seddonym committed Oct 17, 2018
1 parent 3bdc090 commit d89c764
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 11 deletions.
5 changes: 5 additions & 0 deletions HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,8 @@ History

* Improved handling of invalid containers.

latest
------

* Error if a layer is missing.

6 changes: 5 additions & 1 deletion src/layer_linter/cmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,11 @@ def _main(package_name, config_filename=None, is_debug=False,
report = report_class(graph)

for contract in contracts:
contract.check_dependencies(graph)
try:
contract.check_dependencies(graph)
except Exception as e:
ConsolePrinter.print_error(str(e))
return EXIT_STATUS_ERROR
report.add_contract(contract)

report.output()
Expand Down
26 changes: 25 additions & 1 deletion src/layer_linter/contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ def __init__(self, name: str, containers: List[Module], layers: List[Layer],
self.whitelisted_paths = whitelisted_paths if whitelisted_paths else []

def check_dependencies(self, dependencies: DependencyGraph) -> None:
self._check_all_layers_exist_for_all_containers(dependencies)

self.illegal_dependencies: List[List[str]] = []

logger.debug('Checking dependencies for contract {}...'.format(self))
Expand All @@ -43,6 +45,25 @@ def check_dependencies(self, dependencies: DependencyGraph) -> None:
for layer in reversed(self.layers):
self._check_layer_does_not_import_downstream(layer, container, dependencies)

def _check_all_layers_exist_for_all_containers(self, dependencies: DependencyGraph) -> None:
"""
Raise a ValueError if we couldn't find any Python files for each layer in all containers.
"""
for container in self.containers:
self._check_all_layers_exist_for_container(container, dependencies)

def _check_all_layers_exist_for_container(self,
container: Module,
dependencies: DependencyGraph) -> None:
"""
Raise a ValueError if we couldn't find any Python files for each layer.
"""
for layer in self.layers:
layer_module = self._get_layer_module(layer, container)
if layer_module not in dependencies:
raise ValueError(f"Missing layer in container '{container}': "
f"module {layer_module} does not exist.")

def _check_layer_does_not_import_downstream(self, layer: Layer, container: Module,
dependencies: DependencyGraph) -> None:

Expand Down Expand Up @@ -80,13 +101,16 @@ def _get_modules_in_layer(
List of modules names within that layer, including the layer module itself.
Includes grandchildren and deeper.
"""
layer_module = Module("{}.{}".format(container, layer.name))
layer_module = self._get_layer_module(layer, container)
modules = [layer_module]
modules.extend(
dependencies.get_descendants(layer_module)
)
return modules

def _get_layer_module(self, layer: Layer, container: Module) -> Module:
return Module("{}.{}".format(container, layer.name))

def _get_modules_in_downstream_layers(
self, layer: Layer, container: Module, dependencies: DependencyGraph
) -> List[Module]:
Expand Down
12 changes: 11 additions & 1 deletion src/layer_linter/dependencies/graph.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import logging
from typing import List, Optional
from typing import List, Optional, Any
import networkx # type: ignore
from networkx.algorithms import shortest_path # type: ignore

Expand Down Expand Up @@ -129,3 +129,13 @@ def _remove_paths_from_networkx_graph(
def _restore_paths_to_networkx_graph(self, import_paths: List[ImportPath]) -> None:
for import_path in import_paths:
self._add_path_to_networkx_graph(import_path)

def __contains__(self, item: Any) -> bool:
"""
Returns whether or not the given item is a Module in the dependency graph.
Usage:
if module in graph:
...
"""
return item in self.modules
6 changes: 6 additions & 0 deletions tests/unit/dependencies/test_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,9 @@ def test_dependency_count(self):

# Should be number of ImportPaths returned by DependencyAnalyzer.determine_import_paths.
assert graph.dependency_count == 3

def test_contains(self):
graph = graph_module.DependencyGraph(self.PACKAGE)

assert Module('foo.one.alpha') in graph
assert Module('foo.one.omega') not in graph
85 changes: 77 additions & 8 deletions tests/unit/test_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,13 @@ class StubDependencyGraph:
A call to .find_path(downstream=downstream_module, upstream=upstream_module)
will return the supplied path, if it is present in the dictionary. Otherwise,
the stub will return None.
modules: List of all modules in the graph.
Usage:
graph = StubDependencyGraph(
descendants={
Module('foo.one'): [
Module('foo.one'), Module('foo.one.red'), Module('foo.one.green')
],
Module('foo.two'): [
Module('foo.two'),
Module('foo.one.red'), Module('foo.one.green')
],
},
paths = {
Expand All @@ -48,13 +46,15 @@ class StubDependencyGraph:
],
},
...
}
},
modules [Module('foo.one'), Module('foo.one.red'), Module('foo.one.green')],
)
"""
def __init__(self, descendants, paths):
self.descendants = descendants
self.paths = paths
def __init__(self, descendants=None, paths=None, modules=None):
self.descendants = descendants if descendants else {}
self.paths = paths if paths else {}
self.modules = modules if modules else []

def get_descendants(self, module):
try:
Expand All @@ -68,6 +68,9 @@ def find_path(self, downstream, upstream, ignore_paths=None):
except KeyError:
return None

def __contains__(self, item):
return item in self.modules


class TestContractCheck:
def test_kept_contract(self):
Expand Down Expand Up @@ -120,6 +123,18 @@ def test_kept_contract(self):
Module('foo.green.one.alpha')]
},
},
modules=[
Module('foo.green'),
Module('foo.green.one'),
Module('foo.green.one.alpha'),
Module('foo.green.one.beta'),
Module('foo.green.two'),
Module('foo.green.three'),
Module('foo.blue'),
Module('foo.blue.one'),
Module('foo.blue.two'),
Module('foo.blue.three'),
]
)

contract.check_dependencies(graph)
Expand Down Expand Up @@ -163,6 +178,18 @@ def test_broken_contract(self):
Module('foo.green.three.alpha')],
},
},
modules=[
Module('foo.green'),
Module('foo.green.one'),
Module('foo.green.one.alpha'),
Module('foo.green.one.beta'),
Module('foo.green.two'),
Module('foo.green.three'),
Module('foo.blue'),
Module('foo.blue.one'),
Module('foo.blue.two'),
Module('foo.blue.three'),
]
)

contract.check_dependencies(graph)
Expand Down Expand Up @@ -217,6 +244,11 @@ def test_broken_contract_via_other_layer(self):
Module('foo.one'): [Module('foo.one'), Module('foo.two')],
},
},
modules=[
Module('foo.one'),
Module('foo.two'),
Module('foo.three'),
]
)

contract.check_dependencies(graph)
Expand All @@ -226,6 +258,8 @@ def test_broken_contract_via_other_layer(self):
[Module('foo.two'), Module('foo.three')],
]



@pytest.mark.parametrize('longer_first', (True, False))
def test_only_shortest_violation_is_reported(self, longer_first):
contract = Contract(
Expand Down Expand Up @@ -273,6 +307,7 @@ def test_only_shortest_violation_is_reported(self, longer_first):
Module('foo.one.alpha.blue'), Module('foo.one.alpha.green')],
},
paths=paths,
modules=[Module('foo.one'), Module('foo.two')]
)

contract.check_dependencies(graph)
Expand All @@ -286,6 +321,40 @@ def test_only_shortest_violation_is_reported(self, longer_first):
[Module('foo.one.alpha'), Module('foo.another'), Module('foo.two')],
]

def test_missing_contract_raises_exception(self):
contract = Contract(
name='Foo contract',
containers=(
'foo.one',
'foo.two',
),
layers=(
Layer('blue'),
Layer('yellow'), # Missing from foo.two.
Layer('green'),
),
)
graph = StubDependencyGraph(
modules=[
Module('foo.one'),
Module('foo.one.blue'),
Module('foo.one.blue.alpha'),
Module('foo.one.yellow'),
Module('foo.one.green'),
Module('foo.two'),
Module('foo.two.blue'),
Module('foo.two.blue.alpha'),
Module('foo.two.green'),
]
)

with pytest.raises(ValueError) as e:
contract.check_dependencies(graph)

assert str(e.value) == (
"Missing layer in container 'foo.two': module foo.two.yellow does not exist."
)


@mock.patch.object(importlib.util, 'find_spec')
class TestContractFromYAML:
Expand Down

0 comments on commit d89c764

Please sign in to comment.