diff --git a/README.md b/README.md index 890728c..8e513e9 100644 --- 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 index e59b17b..f0e5e1e 100644 --- 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 index 892fdb8..a85b343 100644 --- 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 index fa5988d..efbf629 100644 --- 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 new file mode 100644 index 0000000..f143748 --- /dev/null +++ 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"] + } +} \ No newline at end of file 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 new file mode 100644 index 0000000..059f6fb --- /dev/null +++ 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 index 1e9909e..6b3fee8 100644 --- 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 diff --git a/pactman/verifier/verify.py b/pactman/verifier/verify.py index 07468c1..0bcc21d 100644 --- a/pactman/verifier/verify.py +++ b/pactman/verifier/verify.py @@ -174,7 +174,12 @@ def verify(self, response): if header.lower() != actual.lower(): continue actual = response.headers[actual] - if not self.check_rules(actual, expected, ['header', header]): + # In 2: + rule_section = 'header' + else: + rule_section = 'headers' + if not self.check_rules(actual, expected, [rule_section, header]): log.info(f'{self.interaction_name} headers: {response.json()}') return False if self.body is not MISSING: @@ -190,7 +195,7 @@ def check_rules(self, data, spec, path): r = self.apply_rules(data, spec, path) else: # otherwise the actual must equal the expected (excepting dict elements in actual that are unexpected) - if path[0] == 'header': + if path[0] in ('header', 'headers'): r = self.compare_header(data, spec, path) else: r = self.compare(data, spec, ['body']) @@ -200,14 +205,28 @@ def check_rules(self, data, spec, path): def compare_header(self, data, spec, path): parsed_data = sorted(parse_header(data)) parsed_spec = sorted(parse_header(spec)) + print(parsed_data, parsed_spec) log.debug(f'compare_header {parsed_data} {parsed_spec}') - if parsed_data != parsed_spec: - # there's a mind-bogglingly specific caveat to header matching that says that if the headers don't - # match and they're a Content-Type and an encoding is supplier but not expected then it's OK to pass - data_has_charset = [part for part in parsed_data if part.has_param('charset')] - spec_has_charset = [part for part in parsed_spec if part.has_param('charset')] - if path[1].lower() == 'content-type' and data_has_charset and not spec_has_charset: - return True + if parsed_data == parsed_spec: + return True + + if path[1].lower() != 'content-type': + return self.result.fail(f'{self.interaction_name} header {path[1]} value {data!r} does not match ' + f'expected {spec!r}') + # there's a specific caveat to header matching that says that if the headers don't match and they're a + # Content-Type and an encoding present in one and not the other, then that's OK + + # first, confirm the non-charset parts match + data_without_charset = [part for part in parsed_data if not part.has_param('charset')] + spec_without_charset = [part for part in parsed_spec if not part.has_param('charset')] + if data_without_charset != spec_without_charset: + return self.result.fail(f'{self.interaction_name} header {path[1]} value {data!r} does not match ' + f'expected {spec!r} (ignoring charset)') + + # now see whether the presence of the charset differs + data_has_charset = any(part for part in parsed_data if part.has_param('charset')) + spec_has_charset = any(part for part in parsed_spec if part.has_param('charset')) + if data_has_charset == spec_has_charset: return self.result.fail(f'{self.interaction_name} header {path[1]} value {data!r} does not match ' f'expected {spec!r}') return True @@ -250,11 +269,11 @@ def compare_dict(self, data, spec, path): def apply_rules(self, data, spec, path): # given some actual data and a pact spec at a certain path, check any rules for that path log.debug(f'apply_rules data={data!r} spec={spec!r} path={format_path(path)}') - rule = self.find_rule(path) - log.debug(f'... rule lookup got {rule}') - if rule: + weighted_rule = self.find_rule(path) + log.debug(f'... rule lookup got {weighted_rule}') + if weighted_rule: try: - rule.apply(data, spec, path) + weighted_rule.rule.apply(data, spec, path) except RuleFailed as e: log.debug(f'... failed: {e}') return self.result.fail(str(e), path) @@ -269,13 +288,18 @@ def apply_rules(self, data, spec, path): r = self.apply_rules_dict(data, spec, path) log.debug(f'apply_rules {format_path(path)} DONE = {r!r}') return r - elif not rule: - # in the absence of a rule, and we're at a leaf, we fall back on equality - log.debug('... falling back on equality matching') - return data == spec + elif not weighted_rule: + if path[0] in ('header', 'headers'): + log.debug('... falling back on header matching') + return self.compare_header(data, spec, path) + else: + # in the absence of a rule, and we're at a leaf, we fall back on equality + log.debug('... falling back on equality matching') + return data == spec return True def find_rule(self, path): + # rules are trickier to find because the mapping of content to matchingRules isn't 1:1 so... section = path[0] section_rules = self.matching_rules.get(section) if not section_rules: @@ -284,14 +308,18 @@ def find_rule(self, path): if self.pact.semver['major'] > 2: # version 3 rules paths don't include the interaction section ("body", "headers", ...) path = path[1:] - if section == 'body': - # but body paths always have a '$' at the start, yes + if section == 'body': + # but body paths always have a '$' at the start, yes + path = ['$'] + path + else: + # version 2 paths always have a '$' at the start path = ['$'] + path - weights = sorted((rule.weight(path), i, rule) for i, rule in enumerate(section_rules)) - log.debug(f'... path {path} got weights {weights}') - weight, i, rule = weights[-1] - if weight: - return rule + weighted_rules = sorted(rule.weight(path) for rule in section_rules) + display = [(weighted_rule.weight, weighted_rule.rule.path) for weighted_rule in weighted_rules] + log.debug(f'... path {path} got weights {display}') + weighted_rule = weighted_rules[-1] + if weighted_rule.weight: + return weighted_rule def apply_rules_array(self, data, spec, path): log.debug(f'apply_rules_array {data!r} {spec!r} {path!r}') @@ -308,29 +336,33 @@ def apply_rules_array(self, data, spec, path): if spec and not data: return self.result.fail(f'{self.interaction_name} spec requires data in array but data is empty', path) - # Attempt to find a matchingRule for this path elements in the array - if we find one then we use the first - # spec value (since they must all satisfy the matchingRule and there may only be one) otherwise - # we are comparing value to value so pass through the actual value from the spec. - log.debug('looking for a rule') - rule = self.find_rule(path + [0]) - if rule is not None: - log.debug(f'... got a rule {rule}, applying to first elements') - try: - rule.apply(data[0], spec[0], path) - except RuleFailed as e: - log.debug(f'... failed: {e}') - return self.result.fail(str(e), path) - log.debug(f'... performing elementwise rule application') - for i, data_elem in enumerate(data): - # always apply matching rules using the first element of the spec array - spec_elem = spec[0] - p = path + [i] - if not self.apply_rules(data_elem, spec_elem, p): - log.debug(f'apply_rules_array {path!r} failing on item {i}') + # Attempt to find a matchingRule for the elements in the array + log.debug('iterating elements, looking for rules') + for index, data_elem in enumerate(data): + if not self.apply_rules_array_element(data_elem, spec, path + [index], index): + log.debug(f'apply_rules_array {path!r} failing on item {index}') return False log.debug(f'apply_rules_array {path!r} DONE = True') return True + def apply_rules_array_element(self, data, spec, path, index): + # we're iterating an array - attempt to find a rule for the specific data element and if + # the index of the rule is "*" then just use the first element in the spec, otherwise use + # the *matching* element of the spec. + log.debug(f'apply_rules_array_element data={data!r} spec={spec!r} path={format_path(path)}') + weighted_rule = self.find_rule(path) + log.debug(f'... element rule lookup got {weighted_rule}') + if len(spec) == 1 and index: + # if the spec is a single element but data is longer there's a *good chance* that it's a sample + # for matching rules to be applied to + spec = spec[0] + else: + # element to element comparisons + spec = spec[index] + + # now do normal rule application + return self.apply_rules(data, spec, path) + def apply_rules_dict(self, data, spec, path): log.debug(f'apply_rules_dict {data!r} {spec!r} {path!r}') if fold_type(data) is not dict: