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

Validate that the input to remember will not try to use the numeric % #91

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fireyone29
Copy link

The code in JsonSpec.remember assumes that the incoming argument will be a string and uses the % operator to interpolate from the memory hash into the string. However, it is perfectly possible to pass in a Numeric type (e.g. Fixnum) for which the % operator means something totally different which causes the code to try to convert the hash to fixnum, which it can't. This is especially problematic from the cucumber matchers, where your input is always run through remember.

I ran into the problem when trying to do a cucumber step like "And the JSON at "count" should be 10" but I am completely unable to reproduce it in this repo's cucumber tests (the 10 always comes in as a string some way I don't understand).

It appears possible to fix this either by doing .to_s on the income argument or changing the exclusion criteria to also include .is_a?(Numeric) (i.e. in addition to checking if the memory hash is empty). I think the is numeric check is more precise, so I'll submit a pull request with that change, but both solve my particular problem.

The code in JsonSpec.remember assumes that the incoming argument will
be a string and uses the % operator to interpolate from the memory
hash into the string.  However, it is perfectly possible to pass in a
Numeric type (e.g. Fixnum) for which the % operator means something
totally different which causes the code to try to convert the hash to
fixnum, which it can't.

Fixes #90
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