-
Notifications
You must be signed in to change notification settings - Fork 11
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
mongo7 upgrade #99
mongo7 upgrade #99
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #99 +/- ##
=============================================
- Coverage 71.09% 70.06% -1.04%
+ Complexity 1457 1437 -20
=============================================
Files 91 91
Lines 5048 5034 -14
Branches 478 479 +1
=============================================
- Hits 3589 3527 -62
- Misses 1239 1296 +57
+ Partials 220 211 -9 ☔ View full report in Codecov by Sentry. |
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.
Note to self - didn't review MongoUtils, MongoDBHelper changes yet
src/test/java/us/kbase/test/narrativemethodstore/db/mongo/MongoDynamicRepoDB2Test.java
Outdated
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoDynamicRepoDB.java
Outdated
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoDynamicRepoDB.java
Outdated
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoDynamicRepoDB.java
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoDynamicRepoDB.java
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoDynamicRepoDB.java
Outdated
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoDynamicRepoDB.java
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoDynamicRepoDB.java
Outdated
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoDynamicRepoDB.java
Outdated
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoUtils.java
Outdated
Show resolved
Hide resolved
I don't understand where the +57 codecov misses are coming from, looking at the actual report at codecov there's definitely not 57 new uncovered lines |
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.
There's quite a lot of under the hood behavioral changes now, which look like they should have the same result and aren't subject to race conditions, but the low code coverage for this repo makes me nervous. I think this one is going to need some manual testing covering the altered code paths
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoDynamicRepoDB.java
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoDynamicRepoDB.java
Outdated
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoDynamicRepoDB.java
Outdated
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoDynamicRepoDB.java
Outdated
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/us/kbase/narrativemethodstore/db/mongo/MongoUtils.java
Outdated
Show resolved
Hide resolved
src/test/java/us/kbase/test/narrativemethodstore/db/mongo/MongoDBHelper.java
Outdated
Show resolved
Hide resolved
src/test/java/us/kbase/test/narrativemethodstore/db/mongo/MongoDynamicRepoDB2Test.java
Outdated
Show resolved
Hide resolved
src/test/java/us/kbase/test/narrativemethodstore/db/mongo/MongoDynamicRepoDBTest.java
Outdated
Show resolved
Hide resolved
Where did you see the +57 Codecov misses? What I saw is this link. |
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.
You're def going to want to run some manual tests to check the pull and replace vs. update changes work as expected
No description provided.