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

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

Closed
2 of 4 tasks
timvahlbrock opened this issue Apr 17, 2024 · 5 comments
Labels
documentation Indicates a need for improvements or additions to documentation triage This issue is yet to be triaged by a maintainer

Comments

@timvahlbrock
Copy link
Contributor

Software versions

Please provide at least OS and version of pact-js

  • OS: Win 11
  • Consumer Pact library: 12.4.0
  • Provider Pact library: irrelevant
  • Node Version: 21.6.0

Issue Checklist

Please confirm the following:

  • I have upgraded to the latest
  • I have the read the FAQs in the Readme
  • [ x I have triple checked, that there are no unhandled promises in my code and have read the section on intermittent test failures
  • I have set my log level to debug and attached a log file showing the complete request/response cycle (not possible due to GDPR on main project, can generate log using reproduction repro if required)
  • For bonus points and virtual high fives, I have created a reproduceable git repository (see below) to illustrate the problem

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 974d247#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 timvahlbrock added bug Indicates an unexpected problem or unintended behavior triage This issue is yet to be triaged by a maintainer labels Apr 17, 2024
@timvahlbrock
Copy link
Contributor Author

timvahlbrock commented Apr 17, 2024

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
Copy link
Contributor Author

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

@timvahlbrock
Copy link
Contributor Author

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

@mefellows
Copy link
Member

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 mefellows added documentation Indicates a need for improvements or additions to documentation and removed bug Indicates an unexpected problem or unintended behavior labels Apr 17, 2024
timvahlbrock added a commit to timvahlbrock/pact-js that referenced this issue Apr 17, 2024
Adapted `eachLike` V3 description as discussed in pact-foundation#1207
timvahlbrock added a commit to timvahlbrock/pact-js that referenced this issue Apr 17, 2024
Adapted `eachLike` V3 description as discussed in pact-foundation#1207
@mefellows
Copy link
Member

Closed by #1208.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Indicates a need for improvements or additions to documentation triage This issue is yet to be triaged by a maintainer
Projects
None yet
Development

No branches or pull requests

2 participants