Integers from the provider fail validation against a decimal expectation

[albertotx]

Consider the following response from a provider:

"ratings": [
  {
    "id": 12,
    "count": 4,
    "average": 3.75
  },
  {
    "id": 37,
    "count": 1,
    "average": 4
  },
  {
    "id": 143,
    "count": 0,
    "average": 0
  },
  {
    "id": 574,
    "count": 6,
    "average": 4.333333
  },
]

The response body in the interaction is defined as follows (Javascript):

ratings: eachLike({
  id: 15,
  count: 20,
  average: 4.5
})

Validating the provider yields the following errors:

Expected a Float (like 4.5) but got a Fixnum (4)...
Expected a Float (like 4.5) but got a Fixnum (0)...

Please allow Fixnums in the provider response to validate successfully against Float expectations.

[bethesque]

I'm sorry, I know this is annoying, but you'll have to provide a float for your example data. We cannot support feature you ask for without breaking backwards compatibility. It may be possible in a later version of pact specification, but that is not yet supported in the Ruby implementation.

[albertotx]

The pact specification has its own problems and yes, those require a new specification. However, I do believe it is possible to fix this issue and maintain backward compatibility. I don't know how easy it would be.

1. Add a new option to the Pact Verifier, something like allowIntegersAsFloats. Have it set to false as the default.
2. If a validation of a float fails and the option is set to true, run the validation as an integer and try again. If it succeeds, then great! Otherwise, fail the test.

I am willing to help if you give me some pointers on where to change the validation.

[mefellows]

Would a configuration file be something we could look to introduce?

[albertotx]

The provider validator already has a set of options passed through the command line. It could be added there. That also means the option needs to be added to the runners in other languages that invoke the Ruby core.

[mefellows]

Those flags are really for a separate set of behaviours and configuring where the verification should take place (aside from the version of the specification obviously). What I'm talking about is more something like a tsconfig.json (pactconfig.json?) that specifies specification behaviour. Perhaps this is a way of working around the specification version problem.

I totally understand your point though, just thinking of standard approaches.

I suppose having flags on the binary would be like a CFLAGS type thing, so another option could be an environment variable.

Whatever approach we take, it must be portable to other languages (hence why a config file / environment variable) would be a more standard way of approaching it.

[vgrigoruk]

I'm experiencing exactly the same issue at the moment :-( I providing float in my example data that goes to contract (e.g. { value: like(0.1) }), but actual provider returns { value: 0 } during verification process and it fails.

I think the issue should be reopened @bethesque

btw, @albertotx did you manage to find a fix or workaround?

[mefellows]

Just did a quick spike locally, and modifying this file with this extra condition as per below resolves the issue:

|| (expected.is_a?(Float) && actual.is_a?(Fixnum))

As Beth mentioned, however, this would result in a backward-incompatible change so it would at best need to be wrapped behind a feature flag. I'd defer to Beth here on the best path forward, but I do hear your pain.

[albertotx]

@vgrigoruk no, we did not find a workaround and we abandoned the effort altogether. Talking with developers at a conference recently, many have simply removed these problematic assertions from their contracts. Others suggested using Hoverfly.

@mefellows thank you for proving how simple the solution really is and yes, this should be enabled with a flag to avoid the backward compatibility problems. A couple of months ago you provided several solid options on how to set this flag. Adding this flag would be the biggest effort, unfortunately.

@bethesque Please reconsider reopening the ticket and joining the discussion. I think we are getting close to finding a solution.

[albertotx]

@mefellows, I've been thinking more about the need for a flag to enable this feature, I am left wondering if it's really necessary. Yes, this is a significant change in long-standing behavior. Yes, this change may cause existing failing assertions to pass. Either of these conditions prudently demands a feature flag. But from the mathematical perspective this is merely applying the correct mathematical principle that every integer is a rational number. Users are currently circumventing this unexpected behavior by authoring test data or worse, removing assertions from the contract. Neither of these existing workarounds would be impacted by this change.

Perhaps a survey on Slack would help us get a better feel for the real impact this change would have on users. I bet many if not most would not be negatively impacted and may welcome the change, but I could be wrong.

[mefellows]

Actually it turns out talking to @uglyog that in other implementations prior to spec version 3, numbers are all treated equally. I can't find any specifics about it in the specs myself (either in the test cases or as prose). So in the below PR, floats/integers are now made equivalent.

So it's rather straightforward to enable this new behaviour in Pact JS, as Beth has kindly provided a break glass approach to monkey patch things. You can see an example PR for pact-js here.

It should probably/possibly be wrapped in a flag, but is indicative only at this moment (it also shows how you might go about overriding this behaviour currently).

Keen to hear people's thoughts...

[bethesque]

Integers/Floats have never been treated as the same in the ruby impl - I'm surprised to hear they are in pact-jvm! That monkey patch is a good idea until we release an official solution @mefellows @albertotx. Let me have a think about the cleanest way.

[mefellows]

Unfortunately I didn't charge my laptop over the weekend and so can't get this out behind a feature flag / alpha this morning. I'll aim for something this week.

As for the Ruby change let's keep that conversation going.

[bethesque]

I meant for @albertotx to do it on his codebase, rather than monkeypatching pact-js. We'll get something into the ruby codebase properly.

[bethesque]

Lolz, you know something I'd forgotten? In ruby (1.0 == 1) == true. It's only when you do type based matching that 1.0 does not equal 1. Your case for changing the default matching is strengthened.

[albertotx]

Thank you @mefellows and @bethesque for the patch and for the discussion. I will test the monkey patch locally. It may not be until later today or tomorrow that I'll be able to test it.

Again, a big thanks.

[bethesque]

Ok, I've put out version 1.67.0 of the ruby standalone. I was tempted to make a float match an int, but an int not match a float as a kind of backwards compatible intermediate step, but then I thought about the whole (1.0 == 1) == true thing, and decided it wasn't worth it.

I haven't exposed a flag to use the old matching logic, but I can if anyone asks for it.

[mefellows]

Cool, I'll update Pact JS et. al., today. I think this is the right move.

[mefellows]

For posterity, the updated release that went out was 1.68.0 of the standalone. Version 8.6.0 of Pact Node going out, and I'll update Pact JS to use the latter dependency soon.

[albertotx]

I upgraded to latest JS version (8.2.6) and that in turn upgraded Pact Node to 8.6.0 which includes the Ruby fix by @bethesque. My assertions now work!!!

Thank you so very much for this change! It puts our contract testing effort back on track.

Thank you! Thank you! Thank you!

[mefellows]

That's great news - thanks for following back up @albertotx. All the best :)