search

pact-foundation / pact-js

JS version of Pact. Pact is a contract testing framework for HTTP APIs and non-HTTP asynchronous messaging systems.
https://pact.io
Other
1.63k stars 346 forks source link

regex matchers don't work for method #493

Closed individual-it closed 4 years ago

individual-it commented 4 years ago

Issue Classification

Bug Report

Software versions

Issue Checklist

Confirm the following:

Expected behaviour

regex matchers should also work for method

Actual behaviour

when trying to use regex matcher for method an error is thrown telling that a valid HTTP method should be provided and no POST request is send to create an interaction

  An error was thrown in afterAll
  Error: You must provide a valid HTTP method: GET, POST, PUT, PATCH, DELETE, HEAD, OPTIONS.
      at new e (tests/karma.js:62:86997)
      at n.withRequest (tests/karma.js:54:6980)
      at n.addInteraction (tests/karma.js:62:92290)
      at UserContext.<anonymous> (tests/karma.js:8:950070)
      at <Jasmine>

Steps to reproduce

https://gist.github.com/a4f20344c274ff20c2736fb8f59098b6

Relevant log files

Chrome Headless 85.0.4183.83 (Linux x86_64) ERROR
  An error was thrown in afterAll
  Error: You must provide a valid HTTP method: GET, POST, PUT, PATCH, DELETE, HEAD, OPTIONS.
      at new e (tests/karma.js:62:86997)
      at n.withRequest (tests/karma.js:54:6980)
      at n.addInteraction (tests/karma.js:62:92290)
      at UserContext.<anonymous> (tests/karma.js:8:950070)
      at <Jasmine>
Chrome Headless 85.0.4183.83 (Linux x86_64): Executed 2 of 256 (2 FAILED) (skipped 254) ERROR (0 secs / 0.026 secs)
Chrome Headless 85.0.4183.83 (Linux x86_64) ERROR
  An error was thrown in afterAll
  Error: You must provide a valid HTTP method: GET, POST, PUT, PATCH, DELETE, HEAD, OPTIONS.
      at new e (tests/karma.js:62:86997)
      at n.withRequest (tests/karma.js:54:6980)
      at n.addInteraction (tests/karma.js:62:92290)
      at UserContext.<anonymous> (tests/karma.js:8:950070)
      at <Jasmine>
Chrome Headless 85.0.4183.83 (Linux x86_64): Executed 2 of 256 (2 FAILED) (skipped 254) ERROR (8.166 secs / 0.026 secs
mefellows commented 4 years ago

We don't support these as you've discovered, why would you want to regex the method?

individual-it commented 4 years ago

the app responses with the same response for a POST and a DELETE, basically just "OK, I've done it" and a regex would be the easiest way to define a single interaction for both cases

mefellows commented 4 years ago

OK thanks. I'll leave it open for now, it seems an edge case but I'm willing to think on it!

TimothyJones commented 4 years ago

This issue is not the same, but seems related: https://github.com/pact-foundation/pact-specification/issues/68

a regex would be the easiest way to define a single interaction for both cases

What test runner are you using? A parameterised test might also work for you (eg it.each() from jest)

TimothyJones commented 4 years ago

Thinking about this more - Is what you want to express "When I send a DELETE or a POST, the provider responds with response X"?

If that is the case, I'm not sure that a regex would be the right solution - because it would pass if you didn't test one of the cases.

This is related/analogous to the array matcher, which doesn't let you specify zero-length each-likes, because it would be possible for a test that never returned any data to pass - if we had a regex here, a test might pass without ever generating (say) a DELETE.

I think to best test this contract, you'd want to specify the interaction twice in your contact - although to avoid writing it out twice in your tests, I think you could parameterise the test (even by hand if your test runner doesn't support parameterised tests).

mefellows commented 4 years ago

Actually I would go even further. This seems to be a test of the provider (it can support either). But this is your consumer, you can control what it does. Make it do just one of them, and test that.

That's all a consumer test should do - test what your actual code does.

TimothyJones commented 4 years ago

@individual-it Let us know if the above suggestions work for you.

individual-it commented 4 years ago

Thank you for the quick replies.

I solved my issues exactly as you proposed by using a parameterised test https://github.com/owncloud/owncloud-sdk/pull/552/files#diff-cfe657b17f3ff0e25e2ed0f1873da401R446-R478 that is all good and fine, I just don't get it why a matcher in that case wouldn't be a good thing (maybe the reason is a lack of understanding on contract tests - I just started with them)

Currently I'm changing existing consumer tests from using a real server to be executed, to use pact.io. There is a function in the consumer that sends a DELETE request and an other function that sends a POST request, both functions expect the same response in the case of success. What I see is, that if I would have a matcher (regex or something else) for the method and would use the same contract to test the provider, I would only test one of the requests and not both. Is that what you are saying? But would that not be the case for any matcher?

TimothyJones commented 4 years ago

Ah! This is a good point, and highlights that we could better explain matchers in the documentation.

So, in a consumer test, each interaction is saying "when I call my_code, I send THIS_REQUEST, and I expect to receive THIS_RESPONSE when the provider is in THIS_STATE, that response is then marshalled into this_business_object which is returned by my code".

A pact test asserts everything with underscores, but Pact records all of the things in capitals and stores them in a contract (along with any other interactions). You can see some diagrams of this here.

Those contracts (those collections of interactions + states) can then be played back against the provider at a later time. There are two main steps here:

Matchers are useful because usually you don't care about the specific data that's in the contract.

For example, in a test for get user with ID=10 in the state a user with ID=10 exists, your mock provider might return { "fullName": "John Smith", "id": 10}. But, contract-wise, you're actually happy with any string in the response. For example, the provider's test data might be { "fullName": "Jane Doe", "id": 10} - this would still be within the contract, even if it's not the exact test data that was used when the contract was generated.

In a pact consumer test, you specify the request that you are expecting to receive, and the specified request is exactly what is sent by the pact mock server to your code. But you can use a matcher to widen the allowed data, so that provider verification can succeed even if it has different test data.

In the example above, a response that had "id": 11 would break the contract- because that's not the user with id: 10. So, you might write:

{
    fullName: like("John Smith")
    id: 10
}

This is useful because it avoids the need to have very long test data states, like "A user with Id=10 exists with fullName: John Smith".

However, matchers aren't really useful for reducing the specificity of your request- for example, if your calling code is getUser(10), you wouldn't want to match any user Id, because it's not correct for your code to produce a request for user 12 when you call getUser(10). Similarly, because (presumably) your code will predictably produce DELETE or POST depending on which function is called (or perhaps which object the function was called with), it's better to be specific.

Does this help?

TimothyJones commented 4 years ago

I think this is shaping up to be more a documentation issue than a bug. We've already got a report about improving the matcher documentation, which I think can cover this case too (#490).

I'll close this one for now, but feel free to comment more if you have further questions or if improved documentation wouldn't solve this.

individual-it commented 4 years ago

@TimothyJones thank you for that great explanation, it definitely makes things clearer