Fix testcase exercising

Some failing testcases were being falsely passed because the failure was not being correctly detected. Mostly this was around the appropriate application of matching rules, where several bugs have been fixed: - confusing between rule paths being "headers" vs. "header" in v2/v3 - "path" rule paths were being incorrectly prefixed internally in v3 - array element iteration was always treating the spec as a single sample, breaking rules applying to specific array elements fixes #11
reecetech · Dec 13, 2018 · 03a3c82 · 03a3c82
1 parent 7cb23a4
commit 03a3c82
Show file tree

Hide file tree

Showing 8 changed files with 164 additions and 170 deletions.
diff --git a/README.md b/README.md
@@ -316,6 +316,12 @@ From there you can use pip to install it:
 
 ## Release History
 
+2.6.0
+
+- Fix several issues cause by a failure to detect failure in several test cases
+  (header, path and array element rules may not have been applied)
+- Fix rules applying to a single non-first element in an array
+
 2.5.0
 
 - Fix some bugs around empty array verification

diff --git a/pactman/__version__.py b/pactman/__version__.py
@@ -1 +1 @@
-__version__ = '2.5.0'
+__version__ = '2.6.0'
diff --git a/pactman/test/test_matching_rules.py b/pactman/test/test_matching_rules.py
@@ -35,11 +35,12 @@ def test_invalid_match_type(monkeypatch):
 ])
 def test_weightings(path, weight, spam_weight):
     rule = Matcher(path, {'match': 'type'})
