fix: Replace falsy values with json selector #666
Conversation
There was a problem hiding this comment.
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.
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_hiddenwith 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
|
@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. |
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
quickpizzafor 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