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();
    assertEquals(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?

upvote
  flag
It seems to me as if you are trying to enforce the Singleton pattern. If so, you might like dp4j.com (which does exactly that) – simpatico
upvote
  flag
shouldn't "intentionally empty" be replaced with throwing exception? In that case you could write test that expect that specific exception with specific message, no? not sure if this is overkill – Ewoks

15 Answers 11

up vote 55 down vote accepted

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.

upvote
  flag
This is how it works in Cobertura: //allinonescript.com/questions/951569 – yegor256
upvote
  flag
@Vincenzo: Well, that excludes the whole class - you only want to exclude the constructor. – Jon Skeet
14 upvote
  flag
I don't want to "slightly reduce coverage" in the entire project just because of this particular constructor.. – yegor256
25 upvote
  flag
@Vincenzo: Then IMO you're placing too high a value on a simple number. Coverage is an indicator of testing. Don't be a slave to a tool. The point of coverage is to give you a level of confidence, and to suggest areas for extra testing. Artificially calling an otherwise unused constructor doesn't help with either of those points. – Jon Skeet
12 upvote
  flag
@JonSkeet: I totally agree with "Don't be a slave to a tool", but it does not smell good to remember every "flaw count" in every project. How to make sure the 7/9 result is Cobertura limitation, and not programmer's? A new programmer must enter every failure (that can be a lot in large projects) to check class-by-class. – Eduardo Costa
upvote
  flag
Solution exists! See my answer – kan
1 upvote
  flag
Archimedes has it right IMO. He is testing the code, and at the same time providing full coverage. He is ensuring that coding standards are followed and thereby making it harder for a coder to use the class improperly. Also, given that he's captured the coding standards in a test, the effort to test becomes negligible and there is a real benefit realized. I swiped the code and I'm using it now. Thanks Archimedes :) – Matt Friedman
2 upvote
  flag
This does not answer the question. and by the way, some managers look at the coverage numbers. They don't care why. They know that 85% is better than 75%. – ACV
upvote
  flag
@ACV: Then what they "know" is incorrect. If you have two sets of tests covering the same code, there's no guarantee that the tests with higher coverage will actually be better tests. I would have no hesitation in arguing with a manager who started applying dubious metrics of quality without understanding them. – Jon Skeet
1 upvote
  flag
A practical use case to test otherwise inaccessible code is to achive 100% test coverage so that no one has to look at that class again. If the coverage is stuck at 95% many developers may attempt to figure out the reason for that just to bump into this issue over and over again. – thisismydesign

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).

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.

2 upvote
  flag
That's incorrect; you can instantiate it through reflection, as noted above. – theotherian

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.

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();
}
10 upvote
  flag
But this is eliminating noise in the coverage report by adding noise to the test suite. I would have just ended the sentence at "put the idealism aside". :) – Christopher Orr
9 upvote
  flag
To give this test any sort of meaning, you should probably also assert that the constructor's access level is what you expect it to be. – Jeremy
upvote
  flag
Adding the evil reflection plus Jeremy's ideas plus a meanful name like "testIfConstructorIsPrivateWithoutRaisingExceptions", I guess this is "THE" answer. – Eduardo Costa
1 upvote
  flag
This is syntactically incorrect is it not? What is constructor? Shouldn't Constructor be parameterized and not a raw type? – Adam Parkin
upvote
  flag
As TDD developer call the test "testConstructorIsPrivate()" and use this test to motivate the createion of the private constructor. – timomeinen
1 upvote
  flag
This is wrong: constructor.isAccessible() always returns false, even on a public constructor. One should use assertTrue(Modifier.isPrivate(constructor.getModifiers()));. – timomeinen
upvote
  flag
@timomeinen - I'm no longer actively developing in Java, so I'm too lazy to test out your suggestion, but I'll take your word for it. I'm sure I'll get more comments if you're wrong. :-) – Javid Jamae
upvote
  flag
@AdamParkin yes this was syntactically incorrect, but not because of the lack of parameterization, but because of the missing throws clauses. – drvdijk
upvote
  flag
If you use JaCoCo, >= 0.7.10 will ignore empty private constructors – lightswitch05

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.

upvote
  flag
I do this quite a bit. Cheap as in inexpensive, cheap as in dirty, but effective. – pholser
1 upvote
  flag
That is bluffing the tool. – Apoorv Khurasia
upvote
  flag
With Sonar, this actually causes the class to be missed entirely by code coverage. – Adam Parkin

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());
        }
    }
And then just add classes to the array as you go.

Finally, there is solution!

public enum Foo {;
  public static int bar() {
    return 1;
  }
}
upvote
  flag
How is that testing the class which is posted in the question though? You shouldn't assume that you can turn every class with a private constructor into an enum, or that you'd want to. – Jon Skeet
upvote
  flag
