"Apply matching rules using the first element of the spec array"

[zhammer]

pactman/pactman/verifier/verify.py

Line 325 in 7cb23a4

# always apply matching rules using the first element of the spec array

Out of curiosity: is this behavior part of the pact specification?

It seems odd to me that I wouldn't be able to verify the pact:

provider: AlphabetService
upon_receiving: /alphabet?limit=3
responds_with: 'letters': ['a', 'b', 'c']
matching_rules: None

with the response: 'letters': ['a', 'b', 'c'], since each data_elem will be compared not with its corresponding spec_elem, but with the first element in the spec. (i'll get the error data[1] 'b' does not equal spec 'a'.

i could understand if the idea is that expecting an exact array from a provider is too fragile and we should only look for general list matches, but just want to confirm!

[richard-reece]

Is this an actual issue you've run into? I just created a new test case based on what you've written above, and the current code passes the test case. For your reference, the test case I wrote is:

{
  "match": true,
  "comment": "empty arrays match",
  "expected": {
    "headers": {"Content-Type": "application/json"},
    "body": {
      "letters": ["a", "b", "c"]
    }
  },
  "actual": {
    "headers": {
      "Content-Type": "application/json"
    },
    "body": {
      "letters": ["a", "b", "c"]
    }
  }
}

[richard-reece]

There are specific test cases in the pact specification that relate to this:

testcases/response/body/array in different order.json (specifies that each element is tested when running an equality match)

testcases/response/body/array with type matcher.json (specifies only one element in the expected body, this only one element's type can be applied to all elements in the actual)

[zhammer]

okay, i think i found the issue. the test_verifier test cases use success = not bool(rv.result.fail.call_count) to determine whether or not verification is successful. they don't use the actual rv.verify return value. this means that if a certain failure path forgets to call self.result.fail, the test function will think verification was successful even though some failure was hit.

reproduce

here's our response body test json. i'm too tired to remember why adding the matching rule gets us into the apply_rules_array function but that's where the bug happens.

{
  "match": true,
  "comment": "identical lists match",
  "expected": {
    "headers": {"Content-Type": "application/json"},
    "body": {
      "letters": ["a", "b", "c"]
    },
    "matchingRules": {
      "body": {
        "$.letters[2]": {"matchers": [{"match": "type"}]}
      }
    }

  },
  "actual": {
    "headers": {
      "Content-Type": "application/json"
    },
    "body": {
      "letters": ["a", "b", "c"]
    }
  }
}

running the tests as is should pass. but, if you change:

pactman/pactman/verifier/verify.py

Line 330 in 7cb23a4

return False

to

return self.result.fail('oops')

the test case for this test should fail.

what's happening is, on the second iteration of the for i, data_elem in enumerate(data):, data_elem is 'b', but spec_elem is still 'a' (since we're always comparing against the first item in the spec). we eventually get to:

pactman/pactman/verifier/verify.py

Line 221 in 7cb23a4

if not data == spec:

which should fail verification, but the calling function only returns False and self.result.fail is never called.

note

maybe there is a reason that the test cases are okay with verify returning False as long as self.result.fail is never called? if so, i may be wrong on this, but it seems odd to me.

[zhammer]

this is where having the matching rule present sends us down the apply_rules path:

pactman/pactman/verifier/verify.py

Lines 188 to 190 in 7cb23a4

	if self.matching_rules:
	# if we have matchingRules then just look at those things
	r = self.apply_rules(data, spec, path)

[zhammer]

also can be done more simply with:

{
  "match": true,
  "comment": "identical lists match",
  "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"]
  }
}

[richard-reece]

Thank you so much for the investigation! I'll get a patch out asap.

[richard-reece]

In general there shouldn't be a failure without a result.fail() call (because a failure without an error message is super unhelpful). I might need to rethink how failures work.

[zhammer]

no problem! stepping through new code bases is always a fun adventure, ha.

i'm curious to see your patch for this. unfortunately (or fortunately?) i believe this uncovers a few other false positive tests in the test suite against the pact specification cases. i experimented with some simple fixes to patch the bug in my free time but realized it was quite difficult.

[richard-reece]

I've added a test to the json testcases code to check that there's a fail() called for each time it's actually failed. That's turned up a few places which I'll look at addressing, yep.

[richard-reece]

@zhammer please give that branch a try and let me know whether it addresses your issue

[zhammer]

my pact verifies now!

[richard-reece]

Excellent!