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

Fix datahelper tests #121

Merged
merged 2 commits into from Aug 15, 2014
Merged

Conversation

@anderspitman
Copy link
Contributor

anderspitman commented Aug 15, 2014

I believe these tests broke with commit 0b010fe

At that point joinKey was passed through but the test wasn't expecting it so the deep.equals failed. However, I'm not sure about all the details of how the bolus conversion is supposed to work so maybe the test is failing because of a true bug. I suspect it is the test that is in error though.

I'm also rolling #120 into this PR.

Tests were failing due to deep equals not finding joinKey.
Adding joinKey to the compared arrays fixes it.
@nicolashery
Copy link
Contributor

nicolashery commented Aug 15, 2014

Thanks @andersp! Good catch on the documentation & bug in the tests.

I'll merge this in just a bit, but before I can we have a little formality: could you please agree to our CLA, see "Making Contributions" at http://tidepool-org.github.io/Contributors.html, by pasting in the text "I agree to the terms of..." here in this GitHub PR? Thanks!

@anderspitman
Copy link
Contributor Author

anderspitman commented Aug 15, 2014

You bet:

I agree to the terms of Tidepool Project’s Contributor License Agreement as it exists at http://tidepool-org.github.io/TidepoolCLA.pdf on 2014-08-15

nicolashery added a commit that referenced this pull request Aug 15, 2014
Fix datahelper tests
@nicolashery nicolashery merged commit fedd8ea into tidepool-org:master Aug 15, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.