¿Cómo agregar cobertura de prueba a un constructor privado?

110

Este es el código:

package com.XXX;
public final class Foo {
  private Foo() {
    // intentionally empty
  }
  public static int bar() {
    return 1;
  }
}

Esta es la prueba:

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
  }
}

Funciona bien, la clase está probada. Pero Cobertura dice que no hay cobertura de código del constructor privado de la clase. ¿Cómo podemos agregar cobertura de prueba a un constructor tan privado?

yegor256
fuente
Me parece que está tratando de imponer el patrón Singleton. Si es así, puede que le guste dp4j.com (que hace exactamente eso)
simpatico
¿No debería reemplazarse "intencionalmente vacío" por lanzar una excepción? En ese caso, podría escribir una prueba que espere esa excepción específica con un mensaje específico, ¿no? No estoy seguro si esto es excesivo
Ewoks

Respuestas:

85

Bueno, hay formas en las que podrías usar la reflexión, etc., pero ¿realmente vale la pena? Este es un constructor que nunca debería llamarse , ¿verdad?

Si hay una anotación o algo similar que pueda agregar a la clase para que Cobertura entienda que no se llamará, hágalo: no creo que valga la pena pasar por aros para agregar cobertura artificialmente.

EDITAR: Si no hay forma de hacerlo, simplemente viva con la cobertura ligeramente reducida. Recuerde que la cobertura está destinado a ser algo que es útil para usted - usted debe estar a cargo de la herramienta, y no al revés.

Jon Skeet
fuente
18
No quiero "reducir ligeramente la cobertura" en todo el proyecto solo por este constructor en particular ..
yegor256
36
@Vincenzo: Entonces, en mi opinión, le estás dando un valor demasiado alto a un número simple. La cobertura es un indicador de pruebas. No seas esclavo de una herramienta. El objetivo de la cobertura es brindarle un nivel de confianza y sugerir áreas para pruebas adicionales. Llamar artificialmente a un constructor no utilizado no ayuda con ninguno de esos puntos.
Jon Skeet
19
@JonSkeet: Estoy totalmente de acuerdo con "No seas esclavo de una herramienta", pero no huele bien recordar cada "defecto" en cada proyecto. ¿Cómo asegurarse de que el resultado 7/9 sea una limitación de Cobertura y no un programador? Un nuevo programador debe ingresar cada falla (que puede ser mucho en proyectos grandes) para verificar clase por clase.
Eduardo Costa
5
Esto no responde a la pregunta. y por cierto, algunos gerentes miran los números de cobertura. No les importa por qué. Saben que el 85% es mejor que el 75%.
ACV
2
Un caso de uso práctico para probar código inaccesible de otro modo es lograr una cobertura de prueba del 100% para que nadie tenga que volver a mirar esa clase. Si la cobertura se atasca en el 95%, muchos desarrolladores pueden intentar averiguar la razón de eso solo para tropezar con este problema una y otra vez.
thisismydesign
140

No estoy del todo de acuerdo con Jon Skeet. Creo que si puede obtener una ganancia fácil para brindarle cobertura y eliminar el ruido en su informe de cobertura, entonces debería hacerlo. Dile a tu herramienta de cobertura que ignore al constructor, o deja a un lado el idealismo y escribe la siguiente prueba y termina con ella:

@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();
}
Javid Jamae
fuente
25
Pero esto elimina el ruido en el informe de cobertura al agregar ruido al conjunto de pruebas. Habría terminado la frase con "dejar el idealismo a un lado". :)
Christopher Orr
11
Para darle a esta prueba algún tipo de significado, probablemente también debería afirmar que el nivel de acceso del constructor es el que espera que sea.
Jeremy
Añadiendo el reflejo maligno más las ideas de Jeremy más un nombre significativo como "testIfConstructorIsPrivateWithoutRaisingExceptions", supongo que esta es "LA" respuesta.
Eduardo Costa
1
Esto es sintácticamente incorrecto, ¿no? ¿Qué es constructor? ¿No Constructordebería parametrizarse y no ser un tipo sin formato?
Adam Parkin
2
Esto está mal: constructor.isAccessible()siempre devuelve falso, incluso en un constructor público. Uno debería usar assertTrue(Modifier.isPrivate(constructor.getModifiers()));.
timomeinen
78

Aunque no es necesariamente para cobertura, creé este método para verificar que la clase de utilidad esté bien definida y que también tenga un poco de cobertura.

