Closed ultimateboy closed 7 years ago
@Joshua-Anderson is a potential reviewer of this pull request based on my analysis of git blame
information. Thanks @ultimateboy!
One minor thing I noticed, otherwise LGTM.
Nice catch @Joshua-Anderson. I'm going to spend a bit of time investigating why that was passing as it was written. But fixed.
I'm thinking it was passing because the SDK returned 1
item but the count
on the object was set to two. The code doesn't look at the requested limit, it only looks at the effective limits (returned items vs total count).
Could you possibly add limits tests for KeysList()
?
I also noticed that we still need tests for limits on app.Logs()
. If you want to want to wait and do these in a different PR that's perfectly fine as well.
Thanks @Joshua-Anderson. Somehow missed the keys test. Added.
After my changes to codecov, you need to rebase. Sorry!
@@ master #230 diff @@
==========================================
Files 56 56
Lines 3861 3861
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 1009 1038 +29
+ Misses 2719 2684 -35
- Partials 133 139 +6
Powered by Codecov. Last update a330059...d883144
Fixes deis/workflow-e2e#196 Fixes deis/workflow-e2e#198 Fixes deis/workflow-e2e#199 Fixes deis/workflow-e2e#200 Fixes deis/workflow-e2e#201