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

Issue with matching a list of enums #27

Open
rholshausen opened this issue Jun 7, 2023 · 15 comments
Open

Issue with matching a list of enums #27

rholshausen opened this issue Jun 7, 2023 · 15 comments
Labels
bug Something isn't working

Comments

@rholshausen
Copy link
Contributor

Original issue: pact-foundation/pact-plugins#31

In the matcher do we support list of enums?

  "request": {
    "states": [
      "matching(equalTo, 'ENUM_VAL_1')"
    ]
  },

I’m getting this error --- is it because enum is not added to should_be_packed_type?

Failed to match the request message - BodyMismatches({"$.states": [BodyMismatch { path: "$.states[0]", expected: Some(b"ENUM_VAL_1"), actual: Some(b"0102"), mismatch: "Expected and actual field have different types: 1:(states, Varint, Enum) = ENUM_VAL_1 and 1:(states, LengthDelimited, Unknown) = 0102" }]})

@rholshausen rholshausen added the bug Something isn't working label Jun 7, 2023
@rholshausen
Copy link
Contributor Author

@Yu-Xie are you able to provide the proto file and test code?

opicaud pushed a commit to opicaud/pact-protobuf-plugin that referenced this issue Sep 6, 2023
@stan-is-hate
Copy link
Contributor

Hey folks, I'm running into the same issue. Any way I can help to fix it? Don't have any rust experience tho :(
It is blocking our team from implementing contract tests for several of the services, so I'd be happy to contribute in any way I can.

@rholshausen
Copy link
Contributor Author

A reproducible example project will help us diagnose the problem and then work out how to fix it

@stan-is-hate
Copy link
Contributor

I'll try to produce something in the coming days

@stan-is-hate
Copy link
Contributor

Here's the sample:
https://github.com/stan-is-hate/pact-proto-issue-demo
It requires pact-go and protobuf-plugin both installed, and then you can run a broken test to see the error and the working one to see that otherwise it all works as expected:

gRPC transport running on {59410 127.0.0.1}
--- FAIL: TestBrokenProto (0.96s)
    /Users/stanislav.vodetskyi/github/pact-proto-issue-demo/pact_consumer_test.go:73: rpc error: code = FailedPrecondition desc = Failed to match the request message - BodyMismatches({"$.type": [BodyMismatch { path: "$.type[0]", expected: Some(b"TYPE1"), actual: Some(b"0101"), mismatch: "Expected and actual field have different types: 1:(type, Varint, Enum) = TYPE1 and 1:(type, LengthDelimited, Unknown) = 0101" }]})
FAIL
FAIL	github.com/stan-is-hate/pact-proto-issue-demo	1.350s
FAIL

@rholshausen
Copy link
Contributor Author

Thank you for the sample project, I can replicate it with Rust code.

@rholshausen
Copy link
Contributor Author

I've fixed the encoding of repeated enum fields.

@stan-is-hate the other issue is that this config does not make sense:

               "request": {
			"type": [
				"notEmpty('TYPE1')"
			]
		},

This is configuring the type field to have one item, and whose value must not be empty. But the way Protobuf treats enums, they can not be "empty", as missing values are always substituted with the default value. Also, the notEmpty matcher is meant to be applied to collections (maps and repeated fields in Protobuf).

If you want to state that the type field must not be empty (as in no enum values), that is valid and you can do that with:
sense:

               "request": {
			"type": "notEmpty('TYPE1')"
		},

@stan-is-hate
Copy link
Contributor

Thanks @rholshausen ! I've copied the structure of the example from the actual protos we use in our services.

In our case type is a repeated field of type Type, which is enum. From the provider perspective, it currently enforces this field to have a single element only, but this will change in the future to support multiple values.
How do I encode this from the consumer side, so that it should have either one-and-only-one or at-least-one item?

@rholshausen
Copy link
Contributor Author

I would just set the test up to accept at least one value. That will work with your current provider and work in the future. You can't do an or.

@stan-is-hate
Copy link
Contributor

Yeah, that's what I mean, how do I express "at least one value"?

@rholshausen
Copy link
Contributor Author

               "request": {
			"type": "notEmpty('TYPE1')"
		},

should do it.

I'm currently adding two options to specify the min and max length as well.

@stan-is-hate
Copy link
Contributor

ah, interesting, so "type": "notEmpty('')" would process that it is a non-empty list, gotcha. Got confused at first :)
Thanks, this is awesome!

@stan-is-hate
Copy link
Contributor

what if the list contains structured elements (ie other protos), how would you define "at least one" in that case? If at all possible?

@rholshausen
Copy link
Contributor Author

I'm currently adding atLest(num) and atMost(num) rules to do that.

@stan-is-hate
Copy link
Contributor

Repeated enums seem to work ok in my consumer test, thanks for a quick fix!

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

No branches or pull requests

2 participants