How to add test coverage to a private constructor?
This is the code:
package com.XXX;
public final class Foo {
private Foo() {
// intentionally empty
}
public static int bar() {
return 1;
}
}
This is the test:
package com.XXX;
public FooTest {
@Test
void testValidatesThatBarWorks() {
int result = Foo.bar();
as开发者_Go百科sertEquals(1, result);
}
@Test(expected = java.lang.IllegalAccessException.class)
void testValidatesThatClassFooIsNotInstantiable() {
Class cls = Class.forName("com.XXX.Foo");
cls.newInstance(); // exception here
}
}
Works fine, the class is tested. But Cobertura says that there is zero code coverage of the private constructor of the class. How can we add test coverage to such a private constructor?
I don't entirely agree with Jon Skeet. I think that if you can get an easy win to give you coverage and eliminate the noise in your coverage report, then you should do it. Either tell your coverage tool to ignore the constructor, or put the idealism aside and write the following test and be done with it:
@Test
public void testConstructorIsPrivate() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException {
Constructor<Foo> constructor = Foo.class.getDeclaredConstructor();
assertTrue(Modifier.isPrivate(constructor.getModifiers()));
constructor.setAccessible(true);
constructor.newInstance();
}
Well, there are ways you could potentially use reflection etc - but is it really worth it? This is a constructor which should never be called, right?
If there's an annotation or anything similar that you can add to the class to make Cobertura understand that it won't be called, do that: I don't think it's worth going through hoops to add coverage artificially.
EDIT: If there's no way of doing it, just live with the slightly reduced coverage. Remember that coverage is meant to be something which is useful to you - you should be in charge of the tool, not the other way round.
Although it's not necessarily for coverage, I created this method to verify that the utility class is well defined and do a bit of coverage as well.
/**
* Verifies that a utility class is well defined.
*
* @param clazz
* utility class to verify.
*/
public static void assertUtilityClassWellDefined(final Class<?> clazz)
throws NoSuchMethodException, InvocationTargetException,
InstantiationException, IllegalAccessException {
Assert.assertTrue("class must be final",
Modifier.isFinal(clazz.getModifiers()));
Assert.assertEquals("There must be only one constructor", 1,
clazz.getDeclaredConstructors().length);
final Constructor<?> constructor = clazz.getDeclaredConstructor();
if (constructor.isAccessible() ||
!Modifier.isPrivate(constructor.getModifiers())) {
Assert.fail("constructor is not private");
}
constructor.setAccessible(true);
constructor.newInstance();
constructor.setAccessible(false);
for (final Method method : clazz.getMethods()) {
if (!Modifier.isStatic(method.getModifiers())
&& method.getDeclaringClass().equals(clazz)) {
Assert.fail("there exists a non-static method:" + method);
}
}
}
I have placed the full code and examples in https://github.com/trajano/maven-jee6/tree/master/maven-jee6-test
I had made private the constructor of my class of static utility functions, to satisfy CheckStyle. But like the original poster, I had Cobertura complaining about the test. At first I tried this approach, but this doesn't affect the coverage report because the constructor is never actually executed. So really all this tests is if the constructor is remaining private - and this is made redundant by the accessibility check in the subsequent test.
@Test(expected=IllegalAccessException.class)
public void testConstructorPrivate() throws Exception {
MyUtilityClass.class.newInstance();
fail("Utility class constructor should be private");
}
I went with Javid Jamae's suggestion and used reflection, but added assertions to catch anybody messing with the class being tested (and named the test to indicate High Levels Of Evil).
@Test
public void evilConstructorInaccessibilityTest() throws Exception {
Constructor[] ctors = MyUtilityClass.class.getDeclaredConstructors();
assertEquals("Utility class should only have one constructor",
1, ctors.length);
Constructor ctor = ctors[0];
assertFalse("Utility class constructor should be inaccessible",
ctor.isAccessible());
ctor.setAccessible(true); // obviously we'd never do this in production
assertEquals("You'd expect the construct to return the expected type",
MyUtilityClass.class, ctor.newInstance().getClass());
}
This is so overkill, but I gotta admit I like the warm fuzzy feeling of 100% method coverage.
With Java 8, it is possible to find other solution.
I assume that you simply want to create utility class with few public static methods. If you can use Java 8, then you can use interface instead.
package com.XXX;
public interface Foo {
public static int bar() {
return 1;
}
}
There is no constructor and no complain from Cobertura. Now you need to test only the lines you really care about.
The following worked to me on a class created with Lombok annotation @UtilityClass, that automatically add a private constructor.
@Test
public void testConstructorIsPrivate() throws IllegalAccessException, InvocationTargetException, InstantiationException, NoSuchMethodException {
Constructor<YOUR_CLASS_NAME> constructor = YOUR_CLASS_NAME.class.getDeclaredConstructor();
assertTrue(Modifier.isPrivate(constructor.getModifiers())); //this tests that the constructor is private
constructor.setAccessible(true);
assertThrows(InvocationTargetException.class, () -> {
constructor.newInstance();
}); //this add the full coverage on private constructor
}
Although constructor.setAccessible(true) should works when private constructor has been written manually, with Lombok annotation doesn't work, since it force it. Constructor.newInstance() actually tests that the constructor is invoked and this complete the coverage of the costructor itself. With the assertThrows you prevent that the test fails and you managed the exception since is exactly the error that you expect. Although this is a workaround and I don't appreciate the concept of "line coverage" vs "functionality/behavior coverage" we can find a sense on this test. In fact you are sure that the Utility Class has actually a private Constructor that correctly throws an exception when invoked also via reflaction. Hope this helps.
The reasoning behind testing code that doesn't do anything is to achieve 100% code coverage and to notice when the code coverage drops. Otherwise one could always think, hey I don't have 100% code coverage anymore but it's PROBABLY because of my private constructors. This makes it easy to spot untested methods without having to check that it just was a private constructor. As your codebase grows you'll actually feel a nice warm feeling looking at 100% instead of 99%.
IMO it's best to use reflection here since otherwise you would have to either get a better code coverage tool that ignores these constructors or somehow tell the code coverage tool to ignore the method (perhaps an Annotation or a configuration file) because then you would be stuck with a specific code coverage tool.
In a perfect world all code coverage tools would ignore private constructors that belong to a final class because the constructor is there as a "security" measure nothing else:)
I would use this code:
@Test
public void callPrivateConstructorsForCodeCoverage() throws SecurityException, NoSuchMethodException, IllegalArgumentException, InstantiationException, IllegalAccessException, InvocationTargetException
{
Class<?>[] classesToConstruct = {Foo.class};
for(Class<?> clazz : classesToConstruct)
{
Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);
assertNotNull(constructor.newInstance());
}
}Newer versions of Cobertura have built-in support for ignoring trivial getters/setters/constructors:
https://github.com/cobertura/cobertura/wiki/Ant-Task-Reference#ignore-trivial
Ignore Trivial
Ignore trivial allows the ability to exclude constructors/methods that contain one line of code. Some examples include a call to a super constrctor only, getter/setter methods, etc. To include the ignore trivial argument add the following:
<cobertura-instrument ignoreTrivial="true" />
or in a Gradle build:
cobertura {
coverageIgnoreTrivial = true
}
Don't. What's the point in testing an empty constructor? Since cobertura 2.0 there is an option to ignore such trivial cases (together with setters/getters), you can enable it in maven by adding configuration section to cobertura maven plugin:
<configuration>
<instrumentation>
<ignoreTrivial>true</ignoreTrivial>
</instrumentation>
</configuration>
Alternatively you can use Coverage Annotations: @CoverageIgnore.
My preferred option in 2019+: Use lombok.
Specifically, the @UtilityClass annotation. (Sadly only "experimental" at the time of writing, but it functions just fine and has a positive outlook, so likely to soon be upgraded to stable.)
This annotation will add the private constructor to prevent instantiation and will make the class final. When combined with lombok.addLombokGeneratedAnnotation = true in lombok.config, pretty much all testing frameworks will ignore the auto-generated code when calculating test coverage, allowing you to bypass coverage of that auto-generated code with no hacks or reflection.
Finally, there is solution!
public enum Foo {;
public static int bar() {
return 1;
}
}
I don't know about Cobertura but I use Clover and it has a means of adding pattern-matching exclusions. For example, I have patterns that exclude apache-commons-logging lines so they are not counted in the coverage.
Another option is to create a static initializer similar to the following code
class YourClass {
private YourClass() {
}
static {
new YourClass();
}
// real ops
}
This way the private constructor is considered tested, and the runtime overhead is basically not measurable. I do this to get 100% coverage using EclEmma, but likely it works out for every coverage tool. The drawback with this solution, of course, is that you write production code (the static initializer) just for testing purposes.
ClassUnderTest testClass=Whitebox.invokeConstructor(ClassUnderTest.class);
Sometimes Cobertura marks code not intended to be executed as 'not covered', there's nothing wrong with that. Why are you concerned with having 99% coverage instead of 100%?
Technically, though, you can still invoke that constructor with reflection, but it sounds very wrong to me (in this case).
If I were to guess the intent of your question I'd say:
- You want reasonable checks for private constructors that do actual work, and
- You want clover to exclude empty constructors for util classes.
For 1, it is obvious that you want all initialisation to be done via factory methods. In such cases, your tests should be able to test the side effects of the constructor. This should fall under the category of normal private method testing. Make the methods smaller so that they only do a limited number of determinate things (ideally, just one thing and one thing well) and then test the methods that rely on them.
For example, if my [private] constructor sets up my class's instance fields a to 5. Then I can (or rather must) test it:
@Test
public void testInit() {
MyClass myObj = MyClass.newInstance(); //Or whatever factory method you put
Assert.assertEquals(5, myObj.getA()); //Or if getA() is private then test some other property/method that relies on a being 5
}
For 2, you can configure clover to exclude Util constructors if you have a set naming pattern for Util classes. E.g., in my own project I use something like this (because we follow the convention that names for all Util classes should end with Util):
<clover-setup initString="${build.dir}/clovercoverage.db" enabled="${with.clover}">
<methodContext name="prvtCtor" regexp="^private *[a-zA-Z0-9_$]+Util *( *) *"/>
</clover-setup>
I have deliberately left out a .* following ) because such constructors are not meant to throw exceptions (they are not meant to do anything).
There of course can be a third case where you may want to have an empty constructor for a non-utility class. In such cases, I would recommend that you put a methodContext with the exact signature of the constructor.
<clover-setup initString="${build.dir}/clovercoverage.db" enabled="${with.clover}">
<methodContext name="prvtCtor" regexp="^private *[a-zA-Z0-9_$]+Util *( *) *"/>
<methodContext name="myExceptionalClassCtor" regexp="^private MyExceptionalClass()$"/>
</clover-setup>
If you have many such exceptional classes then you can choose to modify the generalized private constructor reg-ex I suggested and remove Util from it. In this case, you will have to manually make sure that your constructor's side effects are still tested and covered by other methods in your class/project.
<clover-setup initString="${build.dir}/clovercoverage.db" enabled="${with.clover}">
<methodContext name="prvtCtor" regexp="^private *[a-zA-Z0-9_$]+ *( *) .*"/>
</clover-setup>
@Test
public void testTestPrivateConstructor() {
Constructor<Test> cnt;
try {
cnt = Test.class.getDeclaredConstructor();
cnt.setAccessible(true);
cnt.newInstance();
} catch (Exception e) {
e.getMessage();
}
}
Test.java is your source file,which is having your private constructor
You can't.
You're apparently creating the private constructor to prevent instantiation of a class that is intended to contain only static methods. Rather than trying to get coverage of this constructor (which would require that the class be instantiated), you should get rid of it and trust your developers not to add instance methods to the class.
加载中,请稍侯......
精彩评论