@JonSkeet I can for the class in question. And most of utility classes which have only bunch of static methods. Otherwise a class with the only private constructor doesn't have any sense. – kan
1 upvote
  flag
A class with a private constructor may be instantiated from public static methods, although of course then it's easy to get the coverage. But fundamentally I would prefer any class which extends Enum<E> to really be an enum... I believe that reveals intent better. – Jon Skeet
upvote
  flag
@JonSkeet The question assumes a class which should not be instantiated. Otherwise it should be tested. A class which could not be ever instantiated is an enum. And well... yes, it's question of preferences, I prefer 100% coverage more than some vague principles. – kan
3 upvote
  flag
Wow, I'd absolutely prefer code which makes sense over a pretty arbitrary number. (Coverage is no guarantee of quality, nor is 100% coverage feasible in all cases. Your tests should guide your code at best - not steer it over a cliff of bizarre intentions.) – Jon Skeet
1 upvote
  flag
@Kan: Adding a dummy call to the constructor to bluff the tool should not be the intent. Anyone who relies on a single metric to determine project's wellbeing is already on the path to destruction. – Apoorv Khurasia
upvote
  flag
This is relying on language semantics due to a design decision of the underlying platform as a guide of when to make Enums. Enums are enumerations, specifically designed to show a ranked collection of items that are statically known. A static "utils" type class may be a code smell, but that would not make it necessarily an enum. This choice would fundamentally be poor. – Visionary Software Solutions

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.

upvote
  flag
Overkill it may be, but if it was in Unitils or similar, I'd use it – Stewart
upvote
  flag
+1 Good start, though I went with Archimedes's more complete test. – David Harkness
upvote
  flag
The first example doesn't work - the IllegalAccesException means the constructor is never called, so coverage is not recorded. – Tom McIntyre
upvote
  flag
IMO, the solution in the first code snippet is the cleanest and the simplest one in this discussion. Just line with fail(...) is not necessary. – piotr.wittchen

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

8 upvote
  flag
+1 Not only does this solve the problem without tricking the tool, but it fully tests the coding standards of setting up a utility class. I had to change the accessibility test to use Modifier.isPrivate as isAccessible was returning true for private constructors in some cases (mocking library interference?). – David Harkness
4 upvote
  flag
I really want to add this to JUnit's Assert class, but don't want to take credit for your work. I think it's very good. It would be great to have Assert.utilityClassWellDefined() in JUnit 4.12+. Have you considered a pull request? – Visionary Software Solutions
3 upvote
  flag
upvote
  flag
Note that using setAccessible() to make the constructor accessible causes problems for Sonar's code coverage tool (when I do this the class disappears from the code coverage reports of Sonar). – Adam Parkin
upvote
  flag
Thanks, I do reset the accessible flag though. Perhaps it's a bug on Sonar itself? – Archimedes Trajano
upvote
  flag
I looked at my Sonar report for coverage on my batik maven plugin, it seems to cover correctly. site.trajano.net/batik-maven-plugin/cobertura/index.html – Archimedes Trajano
upvote
  flag
That should be a core assert. Thx for sharing. (I forgot the final modifier for the class ! Worth it!) – Snicolas
upvote
  flag
Great idea! It is not "solving the coverage problem" but rather enforcing proper coding standards. – krico
upvote
  flag
@ArchimedesTrajano Great idea. I think calling getDeclaredMethods will be a more thorough check since it will catch private instance methods. – Sam B.
upvote
  flag
I thought about it @SamB. but I didn't go for it because there can be Utility methods with some internal class that they want class level methods to perform but does not expose any of them to the public which I had allowed for. – Archimedes Trajano
upvote
  flag
Why are you reverting accessibility with setAccessible(false)? What you're modifying, is your local "reflective handle" to the class constructor. Resetting could have some sense except you're not using that "handle" anymore. – Piotr Findeisen
upvote
  flag
Because the constructor is private so I disabled the fact that it is private. However it should be private to begin with as such I disable access to it after the test. – Archimedes Trajano
upvote
  flag
@ArchimedesTrajano I think Piotr's point is that setAccessible only operates on that instance of the Constructor object, not on the constructor "itself". You can verify this by calling getDeclaredConstructor() again after setAccessible(true) -- isAccessible() will still be false on the second return value. (I wasn't sure till I verified it myself.) – David Moles

If I were to guess the intent of your question I'd say:

  1. You want reasonable checks for private constructors that do actual work, and
  2. 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

upvote
  flag
It would be nice to explain, why this construct helps with coverage. – Markus
upvote
  flag
True, and secondly: why catching an exception in your test? The exception being thrown should actually make the test fail. – Jordi

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.

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.

Not the answer you're looking for? Browse other questions tagged or ask your own question.