-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat: Converters support pm methods: success, jsonBody, jsonSchema #4856
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
base: main
Are you sure you want to change the base?
Conversation
|
Linked to #4857 |
|
Hi @helloanoop Offcourse, I ll have a look and try to add additional tests that cover the 3 new pm translations. |
|
hi @helloanoop I added the requested unit tests, let me know if you have remarks or changes needed. |
| } | ||
| }, | ||
|
|
||
| // pm.response.to.have.jsonBody → expect(res.getBody()).to.satisfy(u => Array.isArray(u) || (u !== null && typeof u === "object") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the intelliSense i got to understand, pm.response.to.have.jsonBody can accept arguments too, i believe 90% of the people will be happy with just JsonBody(). also i couldn't find any extensive documentation on how postman handles those arguments. Let me know you thoughts on this!
pm.response.to.have.jsonBody()
pm.response.to.have.jsonBody(optionalExpectEqual:Object)
pm.response.to.have.jsonBody(optionalExpectPath:String)
pm.response.to.have.jsonBody(optionalExpectPath:String, optionalValue:*)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not aware of the jsonBody() function supporting arguments.
The PR handles only pm.response.to.have.jsonBody(), which was the case that Portman generates, which is why I'm doing this PR.
Do you want to me to extend the support for all the possible arguments? Or built in a detection in that case that fallbacks to the commenting out of the // pm.response.to.have.jsonBody(optionalExpectEqual:Object)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did some browsing in the Postman documentation and github.
In their docs I did not found a clear explanations, but on Github there was an issue referenced.
Based on that, I added the support for the arguments (to my best knowledge of Chai) and added additional tests to validate the behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @thim81 Thanks for this. let me know when you push the latest changes
I did some browsing in the Postman documentation and github. In their docs I did not found a clear explanations, but on Github there was an issue referenced.
Based on that, I added the support for the arguments (to my best knowledge of Chai) and added additional tests to validate the behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes have been pushed for pm.response.to.have.jsonBody()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thim81 i couldn't find any related changes on last commit , Am i missing something here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I committed, but forgot to push the changes 🤦 . The PR should now be updated for pm.response.to.have.jsonBody()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @thim81 we are having an internal discussion on what should be the translations, like supporting jsonBody, jsonSchema, to make the translations more straight forward, will get back to you soon.
| } | ||
| }, | ||
|
|
||
| // pm.response.to.have.jsonSchema → const tv4 = require("tv4"); expect(tv4.validate(res.getBody(), schema), tv4.error && tv4.error.message).to.be.true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thim81 when i searched online, i came to know that pm.response.to.have.jsonSchema accepts 2 arguments, second one is optional, from the quick glance it seems like, postman is using "ajv" under the hood to validate the jsonSchema, can we also use the same. i think ajv is available across our script runtimes just like tv4, probably will be easier for us to pass ajvOptions if any user explicitly using the second argument.
pm.response.to.have.jsonSchema(schema:Object)
pm.response.to.have.jsonSchema(schema:Object, ajvOptions:Object)
@helloanoop @maintainer-bruno @lohit-bruno @naman-bruno currently "ajv" is only available on developer mode, so the script will fail in safe mode if we adopt ajv, to be in parity with postman, we probably need to have "ajv", what should we do here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know, what the final suggestion will be: using Ajv or tv4?
I took tv4, because of it being mentioned in guides and not needing the developer mode.
Postman is indeed using Ajv behind the scenes: https://learning.postman.com/docs/tests-and-scripts/write-scripts/postman-sandbox-api-reference/#validate-response-data-with-a-json-schema
Changing the PR to switch to Ajv, should be straightforward to do.
…nd pm.response.to.have.jsonSchema logic
|
Is there any news on the tv4 <> AJV preference? |
@thim81 we will most likely be going with ajv, we plan to support jsonSchema, Json within chai using extensions, so that the translations can be easier and straight forward. We haven't yet started working on it. we will work on it soon. Once that PR is merged, I will ping here so that you can make necessary changes to the translation PR. |
|
#2519 related |
|
@sanish-bruno Any news on the item? I see that there conflicts on the PR, should I update them already or wait untill there is movement on the quickjs-emscripten PR? |
@thim81 i think it is better to wait until the quickjs-emscripten. We have been caught up with other issues. So we still haven't got the time to discuss this. I think, It is better to wait! |
|
Is there any news or plans on this PR? |
Description
The Bruno Converters package, already converts a number of Postman methods into Bruno methods.
This PR adds support for the very common methods:
Jira
BEFORE

AFTER

Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.