pact-foundation / pact-js

JS version of Pact. Pact is a contract testing framework for HTTP APIs and non-HTTP asynchronous messaging systems.
https://pact.io
Other
1.62k stars 343 forks source link

`eachLike` behaves like `atLeastOneLike` when min is not explicitly set - contrary to documentation #1207

Closed timvahlbrock closed 5 months ago

timvahlbrock commented 5 months ago

Software versions

Please provide at least OS and version of pact-js

Issue Checklist

Please confirm the following:

Expected behaviour

eachLike sets a min of 0 by default or the documentation is corrected.

Actual behaviour

eachLike sets a min of 1 by default. The documentation states that the length is not checked. Likely introduced in https://github.com/pact-foundation/pact-js/commit/974d247444260ccbcc8709bd4409eea44d10a858#diff-884c1c7cbdaafa772a7a428e0ca83ce1fc58f03972dfa0fabcb89106d6ac35f5

Steps to reproduce

Use eachLike for an array property. Run pact consumer tests. See the property received a min of 1. Should look something like this:

            "$[*].theArrayProperty": {
              "combine": "AND",
              "matchers": [
                {
                  "match": "type",
                  "min": 1
                }
              ]
            },

Relevant log files

Please ensure you set logging to DEBUG and attach any relevant log files here (or link to a gist). As described, I can add logs from a reproduction repo if desired, but I think this might not be required here.

timvahlbrock commented 5 months ago

I do understand why it is not safe to allow empty arrays, as the provider might 'cheat' itself into matching the contract by providing an empty array. But if I get this correctly, eachLike currently mostly behaves like atLeastOneLike, or atLeastLike. Maybe the documentation might needs updating, maybe eachLike should be removed altogether.

timvahlbrock commented 5 months ago

Maybe eachLike should be made into an alias of atLeastOneLike but also be documented as such.

timvahlbrock commented 5 months ago

Personally favor aliasing over removing, as eachLike still communicates, that this value might be empty in production.

mefellows commented 5 months ago

Perhaps. eachLike was the original matcher for arrays and this has always been the semantics of it. New matchers such as atLeastLike, atMostLike and constrainedArrayLike really capture a wider set of use cases.

I agree, eachLike feels a bit lost with the introduction of these additional matchers.

Maybe eachLike should be made into an alias of atLeastOneLike but also be documented as such.

They aren't actually the same thing (different signatures), so aliasing them won't address the problem. They can't also be made to be the same without a breaking change, and that's probably overkill. The simplest would be to improve the doc string to indicate it always has a min of 1, or error if you try to set it less than 1.

mefellows commented 5 months ago

Closed by https://github.com/pact-foundation/pact-js/pull/1208.