pact-foundation / pact-mock_service

Provides a mock service for use with Pact
https://pact.io
MIT License
73 stars 69 forks source link

Discussion - Optional fields in pact #65

Closed soundstep closed 7 years ago

soundstep commented 7 years ago

Hi Beth, I had so many discussions in my company about the same problem, that I'd like to expose it. Just so we can talk about it, as I feel I'm missing some kind of capability. We are "pact verifying" a lot of end points and there are always some properties on a JSON output that are not always there. Properties or even whole objects.

We kind of come to conclusion where we should either:

Here is a simple example of a list of categories for books:

{
    "_embedded": {
        "categories": [
            {
                "name": "Children",
                "_links": {
                    "doc:books": {
                        "href": "http://example.com/list-of-books?category=children"
                    }
                }
            },
            {
                "name": "Factual",
                "_links": {}
            }
        ]
        }
}

In this list of categories, the _links object may or may not contain a url that point to a list of books.

This is how my response interaction looks like:

{
    "_embedded": {
        "categories": Pact.Matchers.eachLike({
            "name": Pact.Matchers.somethingLike("Children"),
            "_links": {
                "doc:books": {
                    "href": Pact.Matchers.term({
                        matcher: "^(?=.*list-of-books)(?=.*category=)",
                        generate: "http://example.com/list-of-books?category=children"
                    })
                }
            }
        })
    }
}

When I come to verifying mocks, which may or may not have the links in each categories, the verification would fail as the pact interaction is expecting to always have a doc:books links.

What are my solutions?

  1. remove the optional part:
{
    "_embedded": {
        "categories": Pact.Matchers.eachLike({
            "name": Pact.Matchers.somethingLike("Children")
        })
    }
}

Problem: it doesn't represent the reality, url are not tested Benefit: can verify any mock files

  1. use provider state, remove eachLike matcher, basically hard-code response
{
    "_embedded": {
        "categories": [
            {
                "name": "Children",
                "_links": {
                    "doc:books": {
                        "href": Pact.Matchers.term({
                            matcher: "^(?=.*list-of-books)(?=.*category=)",
                            generate: "http://example.com/list-of-books?category=children"
                        })
                    }
                }
            },
            {
                "name": "Factual",
                "_links": {}
            }
        ]
    }
}

Problem: it still doesn't represent the reality, can't verify mock files that are different from this response Benefit: can test both case of having and not having the links

How I would like to write the interaction

I don't know if that's feasible, but if I had some kind of an "optional matcher", that would be applied or not applied depending of the existence of a node, it would help me write the interaction. I would also be able to verify any mock files and it would also represent the reality.

Something like (see the in the node property):

{
    "_embedded": {
        "categories": Pact.Matchers.eachLike({
            "name": Pact.Matchers.somethingLike("Children"),
            "_links": {
                "<Pact.Matchers.optional>doc:books": {
                    "href": Pact.Matchers.term({
                        matcher: "^(?=.*list-of-books)(?=.*category=)",
                        generate: "http://example.com/list-of-books?category=children"
                    })
                }
            }
        })
    }
}

Any thought of how to approach this problem?

bethesque commented 7 years ago

Hi Romuald,

This is a frequently asked question. See if this answers your query. If not, lets talk about it so I can add further clarifications. https://github.com/realestate-com-au/pact/wiki/FAQ#why-is-there-no-support-for-specifying-optional-attributes

Essentially, this is the key: "no test will ever fail to tell you that you've made an incorrect assumption". If you can work out how to make a test fail for an optional attribute, I would love to hear your thoughts.

I know it's a pain (and I've felt that pain), but I think option 2 is the only way to do it.

soundstep commented 7 years ago

Hi Beth,

Thanks for the references, very much appreciated!

The solution 2 works in the case of contracting the consumer with the provider, which is the goal of pact. But when it comes down to verifying mocks, any alterations from the hard-coded responses in the pact file would fail the verification. This is somehow crippling, and I feel less of a point in using pact if I can't verify mocks.

Let's imagine I need 100 fixtures json files to test special cases in an acceptance tests step, each fixtures files would have small alterations with optional fields and objects. If I want these fixtures files to be pact verified I would need to create provider states and 100 alterations for the same request. That still sounds a bit weird.

Also, how would I differentiate these 100 interactions from one another as they are the same request? With a dummy parameter? Pact-mock-service doesn't allow me to add the same request multiple, which makes complete sense.

/platform/books?testCase=1
/platform/books?testCase=2
...

Is verifying mocks the actual downside of pact because it hasn't been made for that in the first place? Any further thought appreciated if you have any!

Cheers.

Romu

soundstep commented 7 years ago

A small addition in case it helps other people.

We are going to a solution 3.

We won't hard-code responses, we will still use eachLike matchers. We will include every single optional (but used) fields so they are tested.

On the micro-service/API side, provider states will be used to generate the proper data with optional fields always present, no problem there.

On the mock verification side, and this is the key of the solution, we will only verify "standard" mocks (mocks with all optional fields present), and not verify other altered mocks (missing optional fields).

So this is still not perfect, because not all mocks are verified. But we are "less unhappy" and should be able to effectively catch broken contracts. Compared to the other 2 first solutions, the pact file sanity and tests made on back-end side is stronger.

Hope that make sense, and thanks again for your time @bethesque

Romu

bethesque commented 7 years ago

Ah, I see your problem more clearly now.

Let's imagine I need 100 fixtures json files to test special cases in an acceptance tests step

Personally, I don't use pact for acceptance tests, as I find they generate a million different interactions which are a pain to maintain, as you have found. The way I use pact is to test just the API client thoroughly (the one class through which all HTTP requests to your API go). Then I use a different mocking solution for the higher level tests. I would either mock at the HTTP level, and reuse the same fixtures that I used in the pact tests, or I would mock the API client class, and reuse some object that I shared between the pact tests and the acceptance tests to ensure they stayed in sync.

We won't hard-code responses, we will still use eachLike matchers. Yes, I would always use "like" by default for just about everything.

Also, how would I differentiate these 100 interactions from one another as they are the same request?

The mock service is only designed to have a few interactions loaded into it at a time, and to have a separate test for each one or two requests (ie. sometimes a client will hide the fact that it is making multiple requests to service one method call, for example, following HAL links). You'd need to put each of the hundred interactions into separate test cases I'm afraid.

On the mock verification side, and this is the key of the solution, we will only verify "standard" mocks (mocks with all optional fields present), and not verify other altered mocks (missing optional fields).

There used to be an option to do a more lax verification, but I can't remember the details. I've been trying to find it in the code.

Your solution sounds like it is giving you sufficient certainty without creating a million use cases, so it sounds reasonable to me.