pact-go icon indicating copy to clipboard operation
pact-go copied to clipboard

Incorrect handling of JSON tags

Open askingcat opened this issue 4 years ago • 3 comments

  • Consumer Pact library: Pact go v1.4.3
  • Golang Version: 1.14

Expected behaviour

The names for JSON fields should be derived from the go JSON tag the same way as the default JSON encode does.

Actual behaviour

pact-go requires to use a json tag and always uses the complete content of the tag as a name. This leads to the following interpretations:

1. Name `json:"name,omitempty"` -> "name,omitempty"; should be: "name"
2. Name -> ""; should be "Name"
3. Name `json:"-"` ->   "-" ; should be ignored

askingcat avatar Jun 30 '20 05:06 askingcat

Thanks for the report. Can you please show/demonstrate the code you are using to ensure we understand the requirements better? As it stands this request is quite vague.

mefellows avatar Jun 30 '20 05:06 mefellows

Thanks for the fast feedback!

I would like to be able to reuse the structs I have defined for my API for my contract tests. Some of these structs have optional fields, which are filled most of the times anyway. As an example, I often use an error response similar to the following:

type ErrorResponse struct {
	ErrorID        string `json:"errorID"` // <- required
	ErrorMessage   string `json":errorMessage,omitempty"` // <- optional, human readable message; used most of the times, but not always
}

While verifying the ErrorMessage field will be mapped to "errorMessage,omitempty" instead of "errorMessage".

So it would be very useful if I could use the matcher with this struct directly:

dsl.Response{
	Body:    dsl.Match(&ErrorResponse{}),
}

While checking the implementation within go I discovered some additional cases listed above. I hope the test cases in the linked PR could help as well.

askingcat avatar Jun 30 '20 06:06 askingcat

Oh, dear, I didn't see your PR - that would have clarified it for me! Apologies (and thanks!).

It makes sense. I'll take a look tomorrow in more detail and bring it in if it's a sensible starting point.

Keen for other feedback also, there was recently a discussion about expanding this feature recently (see https://pact-foundation.slack.com/archives/C9UTHTFFB/p1591994063229900).

If you're up for it, please join us in #pact-go at slack.pact.io and we can bring that chat out to a separate issue/feature here.

mefellows avatar Jun 30 '20 06:06 mefellows