Merge pull request #3 from networktocode-llc/api-refactor

Fix tests

Author: lvrfrc87

.travis.yml

@@ -4,7 +4,7 @@ stages:
   - "lint"
   - "test"
 
-if: "type IN (pull_request)"  # Add in "branch" as an option if desired for branch testing as well
+if: "type IN (pull_request)" # Add in "branch" as an option if desired for branch testing as well
 language: "python"
 services:
   - "docker"
@@ -30,7 +30,7 @@ jobs:
         - "pip install invoke toml"
       script:
         - "invoke black"
-        - "invoke bandit"  # Bandit fails to function on > Py3.8 https://github.com/PyCQA/bandit/issues/639
+        - "invoke bandit" # Bandit fails to function on > Py3.8 https://github.com/PyCQA/bandit/issues/639
         # - "invoke pydocstyle"
         - "invoke flake8"
         - "invoke yamllint"

netcompare/__init__.py

@@ -1,5 +1,5 @@
 """Pre/Post Check library."""
-from .check_type import compare
+# from .check_type import compare
 
 
-__all__ = ["compare"]
+# __all__ = ["compare"]

netcompare/check_type.py

@@ -1,5 +1,5 @@
 """CheckType Implementation."""
-from typing import Mapping, Iterable, Tuple, Union, List
+from typing import Mapping, Tuple, Union, List
 from .evaluator import diff_generator
 from .runner import extract_values_from_output
 
@@ -29,6 +29,7 @@ def extract_value_from_json_path(
         """Return the value contained into a Mapping for a defined path."""
         return extract_values_from_output(value, path, exclude)
 
+    # TODO: Refine this typing
     def evaluate(self, reference_value: Mapping, value_to_compare: Mapping) -> Tuple[Mapping, bool]:
         """Return the result of the evaluation and a boolean True if it passes it or False otherwise.
 

netcompare/evaluator.py

@@ -7,19 +7,17 @@
 from functools import partial
 from typing import Mapping, List
 
-from .runner import extract_values_from_output
 
 sys.path.append(".")
 
 
-def diff_generator(pre_data: Mapping, post_data: Mapping, check_definition: Mapping) -> Mapping:
+def diff_generator(pre_result: Mapping, post_result: Mapping) -> Mapping:
     """
     Generates diff between pre and post data based on check definition.
 
     Args:
-        pre_data: pre data result.
-        post_data: post data result.
-        check_definition: check definitions.
+        pre_result: pre data result.
+        post_result: post data result.
 
     Return:
         output: diff between pre and post data.
@@ -32,9 +30,6 @@ def diff_generator(pre_data: Mapping, post_data: Mapping, check_definition: Mapp
         >>> print(diff_generator(check_definition, post_data, check_definition))
         {'10.17.254.2': {'state': {'new_value': 'Up', 'old_value': 'Idle'}}}
     """
-    pre_result = extract_values_from_output(check_definition, pre_data)
-    post_result = extract_values_from_output(check_definition, post_data)
-
     diff_result = DeepDiff(pre_result, post_result)
 
     result = diff_result.get("values_changed", {})

tests/test_diff_generator.py

@@ -4,6 +4,7 @@
 import sys
 from .utility import load_json_file
 from netcompare.evaluator import diff_generator
+from netcompare.runner import extract_values_from_output
 
 sys.path.append("..")
 
@@ -13,67 +14,40 @@
 expected output: {expected_output}
 """
 
-exact_match_of_global_peers_via_napalm_getter = (
-    "napalm_getter.json",
-    {
-        "check_type": "exact_match",
-        "path": "global.$peers$.*.[is_enabled,is_up]",
-        # "reference_key_path": "global.peers",
-    },
-)
+exact_match_of_global_peers_via_napalm_getter = ("napalm_getter.json", "global.$peers$.*.[is_enabled,is_up]", [])
 
 exact_match_of_bgpPeerCaps_via_api = (
     "api.json",
-    {
-        "check_type": "exact_match",
-        "path": "result[0].vrfs.default.peerList[*].[$peerAddress$,state,bgpPeerCaps]",
-        # "reference_key_path": "result[0].vrfs.default.peerList[*].peerAddress",
-    },
+    "result[0].vrfs.default.peerList[*].[$peerAddress$,state,bgpPeerCaps]",
+    [],
 )
 
-exact_match_of_bgp_neigh_via_textfsm = (
-    "textfsm.json",
-    {
-        "check_type": "exact_match",
-        "path": "result[*].[$bgp_neigh$,state]",
-        # "reference_key_path": "result[*].bgp_neigh"
-    },
-)
+exact_match_of_bgp_neigh_via_textfsm = ("textfsm.json", "result[*].[$bgp_neigh$,state]", [])
 
 raw_diff_of_interface_ma1_via_api_value_exclude = (
     "raw_value_exclude.json",
-    {"check_type": "exact_match", "path": "result[*]", "exclude": ["interfaceStatistics", "interfaceCounters"]},
+    "result[*]",
+    ["interfaceStatistics", "interfaceCounters"],
 )
 
 raw_diff_of_interface_ma1_via_api_novalue_exclude = (
     "raw_novalue_exclude.json",
-    {"check_type": "exact_match", "exclude": ["interfaceStatistics", "interfaceCounters"]},
+    None,
+    ["interfaceStatistics", "interfaceCounters"],
 )
 
-raw_diff_of_interface_ma1_via_api_novalue_noexclude = (
-    "raw_novalue_noexclude.json",
-    {"check_type": "exact_match"},
-)
+raw_diff_of_interface_ma1_via_api_novalue_noexclude = ("raw_novalue_noexclude.json", None, [])
 
-exact_match_missing_item = (
-    "napalm_getter_missing_peer.json",
-    {"check_type": "exact_match"},
-)
+exact_match_missing_item = ("napalm_getter_missing_peer.json", None, [])
 
-exact_match_additional_item = ("napalm_getter_additional_peer.json", {"check_type": "exact_match"})
+exact_match_additional_item = ("napalm_getter_additional_peer.json", None, [])
 
-exact_match_changed_item = (
-    "napalm_getter_changed_peer.json",
-    {"check_type": "exact_match"},
-)
+exact_match_changed_item = ("napalm_getter_changed_peer.json", None, [])
 
 exact_match_multi_nested_list = (
     "exact_match_nested.json",
-    {
-        "check_type": "exact_match",
-        "path": "global.$peers$.*.*.ipv4.[accepted_prefixes,received_prefixes]",
-        # "reference_key_path": "global.peers",
-    },
+    "global.$peers$.*.*.ipv4.[accepted_prefixes,received_prefixes]",
+    [],
 )
 
 eval_tests = [
@@ -90,13 +64,14 @@
 ]
 
 
-@pytest.mark.parametrize("filename, path", eval_tests)
-def test_eval(filename, path):
+@pytest.mark.parametrize("filename, path, exclude", eval_tests)
+def test_eval(filename, path, exclude):
 
     pre_data = load_json_file("pre", filename)
     post_data = load_json_file("post", filename)
     expected_output = load_json_file("results", filename)
-
-    output = diff_generator(pre_data, post_data, path)
+    pre_value = extract_values_from_output(pre_data, path, exclude)
+    post_value = extract_values_from_output(post_data, path, exclude)
+    output = diff_generator(pre_value, post_value)
 
     assert expected_output == output, assertion_failed_message.format(output=output, expected_output=expected_output)

tests/test_type_check.py

@@ -21,29 +21,17 @@ def test_CheckType_raises_NotImplementedError_for_invalid_check_type():
         CheckType.init("does_not_exist")
 
 
-def test_CheckType_raises_NotImplementedError_when_calling_check_logic_method():
-    """Validate that CheckType raises a NotImplementedError when passed a non-existant check_type."""
-    with pytest.raises(NotImplementedError):
-        CheckType().check_logic()
-
-
 exact_match_test_values_no_change = (
     ("exact_match",),
     "api.json",
-    {
-        "path": "result[0].vrfs.default.peerList[*].[$peerAddress$,establishedTransitions]",
-        # "reference_key_path": "result[0].vrfs.default.peerList[*].peerAddress",
-    },
+    "result[0].vrfs.default.peerList[*].[$peerAddress$,establishedTransitions]",
     ({}, True),
 )
 
 exact_match_test_values_changed = (
     ("exact_match",),
     "api.json",
-    {
-        "path": "result[0].vrfs.default.peerList[*].[$peerAddress$,prefixesSent]",
-        # "reference_key_path": "result[0].vrfs.default.peerList[*].peerAddress",
-    },
+    "result[0].vrfs.default.peerList[*].[$peerAddress$,prefixesSent]",
     (
         {
             "10.1.0.0": {"prefixesSent": {"new_value": 52, "old_value": 50}},
@@ -56,30 +44,21 @@ def test_CheckType_raises_NotImplementedError_when_calling_check_logic_method():
 tolerance_test_values_no_change = (
     ("tolerance", 10),
     "api.json",
-    {
-        "path": "result[0].vrfs.default.peerList[*].[$peerAddress$,establishedTransitions]",
-        # "reference_key_path": "result[0].vrfs.default.peerList[*].peerAddress",
-    },
+    "result[0].vrfs.default.peerList[*].[$peerAddress$,establishedTransitions]",
     ({}, True),
 )
 
 tolerance_test_values_within_threshold = (
     ("tolerance", 10),
     "api.json",
-    {
-        "path": "result[0].vrfs.default.peerList[*].[$peerAddress$,prefixesSent]",
-        # "reference_key_path": "result[0].vrfs.default.peerList[*].peerAddress",
-    },
+    "result[0].vrfs.default.peerList[*].[$peerAddress$,prefixesSent]",
     ({}, True),
 )
 
 tolerance_test_values_beyond_threshold = (
     ("tolerance", 10),
     "api.json",
-    {
-        "path": "result[0].vrfs.default.peerList[*].[$peerAddress$,prefixesReceived]",
-        # "reference_key_path": "result[0].vrfs.default.peerList[*].peerAddress",
-    },
+    "result[0].vrfs.default.peerList[*].[$peerAddress$,prefixesReceived]",
     (
         {
             "10.1.0.0": {"prefixesReceived": {"new_value": 120, "old_value": 100}},
@@ -104,41 +83,34 @@ def test_check_type_results(check_type_args, filename, path, expected_results):
     check = CheckType.init(*check_type_args)
     pre_data = load_json_file("pre", filename)
     post_data = load_json_file("post", filename)
-    actual_results = check.evaluate(pre_data, post_data, path)
+    pre_value = check.extract_value_from_json_path(pre_data, path)
+    post_value = check.extract_value_from_json_path(post_data, path)
+    actual_results = check.evaluate(pre_value, post_value)
     assert actual_results == expected_results
 
 
 napalm_bgp_neighbor_status = (
     "napalm_get_bgp_neighbors.json",
     ("exact_match",),
-    {
-        "path": "global.$peers$.*.[is_enabled,is_up]",
-        # "reference_key_path": "global.peers"
-    },
+    "global.$peers$.*.[is_enabled,is_up]",
     0,
 )
 
 napalm_bgp_neighbor_prefixes_ipv4 = (
     "napalm_get_bgp_neighbors.json",
     ("tolerance", 10),
-    {
-        "path": "global.$peers$.*.*.ipv4.[accepted_prefixes,received_prefixes,sent_prefixes]",
-        # "reference_key_path": "global.peers",
-    },
+    "global.$peers$.*.*.ipv4.[accepted_prefixes,received_prefixes,sent_prefixes]",
     1,
 )
 
 napalm_bgp_neighbor_prefixes_ipv6 = (
     "napalm_get_bgp_neighbors.json",
     ("tolerance", 10),
-    {
-        "path": "global.$peers$.*.*.ipv6.[accepted_prefixes,received_prefixes,sent_prefixes]",
-        # "reference_key_path": "global.peers",
-    },
+    "global.$peers$.*.*.ipv6.[accepted_prefixes,received_prefixes,sent_prefixes]",
     2,
 )
 
-napalm_get_lldp_neighbors_exact_raw = ("napalm_get_lldp_neighbors.json", ("exact_match",), {}, 0)
+napalm_get_lldp_neighbors_exact_raw = ("napalm_get_lldp_neighbors.json", ("exact_match",), None, 0)
 
 check_tests = [
     napalm_bgp_neighbor_status,
@@ -151,10 +123,13 @@ def test_check_type_results(check_type_args, filename, path, expected_results):
 @pytest.mark.parametrize("filename, check_args, path, result_index", check_tests)
 def test_checks(filename, check_args, path, result_index):
     """Validate multiple checks on the same data to catch corner cases."""
+    check = CheckType.init(*check_args)
     pre_data = load_json_file("pre", filename)
     post_data = load_json_file("post", filename)
     result = load_json_file("results", filename)
 
-    check = CheckType.init(*check_args)
-    check_output = check.evaluate(pre_data, post_data, path)
-    assert list(check_output) == result[result_index]
+    pre_value = check.extract_value_from_json_path(pre_data, path)
+    post_value = check.extract_value_from_json_path(post_data, path)
+    actual_results = check.evaluate(pre_value, post_value)
+
+    assert list(actual_results) == result[result_index]