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

Added Min and Max to pactum-matchers for int and float #24

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

nergmada
Copy link

This PR adds optional non-breaking min and max criteria for ints and floats in pactum-matchers. I have that updates pactum to use this project as a PR dependency that would make debugging easier in the future.

@nergmada
Copy link
Author

Related PR: pactumjs/pactum#306

@ASaiAnudeep
Copy link
Member

We already have lt and gt for comparing min and max

@nergmada
Copy link
Author

How do you ensure that a number is in a range with lt and gt? Like how would I express

0 <= x <= 10

@nergmada
Copy link
Author

Let me know. I would like to dive deeper into adding more complex matchers

@ASaiAnudeep
Copy link
Member

One way to solve is to call expectJsonMatch multiple times

await spec()
  .get('https://randomuser.me/api')
  .expectJsonMatch({
    "age": gt(0)
  })
  .expectJsonMatch({
    "age": lt(10)
  });

Other ideas would be (need dev effort)

  1. Create a new matcher like "age": range(0,10)
  2. Find a way to create and use compound matchers like "age": match(gt(0), lt(10))
  3. Extend existing macher - int(5, { min: 0, max: 10 }) or int({ min: 0, max: 10 })

@@ -212,6 +212,25 @@ function compareWithInt(actual, rule, path) {
if (type !== 'number') {
throw `Json doesn't have type 'number' at '${path}' but found '${type}'`;
} else {
if ('options' in rule && rule.options !== undefined) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this new code to a common function and reuse for both int and float comparisons

@ASaiAnudeep
Copy link
Member

@nergmada your changes are looking good. Could you please add some test cases.

@ASaiAnudeep
Copy link
Member

@nergmada are you planning to add unit tests for your changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants