Skip to content
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

Matchers not working according to docs #308

Open
MdotMertens opened this issue Sep 28, 2022 · 5 comments
Open

Matchers not working according to docs #308

MdotMertens opened this issue Sep 28, 2022 · 5 comments
Labels
type:bug Something isn't working

Comments

@MdotMertens
Copy link

MdotMertens commented Sep 28, 2022

Hello,
I want to use the matchers in my consumer tests but using them is kinda weird.
The way that I would expect them to work according to docs is:

    event = {
        "event": "delivery:created",
        "delivery_type": matchers.Term("express|standard", "standard"),
    }

    (
        pact_no_publish.given("Delivery has standard shipping")
        .expects_to_receive("delivery:created")
        .with_content(event)
        .with_metadata({"Content-Type": "application/json"})
    )

What this does is to pass the Term object into the message.

What I have to do in this case is this:

    event = {
        "event": "delivery:created",
        "delivery_type": matchers.Term("express|standard", "standard").generate()["data"]["generate"],
    }

This works but is the same as passing in a string directly and defeats the purpose of the matcher.

@mefellows
Copy link
Member

Thanks for the report, and that doesn't seem right to me at all. Are you able to provide a repro we can use to look at this?

P.S. matchers.Term("express|standard", "standard") - I think you are looking for an enum-like matcher here.

You probably actually want matchers.Term("^express|standard$", "standard") which will match the exact value. It's unlikely those words are going to be a subset of others, but you might want to know that for future matching.

@MdotMertens
Copy link
Author

Sure, here it is:
https://github.com/MdotMertens/python-pact-matchers

In it I reproduced the bug by using the Like matcher.
It passes the matcher into my consumer function instead of an actual value.

@mefellows
Copy link
Member

Ah, thanks! I see the problem.

In other languages, the event would have the matchers stripped. This can be done via the reify command on the pact-message command.

Screen Shot 2022-09-29 at 10 37 23 am

    with pact_no_publish:
        handler(event=event, context={})

I don't know Python very well, but ideally the with pact_no_pubish would provide the reified (body without matchers etc.) to the block, so that handler could be invoked with the cleaned JSON blob, and not with the matchers.

It should be fairly straightforward in the meantime to create a utility function that recurses the event object and detects the presence of matchers, and extracts the value. Then your test should work as expected, and the matching rules will be properly serialised into the contract:

    with pact_no_publish:
        reified_event = reify(event)
        handler(event=reified_event, context={})

This is a bug, IMO, or at the very least, we should provide instructions as to how to use this better.

@MdotMertens
Copy link
Author

MdotMertens commented Oct 17, 2022

Had a go at it and the code was actually pretty straight forward.
The implementation for this was even there it was just not used.

The context manager does nothing for it and the unit tests are a knit a bit too tightly for implementing it in the context manager, since this would require a lot of change. It just handles cleaning up the test, there is nothing that gets setup when entering the conetxt.

Would love to get some feedback and to get this merged/improved. 😄

@ims-swilkinson
Copy link

@MdotMertens, you can make it work by calling matchers.get_generated_values(event), this will return a "reified" version of event that you can pass into your handler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants