-
-
Notifications
You must be signed in to change notification settings - Fork 349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Matcher of a type or null #51
Comments
Are you referring to optional types? If so, we don't support this. The article explains why. |
@mefellows not optional attributes, but the value could be null, like {"item": null} |
Hmm I don't believe so. From my perspective, a null value is equivalent to not having it there in the first place. Out of interest, have you tried? |
I tried and it failed on the null values and pass on the other values :) |
Sorry to say, but it does look like the same one. Leaving open until v3 implementation is completed. |
thanks @mefellows , that would be great. |
@bethesque just checking, this is not an available feature correct? |
No. It's because this test doesn't tell you anything. If you always get nulls in test environment, then you think you've passed, but what then the production environment returns a different type to what you expected, and your code blows up. It will not be supported. |
I know this ticket is closed but I ran into this just now and wanted to share a different opinion maybe people find it useful. While developing frontend applications I ran into a lot of cases where some kind of empty(ish) value was present in the response from the backend that created issues on the consumer side. Usually these were empty arrays, empty objects or a string that says "null". This lead to all sorts of long investigation and some defensive programming as a remedy (which I personally don't really like). So usually after a couple of meetings with service owners we managed to push for providing nothing instead of an empty value in case some data was actually not available. With a very exaggerated example you can end up with the following. Response: {
"usefulData": { "a": 1, "b": 2, "c": 3 },
"emptyThing": [],
"almostUsefulData": {
"butStillEmpty": [],
"thisDoesntHelpEither": {
"exxageratedButHappens": []
}
},
"andTheListGoesOn": [],
"nowThisIsJustRude": "null",
"butThisIsNotMuchBetterEitherBecauseIStillDontCare": null
} Which in the consumer will cause code like: const almostUsefulData = response && response.almostUsefulData;
const butStillEmpty =
almostUsefulData &&
almostUsefulData.butStillEmpty &&
almostUsefulData.butStillEmpty.length;
const thisDoesntHelpEither =
almostUsefulData && almostUsefulData.thisDoesntHelpEither;
const exxageratedButHappens =
thisDoesntHelpEither &&
thisDoesntHelpEither.exxageratedButHappens &&
thisDoesntHelpEither.exxageratedButHappens;
const hasData = almostUsefulData && (butStillEmpty || exxageratedButHappens);
if (hasData) {
// show it
} I left a bug in the above code just to show how easy it is to make one and hard to figure out where it is :). Instead of this much simpler one: const hasData = response && response.almostUsefulData;
if (hasData) {
// show it
} BUT: Opinions? |
I like your slightly contrived, a little bit long yet also illuminating example :) In any case, yes, our standard advice is to make sure you explicitly test the case where values exist, don't exist or are returned as empty values (e.g. |
Thanks for the quick response, sorry for the example but it really is not that far from reality. So if I understand it correctly the only thing missing from Pact to validate these kind of responses from the provider is something saying: // GIVEN // WHEN // THEN So for now all we can do is write pact for when things DO exist and use other forms of testing when things are missing. |
There's nothing stopping you from doing exactly that now. That's the whole point of provider states. Or am I missing something? |
Probably I am missing something because I couldn't find a way to check with pact for things that should NOT be present. Time to google... |
You would write a test as you stated above with a state indicating so (e.g. given |
It is not clear to me if there are 10 fields in json which can either have a value or not have value or takes only defined format then should we write additional 30 pact tests each with a state of a field testing all combinations of possible values for all fields ? Thanks |
@kxramas It's hard to say without more context - it's unusual that every field is optional however, and usually there are different sub-shapes in the payload you might be separately interested in which could form the basis of different test expectations. For example, a User may have a "profile" sub-object which is optional (perhaps that information is collected later on). Often times, this can be improved by having links to the resource if available (e.g. using HAL links), but sometimes you can't change the implementation. If they are all indeed optional, I'd first look at how you might be able to improve the API. The key thing is how your client behaves in the different circumstances - will it not work if none of the fields came back, or just a few? That might help you in your endeavour. |
If it was me, I'd do one test which had every field populated, and one that had the minimum that could be expected. Really, what you're testing in this case is that code that deserialises the response into your local domain objects can handle both scenarios. |
Hello, I would like to bring this up again because this is a serious limitation of the tool. In our company we have api responses with hundreds of fields of which dozens are optional and can be null. Unless I'm missing something about your position on optional types you suggest to write hundreds of cases for each combination of null/type fields which is terribly unpractical. Also you must understand that after a certain size projects will not adapt their code and style just for one tool. I understand the desire for ideal world where everything is perfectly typed but in fast growing environment this simply does not happen. As you see the amount of requests for this feature I suggest you to reconsider it. Thank you. |
Hi Max, thanks for the feedback. I can certainly empathise. At this stage, we're exploring the idea of "provider driven" contracts, where more of a schema comparison from the provider side against consumer test examples. You could think of this as a contained experiment, and learnings may be applied into the Matcher library. At this stage, as you can see from the above discussion (and Pact's official position on it, changing the core Matchers to support this is unlikely to change quickly. See pactflow/roadmap#4 (Pactflow specific) and pact-foundation/pact-specification#76 for future thinking that encompasses your suggestion. |
@mefellows thank you for fast response :) I found in specification 3 there is a |
You're welcome. With the combine matcher, I believe what you're suggesting is in theory possible, but I'm not entirely sure I'm afraid. @uglyog @bethesque care to weigh in on this one? |
I checked into the v3 features, I was able to generate specs but it seems the verifier does not support and style of matchers yet. Also I think contract generator does not have an api for combined fields. |
Do you need to test all the combinations? How few examples could you use to cover all the fields?
No, it's not supported in v2. Which language are you verifying in? It sounds to me like Pact is not a good fit for this particular use case. I would look at using the "Atlassian flow" which allows you to verify a pact against an OAS. You can continue to use Pact on the consumer side, but verify using https://www.npmjs.com/package/swagger-mock-validator on the provider side. |
Or wait until Pactflow's "provider driven" flow is out, as Matt suggested. |
It's also worth expanding on that briefly, because it might sound like a limitation (still having to use Pact on the consumer side). But actually, it allows you to vastly expand your suite of consumer tests without burdening the provider side, because all it does is compare schemas. It also allows this specific use case. |
@bethesque thanks for response,
So this is exactly my point, since different fields are optional Pact contracts will not cover the responses our backend can produce. Unless I'm missing something. I understand this is not supported in v2 so I'm looking forward v3, actually I'm preparing a pr because it seems that it can be done. @mefellows |
@maximvl apologies for the delay. The basic flow:
Separate to the above (before/after) the consumer runs its own pipeline:
The pact contract is now statically compared to the OAS, using a type of schema verification. If a field is null or not in the pact, so long as the OAS schema allows it it is ✅ . See also pactflow/roadmap#4. Please note that this is a feature that will be unique to Pactflow. It may be made open via the OSS pact broker eventually, but it's not currently planned. |
What if instead of "oops everything could fall through because it's all null", you implement syntax like
where the test case then has to hit both things on either side of the & condition? |
I think we've talked about something like that. You would still need a way of communicating those two states to the provider, so presumably it would need to map to a provider state (e.g. by convention). How do you envisage the consumer DSL and provider side working here? |
Hello, I am also struggling with similar kind of issues, I am writing test case for an api having 100 of filed and there are multiple optional/null value fields that some time return value or some time not. I have also read pact optional attribute but it is not helpful I don't want to create multiple scenarios, What are the other why to validate both attribute having null or some value. and is there are any way to compare only schema of expected response not value? |
No, Pact doesn't have a separate way of doing this other than what is discussed here. If you want a purely schema based test, you might be interested in a feature releasing next week at Pactflow which does exactly this: https://pactflow.io/bi-directional-contract-testing/#register |
Pact should allow nullable fields. I don't care what the orthodoxy says. The answers given here are objectively unhelpful. Nullable fields are such a basic thing apps do. At my company we literally cloned and modified a pact verifier, because we really needed this functionality. |
That is the beauty of open source! You can disagree and fork. I know it's hard, but as soon as you start to follow the line of thinking it quickly moves the framework away from what is uniquely powerful with Pact and its core design - and at that point you may as well do schema validation through other means and tools. We've taken the choice that it's best to be strict, even if that means it doesn't work for everyone. |
Hi I just wanted to add a couple data points here. I don't think my use case completely makes the argument one way or another but I haven't seen this perspective represented in the discussion:
|
Thanks @dantswain. I don't think this changes the philosophical argument here but another data point is helpful. The reality is, there are other solutions to test this kind of contract (the most common and readily available being a schema check). After many debates, we always land on "why would Pact support this when there are other tools that can do it" with the additional "now it's a different mode we need to support / document / maintain" and finally, we have to explain the loss in fidelity that that kind of testing enables. That is, a schema check enables a wide range of false positives (see https://pactflow.io/blog/schemas-are-not-contracts/ for a wider post on the matter). |
For me, it makes sense to allow optional field definition in contract because that's the truth about the contract. I don't understand why Pact is trying to prevent optional field on principle level. |
No, that is not what contract testing is about. The whole point of capturing the consumer contract is to avoid the ambiguity in such cases.
It's quite simple, if it doesn't matter, exclude it from the contract altogether.
Exactly.
Pact is simply not a schema checking tool - there are plenty of schema checking tools out there, Pact is not one of them. For a long form on why schemas are not contracts, see this post: https://pactflow.io/blog/schemas-are-not-contracts/. I'm going to lock this conversation, the position from Pact is clear on this topic. |
Is there a way to match the value to a type or null (not empty, not a string)?
The text was updated successfully, but these errors were encountered: