-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Bigtable: Added unit tests for GCRule#fromProto
#4990
Conversation
Bigtable: Added unit tests for `GCRule#fromProto`. Observation: If used multiple `setXXX()` on protobuf `GcRule#Builder` then the last most added values would be persisted.
@sduskis / @igorbernstein2 |
...ogle-cloud-bigtable/src/test/java/com/google/cloud/bigtable/admin/v2/models/GCRulesTest.java
Outdated
Show resolved
Hide resolved
...ogle-cloud-bigtable/src/test/java/com/google/cloud/bigtable/admin/v2/models/GCRulesTest.java
Outdated
Show resolved
Hide resolved
The proto uses a one-of for max age & max versions, so you can't set both. This confuses a lot of people, which is one of the reasons we created this wrapper |
Codecov Report
@@ Coverage Diff @@
## master #4990 +/- ##
============================================
- Coverage 50.32% 49.68% -0.64%
+ Complexity 23668 22304 -1364
============================================
Files 2238 2238
Lines 226059 221344 -4715
Branches 24959 24079 -880
============================================
- Hits 113754 109968 -3786
+ Misses 103704 102914 -790
+ Partials 8601 8462 -139
Continue to review full report at Codecov.
|
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
Fixes #4057
Observation:
If used multiple
setXXX()
on protobufGcRule#Builder
then the last most added value would be persisted after thebuild()
. This is also valid for modelGCRule#fromProto
.Added few unit test cases around the
fromProto
operation.