-
Notifications
You must be signed in to change notification settings - Fork 15
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
Provide option to save bodies as JSON objects #487
Provide option to save bodies as JSON objects #487
Conversation
Added in setSaveStringBodies to allow for non-string bodies in request and response
Added unit tests for default and setting values for new saveStringBodies
FromHarContent needed to have the same check as the others to return text as is if the saveStringBodies was false
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.
@KayleighYum Thank you very much for your PR! It is a good improvement for kassette.
I have left some comments about the things I think should or could be changed before integrating this new feature.
Thanks @divdavem! I'll take a look at the comments |
Made changes based on feedback from PR
Broken link in the api docs generation calling method name used in an earlier iteration of this PR
@divdavem I have fixed the issue with the failing api doc generation. Apologies about that |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #487 +/- ##
==========================================
+ Coverage 91.52% 91.64% +0.11%
==========================================
Files 36 36
Lines 1192 1209 +17
Branches 268 273 +5
==========================================
+ Hits 1091 1108 +17
Misses 50 50
Partials 51 51
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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 @KayleighYum !
Thank you very much for the changes you have already made following my comments and to fix the build failure.
If it is ok for you, I have added some more comments to improve the code before we can integrate it.
Thank you!
Made adjustments based on feedback from the pr
Hey @divdavem I've made the adjustments based on your feedback, please can you resolve the conversations if you're happy with the changes or let me know if I've missed anything |
Added some more unit tests to increase code coverage
Co-authored-by: divdavem <[email protected]>
…hYum/kassette-stringified-json into feature/non-stringified-json-har
Moved body.toString evaluation
@KayleighYum Thank you very much for your PR! It is now integrated. 🎉 |
A new configuration option has been added,
saveStringBodies
, which is set to true by default. This can be changed with the newsetSaveStringBodies
method. A new parameter has been added totoHarPostData
andtoHarContent
wheresaveStringBodies
will be passed in.When
saveStringBodies
is set to false and the mimeType of the request/response is application/json the body will be json parsed and saved.This will only be applied when the
mockFormat
is 'HAR'.This will be applied to both request (when
saveInputRequestBody
is true) and response bodies.