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

getMissingRequiredVariables report missing falsy yet valid values #164

Closed
wants to merge 4 commits into from

Conversation

theobat
Copy link
Contributor

@theobat theobat commented May 21, 2017

Hi @mattkrick,

I found a problem while trying to query something containing a floor in a very basic form. Floor is expected to be a non null Int but very often equals 0. I found out that the falsy values are not handled in
getMissingRequiredVariables. This PR solves the problem by specifically looking for null or undefined.

Edit: coverage decreases because the queryHelpers.js file was not in the tests percentage before and I only test one of the few functions of the file

@coveralls
Copy link

coveralls commented May 21, 2017

Coverage Status

Coverage decreased (-4.01%) to 72.388% when pulling 1d97208 on theobat:master into da2e138 on mattkrick:master.

@theobat
Copy link
Contributor Author

theobat commented May 23, 2017

Also if you don't have time to review this in the near future please let me know so that I use my fork as a dependency instead. Thanks.
@mattkrick

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