-    assert rule.weight(['$', 'body', 'item1', 'level', 1, 'id']) == weight
-    assert rule.weight(['$', 'body', 'item2', 'spam', 1, 'id']) == spam_weight
+    assert rule.weight(['$', 'body', 'item1', 'level', 1, 'id']).weight == weight
+    assert rule.weight(['$', 'body', 'item2', 'spam', 1, 'id']).weight == spam_weight
 
 
 @pytest.mark.parametrize('path, result', [
+    ('', []),
     ('$', ['$']),
     ('$.body', ['$', 'body']),
     ('$.body.item1', ['$', 'body', 'item1']),

diff --git a/pactman/test/test_verifier.py b/pactman/test/test_verifier.py
@@ -16,12 +16,12 @@
 BASE_DIR = pathlib.Path(__file__).absolute().parents[0]
 
 
-def all_testcases(path):
+def all_testcases(path, version):
     for entry in path.iterdir():
         if entry.is_file() and entry.suffix == '.json':
-            yield str(entry)
+            yield str(entry), version
         elif entry.is_dir():
-            yield from all_testcases(entry)
+            yield from all_testcases(entry, version)
 
 
 @pytest.fixture
@@ -363,84 +363,25 @@ def json(self):
 
 
 @pytest.mark.parametrize(
-    'filename',
-    all_testcases(BASE_DIR / 'pact-spec-version-1.1' / 'testcases' / 'response')
-)
-def test_version_1_1_response_testcases(filename, mock_pact, mock_result):
-    with open(filename) as file:
-        case = json.load(file)
-        rv = ResponseVerifier(mock_pact('1.1.0'), case['expected'], mock_result)
-        rv.verify(FakeResponse(case['actual']))
-        success = not bool(rv.result.fail.call_count)
-        assert case['match'] == success
-
-
-@pytest.mark.parametrize(
-    'filename',
-    all_testcases(BASE_DIR / 'pact-spec-version-1.1' / 'testcases' / 'request')
-)
-def test_version_1_1_request_testcases(filename, mock_pact, mock_result):
-    with open(filename) as file:
-        case = json.load(file)
-        rv = RequestVerifier(mock_pact('1.1.0'), case['expected'], mock_result)
-        rv.verify(FakeRequest(case['actual']))
-        success = not bool(rv.result.fail.call_count)
-        assert case['match'] == success
-
-
-@pytest.mark.parametrize(
-    'filename',
-    all_testcases(BASE_DIR / 'pact-spec-version-2' / 'testcases' / 'response')
-)
-def test_version_2_response_testcases(filename, mock_pact, mock_result):
-    if filename.endswith(' xml.json'):
-        # some of the files don't declare the damned content-type!
-        raise pytest.skip('XML content type not supported')
-    with open(filename) as file:
-        case = json.load(file)
-        if case['expected'].get('headers', {}).get('Content-Type', "") == 'application/xml':
-            raise pytest.skip('XML content type not supported')
-        rv = ResponseVerifier(mock_pact('2.0.0'), case['expected'], mock_result)
-        rv.verify(FakeResponse(case['actual']))
-        success = not bool(rv.result.fail.call_count)
-        assert case['match'] == success
-
-
-@pytest.mark.parametrize(
-    'filename',
-    all_testcases(BASE_DIR / 'pact-spec-version-2' / 'testcases' / 'request')
-)
-def test_version_2_request_testcases(filename, mock_pact, mock_result):
-    with open(filename) as file:
-        case = json.load(file)
-        if case['expected'].get('headers', {}).get('Content-Type', "") == 'application/xml':
-            raise pytest.skip('XML content type not supported')
-        rv = RequestVerifier(mock_pact('2.0.0'), case['expected'], mock_result)
-        rv.verify(FakeRequest(case['actual']))
-        success = not bool(rv.result.fail.call_count)
-        assert case['match'] == success
-
-
-@pytest.mark.parametrize(
-    'filename',
-    all_testcases(BASE_DIR / 'pact-spec-version-3' / 'testcases' / 'request')
-)
-def test_version_3_request_testcases(filename, mock_pact, mock_result):
-    with open(filename) as file:
-        case = json.load(file)
-        if case['expected'].get('headers', {}).get('Content-Type', "") == 'application/xml':
-            raise pytest.skip('XML content type not supported')
-        rv = RequestVerifier(mock_pact('3.0.0'), case['expected'], mock_result)
-        rv.verify(FakeRequest(case['actual']))
-        success = not bool(rv.result.fail.call_count)
-        assert case['match'] == success
-
-
-@pytest.mark.parametrize(
-    'filename',
-    all_testcases(BASE_DIR / 'pact-spec-version-3' / 'testcases' / 'response')
+    'filename, version, verifier, response',
+    chain(
+        ((f, v, ResponseVerifier, FakeResponse) for f, v in chain(
+            all_testcases(BASE_DIR / 'pact-spec-version-1.1' / 'testcases' / 'response', '1.1.0'),
+            all_testcases(BASE_DIR / 'pact-spec-version-2' / 'testcases' / 'response', '2.0.0'),
+            all_testcases(BASE_DIR / 'pact-spec-version-3' / 'testcases' / 'response', '3.0.0'),
+            all_testcases(BASE_DIR / 'testcases-version-2' / 'response', '2.0.0'),
+            all_testcases(BASE_DIR / 'testcases-version-3' / 'response', '3.0.0'),
+        )),
+        ((f, v, RequestVerifier, FakeRequest) for f, v in chain(
+            all_testcases(BASE_DIR / 'pact-spec-version-1.1' / 'testcases' / 'request', '1.1.0'),
+            all_testcases(BASE_DIR / 'pact-spec-version-2' / 'testcases' / 'request', '2.0.0'),
+            all_testcases(BASE_DIR / 'pact-spec-version-3' / 'testcases' / 'request', '3.0.0'),
+            # all_testcases(BASE_DIR / 'testcases-version-2' / 'request', '2.0.0'),
+            all_testcases(BASE_DIR / 'testcases-version-3' / 'request', '3.0.0'),
+        ))
+    )
 )
-def test_version_3_response_testcases(filename, mock_pact, mock_result):
+def test_testcases(filename, version, verifier, response, mock_pact, mock_result):
     if filename.endswith(' xml.json'):
         # some of the files don't declare the damned content-type!
         raise pytest.skip('XML content type not supported')
@@ -451,39 +392,8 @@ def test_version_3_response_testcases(filename, mock_pact, mock_result):
             raise pytest.skip('JSON test case mal-formed')
         if case['expected'].get('headers', {}).get('Content-Type', "") == 'application/xml':
             raise pytest.skip('XML content type not supported')
-        rv = ResponseVerifier(mock_pact('3.0.0'), case['expected'], mock_result)
-        rv.verify(FakeResponse(case['actual']))
-        success = not bool(rv.result.fail.call_count)
-        assert case['match'] == success
-
-
-@pytest.mark.parametrize(
-    'filename, verifier, result',
-    # chain(
-    #     ((t, RequestVerifier, FakeRequest) for t in all_testcases(BASE_DIR / 'testcases-version-2' / 'request')),
-    ((t, ResponseVerifier, FakeResponse) for t in all_testcases(BASE_DIR / 'testcases-version-2' / 'response'))
-    # )
-)
-def test_local_version_2_testcases(filename, verifier, result, mock_pact, mock_result):
-    with open(filename) as file:
-        case = json.load(file)
-        rv = verifier(mock_pact('2.0.0'), case['expected'], mock_result)
-        rv.verify(result(case['actual']))
-        success = not bool(rv.result.fail.call_count)
-        assert case['match'] == success
-
-
-@pytest.mark.parametrize(
-    'filename, verifier, result',
-    chain(
-        ((t, RequestVerifier, FakeRequest) for t in all_testcases(BASE_DIR / 'testcases-version-3' / 'request')),
-        ((t, ResponseVerifier, FakeResponse) for t in all_testcases(BASE_DIR / 'testcases-version-3' / 'response'))
-    )
-)
-def test_local_version_3_testcases(filename, verifier, result, mock_pact, mock_result):
-    with open(filename) as file:
-        case = json.load(file)
-        rv = verifier(mock_pact('3.0.0'), case['expected'], mock_result)
-        rv.verify(result(case['actual']))
+        rv = verifier(mock_pact(version), case['expected'], mock_result)
+        result = rv.verify(response(case['actual']))
         success = not bool(rv.result.fail.call_count)
+        assert result == success, 'fail() was not called for failure here'
         assert case['match'] == success
diff --git a/pactman/test/testcases-version-3/response/array contents match.json b/pactman/test/testcases-version-3/response/array contents match.json
@@ -0,0 +1,20 @@
+{
+  "match": true,
+  "comment": "match third array element by type",
+  "expected": {
+    "headers": {"Content-Type": "application/json"},
+    "body": ["a", "b", "c"],
+    "matchingRules": {
+      "body": {
+        "$[2]": {"matchers": [{"match": "type"}]}
+      }
+    }
+
+  },
+  "actual": {
+    "headers": {
+      "Content-Type": "application/json"
+    },
+    "body": ["a", "b", "c"]
+  }
+}
diff --git a/pactman/test/testcases-version-3/response/matches content type with charset.json b/pactman/test/testcases-version-3/response/matches content type with charset.json
@@ -0,0 +1,14 @@
+{
+  "match": false,
+  "comment": "content-type is different *and* charset presence differs",
+  "expected" : {
+    "headers": {
+      "Content-Type": "application/json"
+    }
+  },
+  "actual": {
+    "headers": {
+      "Content-Type": "text/plain; charset=UTF-8"
+    }
+  }
+}
diff --git a/pactman/verifier/matching_rule.py b/pactman/verifier/matching_rule.py
@@ -34,8 +34,12 @@ def split_path(path):
     * Path elements represent keys.
     * A star (*) can be used to match all keys of a map or all items of an array (one level only).
 
+    The empty path (see "path" matching rules) should result in an empty split path.
+
     Returns an iterator that has each path element as an item with array indexes converted to integers.
     """
+    if not path:
+        return
     for elem in re.split(r'[\.\[]', path):
         if elem == '*]':
             yield '*'
@@ -90,6 +94,18 @@ def fold_type(obj):
     return type(obj)
 
 
+class WeightedRule:
+    def __init__(self, rule, weight):
+        self.rule = rule
+        self.weight = weight
+
+    def __lt__(self, other):
+        return self.weight < other.weight
+
+    def __str__(self):
+        return f'rule="{self.rule}", weight={self.weight}'
+
+
 class Matcher:
     """Hold a JSONpath spec and a matchingRule rule and know how to test it.
 
@@ -130,7 +146,7 @@ def weight(self, element_path):
 
         Return the weight, or 0 if there is no match.
         """
-        return weight_path(list(split_path(self.path)), element_path)
+        return WeightedRule(self, weight_path(list(split_path(self.path)), element_path))
 
     def check_min(self, data, path):
         if 'min' not in self.rule:
@@ -303,13 +319,8 @@ def rule_matchers_v2(rules):
     """
     matchers = defaultdict(list)
     for path, spec in rules.items():
-        if path == '$.path':
-            # "path" rules are a bit different - there's no jsonpath as there's only a single value to compare, so we
-            # hard-code the path to '$' which always matches when looking for weighted path matches
-            matchers['path'].append(Matcher.get_matcher('$', spec))
-        else:
-            section = list(split_path(path))[1]
-            matchers[section].append(Matcher.get_matcher(path, spec))
+        section = list(split_path(path))[1]
+        matchers[section].append(Matcher.get_matcher(path, spec))
     return matchers
 
 
@@ -350,8 +361,8 @@ def rule_matchers_v3(rules):
     matchers = {}
     if 'path' in rules:
         # "path" rules are a bit different - there's no jsonpath as there's only a single value to compare, so we
-        # hard-code the path to '$' which always matches when looking for weighted path matches
-        matchers['path'] = [MultipleMatchers('$', **rules['path'])]
+        # hard-code the path to '' which always matches when looking for weighted path matches
+        matchers['path'] = [MultipleMatchers('', **rules['path'])]
     if 'query' in rules:
         # "query" rules are a bit different too - matchingRules are a flat single-level dictionary of keys which map to
         # array elements, but the data they match is keys mapping to an array, so alter the path such that the rule