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

Allows variables that evaluate to falsy to be returned #23

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

rcrichton
Copy link

The current code skips particular scopes that have a variable defined when the variable is falsy. This is no good as legitimate values such as empty string and boolean 'false' are then skipped and end up being set to undefined.

I'm adding this here as this seem to be an active fork of the original. Hopefully this may be merged back at some point.

The current code skips particular scopes that have a variable defined
when the variable is falsy. This is no good as legitimate values such
as empty string and boolean 'false' are then skipped and end up being
set to undefined.
@thim81
Copy link
Collaborator

thim81 commented Nov 4, 2021

@rcrichton apologies for the late response, forgot to turn on notifications on my repo.

I ll review your PR more in detail and afterwards merge and release it.

@thim81
Copy link
Collaborator

thim81 commented Nov 4, 2021

@rcrichton Could you provide me a small example of a postman collection where this happens? That way I can add a test for it, to prevent regression in future releases

@thim81 thim81 self-assigned this Nov 4, 2021
@thim81 thim81 merged commit 28e5acc into apideck-libraries:main Nov 22, 2021
@thim81
Copy link
Collaborator

thim81 commented Nov 22, 2021

@rcrichton Your PR changes are now included 1.8.3 release

@rcrichton
Copy link
Author

Thanks, it's been on my todo list to get back to this, I'm glad you were happy to merge it anyways.

@thim81
Copy link
Collaborator

thim81 commented Nov 23, 2021

@rcrichton feel free to still share a small example of a postman collection where the behaviour occurs, so that we can create a test for it.

@rcrichton
Copy link
Author

rcrichton commented Nov 29, 2021

I created a test postman collection.

To try this out:

Using 1.8.2, pre my PR

npx @apideck/[email protected] postman-to-k6-bug.postman_collection.json -o script.js && docker run -iv `pwd`:/home/k6 loadimpact/k6 run --http-debug=full /home/k6/script.js

Using 1.8.3, after my fix

npx @apideck/[email protected] postman-to-k6-bug.postman_collection.json -o script.js && docker run -iv `pwd`:/home/k6 loadimpact/k6 run --http-debug=full /home/k6/script.js

The collection points to an httpbin endpoint that return exactly what was sent. Notice in the 1.8.2 output in the http debug log message that what was sent is incorrect:

\"data\": \"{\\n  emptyString: 'undefined',\\n  boolean: undefined\\n}\"

Where as the 1.8.3 output is correct:

\"data\": \"{\\n  emptyString: '',\\n  boolean: false\\n}\"

@thim81
Copy link
Collaborator

thim81 commented Nov 29, 2021

@rcrichton Thanks for sharing the test collection and details.
I've create a follow-up item #27 for me to add this to the test suite.

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