/**
 * 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);
        }
    }
}

He colocado el código completo y los ejemplos en https://github.com/trajano/maven-jee6/tree/master/maven-jee6-test

Arquímedes Trajano
fuente
11
+1 Esto no solo resuelve el problema sin engañar a la herramienta, sino que también prueba completamente los estándares de codificación para configurar una clase de utilidad. Tuve que cambiar la prueba de accesibilidad para usar Modifier.isPrivatecomo isAccessibleregresaba truepara constructores privados en algunos casos (¿burlarse de la interferencia de la biblioteca?).
David Harkness
4
Realmente quiero agregar esto a la clase Assert de JUnit, pero no quiero atribuirme el mérito de su trabajo. Creo que es muy bueno. Sería genial tenerlo Assert.utilityClassWellDefined()en JUnit 4.12+. ¿Ha considerado una solicitud de extracción?
Visionary Software Solutions
Tenga en cuenta que usar setAccessible()para hacer accesible al constructor causa problemas para la herramienta de cobertura de código de Sonar (cuando hago esto, la clase desaparece de los informes de cobertura de código de Sonar).
Adam Parkin
Gracias, restablezco la bandera accesible. ¿Quizás es un error en el propio Sonar?
Archimedes Trajano
Miré mi informe de Sonar para ver la cobertura de mi complemento batik maven, parece cubrir correctamente. site.trajano.net/batik-maven-plugin/cobertura/index.html
Archimedes Trajano
19

Hice privado el constructor de mi clase de funciones de utilidad estática, para satisfacer CheckStyle. Pero al igual que el póster original, Cobertura se quejó de la prueba. Al principio probé este enfoque, pero esto no afecta el informe de cobertura porque el constructor nunca se ejecuta realmente. Entonces, realmente todas estas pruebas son si el constructor permanece privado, y esto se vuelve redundante por la verificación de accesibilidad en la prueba posterior.

@Test(expected=IllegalAccessException.class)
public void testConstructorPrivate() throws Exception {
    MyUtilityClass.class.newInstance();
    fail("Utility class constructor should be private");
}

Seguí la sugerencia de Javid Jamae y usé la reflexión, pero agregué afirmaciones para atrapar a cualquiera que se metiera con la clase que se estaba probando (y nombré la prueba para indicar 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());
}

Esto es tan exagerado, pero debo admitir que me gusta la sensación cálida y difusa de la cobertura del método al 100%.

Ben Hardy
fuente
Puede ser exagerado, pero si estuviera en Unitils o similar, lo usaría
Stewart
+1 Buen comienzo, aunque fui con la prueba más completa de Arquímedes .
David Harkness
El primer ejemplo no funciona: la IllegalAccesException significa que nunca se llama al constructor, por lo que la cobertura no se registra.
Tom McIntyre
En mi opinión, la solución en el primer fragmento de código es la más limpia y simple de esta discusión. No fail(...)es necesario simplemente alinear con .
Piotr Wittchen
9

Con Java 8 , es posible encontrar otra solución.

Supongo que simplemente desea crear una clase de utilidad con pocos métodos estáticos públicos. Si puede usar Java 8, puede usarlo interfaceen su lugar.

package com.XXX;

public interface Foo {

  public static int bar() {
    return 1;
  }
}

No hay constructor y no se quejan de Cobertura. Ahora necesita probar solo las líneas que realmente le interesan.

Arnost Valicek
fuente
1
Desafortunadamente, sin embargo, no puede declarar la interfaz como "final", evitando que alguien la subclasifique; de ​​lo contrario, este sería el mejor enfoque.
Michael Berry
5

El razonamiento detrás de probar el código que no hace nada es lograr una cobertura del código del 100% y darse cuenta de cuándo cae la cobertura del código. De lo contrario, uno siempre podría pensar, oye, ya no tengo una cobertura de código del 100%, pero PROBABLEMENTE se debe a mis constructores privados. Esto facilita la detección de métodos no probados sin tener que verificar que solo sea un constructor privado. A medida que su código base crece, realmente sentirá una agradable sensación de calor al mirar al 100% en lugar del 99%.

En mi opinión, es mejor usar la reflexión aquí, ya que de lo contrario tendría que obtener una mejor herramienta de cobertura de código que ignore estos constructores o de alguna manera decirle a la herramienta de cobertura de código que ignore el método (tal vez una anotación o un archivo de configuración) porque entonces estaría atascado con una herramienta de cobertura de código específica.

En un mundo perfecto, todas las herramientas de cobertura de código ignorarían los constructores privados que pertenecen a una clase final porque el constructor está allí como una medida de "seguridad", nada más :)
Yo usaría este código:

    @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());
        }
    }
Y luego simplemente agregue clases a la matriz a medida que avanza.

jontejj
fuente
5

Las versiones más nuevas de Cobertura tienen soporte incorporado para ignorar captadores / definidores / constructores triviales:

https://github.com/cobertura/cobertura/wiki/Ant-Task-Reference#ignore-trivial

Ignorar trivial

Ignorar trivial permite la posibilidad de excluir constructores / métodos que contienen una línea de código. Algunos ejemplos incluyen una llamada a un superconstrctor solamente, métodos getter / setter, etc. Para incluir el argumento trivial ignorar agregue lo siguiente:

<cobertura-instrument ignoreTrivial="true" />

o en una compilación de Gradle:

cobertura {
    coverageIgnoreTrivial = true
}
Mike Buhot
fuente
4

No lo hagas. ¿Qué sentido tiene probar un constructor vacío? Dado que cobertura 2.0 hay una opción para ignorar estos casos triviales (junto con setters / getters), puede habilitarlo en maven agregando la sección de configuración al complemento cobertura maven:

<configuration>
  <instrumentation>
    <ignoreTrivial>true</ignoreTrivial>                 
  </instrumentation>
</configuration>

Alternativamente, puede utilizar las anotaciones de cobertura : @CoverageIgnore.

Krzysztof Krasoń
fuente
3

¡Finalmente, hay una solución!

public enum Foo {;
  public static int bar() {
    return 1;
  }
}
kan
fuente
Sin embargo, ¿cómo es eso de probar la clase que se publica en la pregunta? No debe asumir que puede convertir cada clase con un constructor privado en una enumeración, o que le gustaría hacerlo.
Jon Skeet
@JonSkeet Puedo para la clase en cuestión. Y la mayoría de las clases de servicios públicos que solo tienen un montón de métodos estáticos. De lo contrario, una clase con el único constructor privado no tiene ningún sentido.
kan
1
Se puede crear una instancia de una clase con un constructor privado a partir de métodos estáticos públicos, aunque, por supuesto, es fácil obtener la cobertura. Pero fundamentalmente preferiría cualquier clase que se extienda Enum<E>para ser realmente una enumeración ... Creo que eso revela mejor la intención.
Jon Skeet
4
Vaya, preferiría absolutamente el código que tiene sentido sobre un número bastante arbitrario. (La cobertura no es garantía de calidad, ni la cobertura del 100% es factible en todos los casos. Tus pruebas deberían guiar tu código en el mejor de los casos, no conducirlo por un precipicio de intenciones extrañas).
Jon Skeet
1
@Kan: Agregar una llamada ficticia al constructor para engañar a la herramienta no debería ser la intención. Cualquiera que confíe en una única métrica para determinar el bienestar del proyecto ya está en el camino de la destrucción.
Apoorv Khurasia
1

No sé sobre Cobertura, pero uso Clover y tiene un medio para agregar exclusiones de coincidencia de patrones. Por ejemplo, tengo patrones que excluyen líneas apache-commons-logging para que no se cuenten en la cobertura.

John Engelman
fuente
1

Otra opción es crear un inicializador estático similar al siguiente código

class YourClass {
  private YourClass() {
  }
  static {
     new YourClass();
  }

  // real ops
}

De esta manera, el constructor privado se considera probado y la sobrecarga del tiempo de ejecución básicamente no es medible. Hago esto para obtener una cobertura del 100% con EclEmma, ​​pero es probable que funcione para todas las herramientas de cobertura. El inconveniente de esta solución, por supuesto, es que escribe el código de producción (el inicializador estático) solo con fines de prueba.

Christian Lewold
fuente
Hago esto bastante. Barato como barato, barato como sucio, pero efectivo.
pholser
Con Sonar, esto en realidad hace que la clase se pierda por completo debido a la cobertura del código.
Adam Parkin
1

ClassUnderTest testClass = Whitebox.invokeConstructor (ClassUnderTest.class);

acpuma
fuente
Esta debería haber sido la respuesta correcta, ya que responde exactamente a lo que se pregunta.
Chakian
0

A veces, Cobertura marca el código que no está destinado a ejecutarse como 'no cubierto', no hay nada de malo en eso. ¿Por qué le preocupa tener 99%cobertura en lugar de 100%?

Técnicamente, sin embargo, todavía puedes invocar ese constructor con reflexión, pero me suena muy mal (en este caso).

Nikita Rybak
fuente
0

Si tuviera que adivinar la intención de su pregunta, diría:

  1. Quiere verificaciones razonables para constructores privados que realicen un trabajo real, y
  2. Desea que clover excluya los constructores vacíos para las clases de servicios.

Para 1, es obvio que desea que toda la inicialización se realice a través de métodos de fábrica. En tales casos, sus pruebas deberían poder probar los efectos secundarios del constructor. Esto debería caer en la categoría de prueba de método privado normal. Haga los métodos más pequeños para que solo hagan un número limitado de cosas determinadas (idealmente, solo una cosa y una cosa bien) y luego pruebe los métodos que se basan en ellos.

Por ejemplo, si mi constructor [privado] configura los campos de instancia de mi clase aen 5. Entonces puedo (o más bien debo) probarlo:

@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
}

Para 2, puede configurar clover para excluir los constructores Util si tiene un patrón de nomenclatura establecido para las clases Util. Por ejemplo, en mi propio proyecto utilizo algo como esto (porque seguimos la convención de que los nombres de todas las clases de Util deben terminar con Util):

<clover-setup initString="${build.dir}/clovercoverage.db" enabled="${with.clover}">
    <methodContext name="prvtCtor" regexp="^private *[a-zA-Z0-9_$]+Util *( *) *"/>
</clover-setup>

Deliberadamente he omitido un .*siguiente) porque tales constructores no están destinados a lanzar excepciones (no están destinados a hacer nada).

Por supuesto, puede haber un tercer caso en el que desee tener un constructor vacío para una clase que no sea de utilidad. En tales casos, le recomendaría que coloque un methodContextcon la firma exacta del 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>

Si tiene muchas de estas clases excepcionales, puede elegir modificar el constructor privado generalizado reg-ex que sugerí y eliminarlo Util. En este caso, deberá asegurarse manualmente de que los efectos secundarios de su constructor aún se prueben y cubran con otros métodos en su clase / proyecto.

<clover-setup initString="${build.dir}/clovercoverage.db" enabled="${with.clover}">
    <methodContext name="prvtCtor" regexp="^private *[a-zA-Z0-9_$]+ *( *) .*"/>
</clover-setup>
Apoorv Khurasia
fuente
0
@Test
public void testTestPrivateConstructor() {
    Constructor<Test> cnt;
    try {
        cnt = Test.class.getDeclaredConstructor();
        cnt.setAccessible(true);

        cnt.newInstance();
    } catch (Exception e) {
        e.getMessage();
    }
}

Test.java es su archivo fuente, que tiene su constructor privado

DPREDDY
fuente
Sería bueno explicar por qué esta construcción ayuda con la cobertura.
Markus
Es cierto, y en segundo lugar: ¿por qué detectar una excepción en su prueba? La excepción que se lanza debería hacer que la prueba falle.
Jordi
0

Lo siguiente me funcionó en una clase creada con la anotación de Lombok @UtilityClass, que agrega automáticamente un constructor privado.

@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
}

Aunque constructor.setAccessible (true) debería funcionar cuando el constructor privado se ha escrito manualmente, con la anotación de Lombok no funciona, ya que la fuerza. Constructor.newInstance () en realidad prueba que se invoca al constructor y esto completa la cobertura del propio constructor. Con assertThrows evitas que la prueba falle y manejaste la excepción ya que es exactamente el error que esperas. Aunque esta es una solución alternativa y no aprecio el concepto de "cobertura de línea" frente a "cobertura de funcionalidad / comportamiento", podemos encontrar un sentido en esta prueba. De hecho, está seguro de que la clase de utilidad tiene en realidad un constructor privado que arroja correctamente una excepción cuando se invoca también mediante reflacción. Espero que esto ayude.

Riccardo Solimena
fuente
Hola @ShanteshwarInde. Muchas gracias. Mi entrada ha sido editada y completada siguiendo sus sugerencias. Saludos.
Riccardo Solimena
0

Mi opción preferida en 2019: usar lombok.

Específicamente, la @UtilityClassanotación . (Lamentablemente, solo es "experimental" en el momento de escribir este artículo, pero funciona bien y tiene una perspectiva positiva, por lo que es probable que pronto se actualice a estable).

Esta anotación agregará el constructor privado para evitar la creación de instancias y hará que la clase sea final. Cuando se combinan con lombok.addLombokGeneratedAnnotation = truein lombok.config, casi todos los marcos de prueba ignorarán el código generado automáticamente al calcular la cobertura de la prueba, lo que le permitirá omitir la cobertura de ese código generado automáticamente sin hacks ni reflejos.

Michael Berry
fuente
-2

No puedes.

Aparentemente, está creando el constructor privado para evitar la creación de instancias de una clase que está destinada a contener solo métodos estáticos. En lugar de intentar obtener cobertura de este constructor (lo que requeriría que la clase sea instanciada), debe deshacerse de él y confiar en que sus desarrolladores no agregarán métodos de instancia a la clase.

Luego
fuente
3
Eso es incorrecto; puede instanciarlo a través de la reflexión, como se señaló anteriormente.
theotherian
Eso es malo, nunca deje que aparezca el constructor público predeterminado, debe agregar el privado para evitar llamarlo.
Lho Ben