-
Notifications
You must be signed in to change notification settings - Fork 73
db/redis: add tests for FieldMap() #141
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
Conversation
Current coverage is 85.15% (diff: 100%)@@ master #141 diff @@
==========================================
Files 28 28
Lines 1907 1907
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
Hits 1624 1624
Misses 188 188
Partials 95 95
|
@@ -11,6 +11,8 @@ import ( | |||
"sync" | |||
"testing" | |||
"time" | |||
|
|||
"github.com/NYTimes/video-transcoding-api/db" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A quick note that this package doesn't depend on anything else in the video-transcoding-api on purpose: it's an independent library and if we want to extract this as an external repo, we could, and then users wouldn't need to download the video-transcoding-api code.
You tests don't change this because you're importing code from video-transcoding-api only in the test file, but the ideal approach would be to add a []struct{} field to some of the types declared in stub_test.go
and use those types in the tests for FieldMap.
@fsouza should be done now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks!
Initially I started this branch to solve the issue #139 so I wrote tests to instrument the modifications. The code to support flatten arrays of structs into hashes sounds like a workaround and I decided to give up.
Talking with @fsouza we'll probably abort this approach and save the json as a string.