1.x: Test all private Constructors #4125
Conversation
Current coverage is 81.33%@@ 1.x #4125 diff @@
==========================================
Files 257 257
Lines 16811 16811
Methods 0 0
Messages 0 0
Branches 2547 2547
==========================================
+ Hits 13626 13674 +48
+ Misses 2277 2239 -38
+ Partials 908 898 -10
|
| @@ -31,7 +31,7 @@ | |||
|
|
|||
| /** Utility class, no instances. */ | |||
| private Exceptions() { | |||
| throw new IllegalStateException("No instances!"); | |||
| throw new AssertionError("No instances."); | |||
There was a problem hiding this comment.
Why the change to AssertionError?
There was a problem hiding this comment.
I find it that this exception matches the use case a bit better since we're asserting that this constructor shall never be called.
There was a problem hiding this comment.
I find these as unnecessary changes. The methods throw to those who instantiate the class via reflection and are not test methods themselves to fail with AssertionError like TestSubscriber.assertX.
|
👍 Great! |
|
👍 Now I'm not that fanatic regarding testing private constructors, but since test coverage causes addiction and constant desire to increase it — let's merge it! |
|
Can I suggest in the spirit of DRY that you make a static method available somewhere (a new class public static void checkConstructorPrivate(Class<?> cls) {
PrivateConstructorChecker
.forClass(cls)
.expectedTypeOfException(IllegalStateException.class)
.expectedExceptionMessage("No instances!").check();
}and call that instead? |
|
Thanks! @davidmoten I'll refactor those checks into a simple call as you and I suggested. |
No description provided.