-
Notifications
You must be signed in to change notification settings - Fork 39
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
chore: fix jacoco coverage minimum, throw in memory provider #561
Conversation
if (ProviderState.ERROR.equals(state)) { | ||
errorCode = ErrorCode.GENERAL; | ||
if (ProviderState.NOT_READY.equals(state)) { | ||
throw new ProviderNotReadyError("provider not yet initialized"); |
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.
@liran2000 throwing like this ends in the same result, but it's a bit easier to trace, and also simpler to write. This is how we generally recommend it's done. There is a small performance cost in serialization of the exception, but I think it's worth it.
@Test | ||
void notFound() { | ||
assertThrows(FlagNotFoundError.class, () -> { | ||
provider.getBooleanEvaluation("not-found-flag", false, new ImmutableContext()); | ||
}); | ||
} | ||
|
||
@Test | ||
void typeMismatch() { | ||
assertThrows(TypeMismatchError.class, () -> { | ||
provider.getBooleanEvaluation("string-flag", false, new ImmutableContext()); | ||
}); | ||
} |
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.
These bring coverage of the new package up to 80%+ to fix the failure in main.
Signed-off-by: Todd Baert <[email protected]>
cf0ed4d
to
8f4d50a
Compare
Codecov Report
@@ Coverage Diff @@
## main #561 +/- ##
============================================
- Coverage 95.11% 94.49% -0.63%
- Complexity 329 357 +28
============================================
Files 31 32 +1
Lines 758 835 +77
Branches 37 50 +13
============================================
+ Hits 721 789 +68
- Misses 20 26 +6
- Partials 17 20 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Small changes to satisfy jacoco coverage failing in main (bring above 80%).
Also throwing in the new in-memory provider instead of returning error resolutions.
cc @liran2000 I missed these small things on your PR. I hope they make sense.