-
Notifications
You must be signed in to change notification settings - Fork 11
fix: Replace falsy values with json selector #666
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
Conversation
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.
Pull Request Overview
This PR fixes the issue where JSON body selectors were not matching falsy values by updating the condition check to only return when the value is undefined.
- Changed condition in replaceJsonBody from a falsy check to a strict undefined check.
- Added tests to cover scenarios with empty string and boolean values in JSON bodies.
Comments suppressed due to low confidence (2)
src/rules/shared.ts:258
- Consider adding a test case for when the JSON property is null, if null is a possible value, to ensure the intended behavior is maintained.
if (valueToReplace === undefined) return
src/rules/shared.ts:573
- It may be beneficial to include tests for additional edge cases, such as when JSON values are explicitly null or other uncommon falsy values, to improve overall test coverage.
})
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.
Works well for parameterization, but I found issues when trying to extract a falsy value from a correlation.
- I tested the correlation rule using this endpoint
https://pokeapi.co/api/v2/pokemon/ditto
- Attempt to extract
abilities[0].is_hidden
with no success
I did some debugging and found two missing places for it to work for correlation rules. We need to explicitly compare against undefined
, similar to what you did in the PR.
k6-studio/src/rules/correlation.ts
Line 128 in f1f035a
if (extractedValue && correlationExtractionSnippet) { |
if (!extractedValue) { |
@cristianoventura good catch! This is an interesting case, because usually you wouldn't want to replace all "false" value, but perhaps, with the combination of custom replacer, this could be used to have dynamic boolean in payload depending on a previous response. |
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.
LGTM! 🚀
Description
This fixes the issue when JSON body selectors don't work on falsy properties (empty string, boolean false).
How to Test
Create a parameterization or correlation rule targeting a falsy property using JSON body selector.
Expected:
Rule matches and value replaced
Current:
Rule doesn't match
Warning
Don't rely on
quickpizza
for testing this because it sets incorrect content type (text/plain) and JSON selectors are not applied.Checklist
npm run lint
) and all checks pass.npm test
) and all tests pass.Screenshots (if appropriate):
Related PR(s)/Issue(s)
Resolves #654