Revisión Pruebas Reusable-DAS Grupo1 #19
Conversation
Se crea lo primero la instancia del pool y luego hacemos algunas verificaciones como si la instancia es nula o si la instancia 1 sería igual a las instancia 2
Se crea una instancia del pool y luego se hacen varias comprobaciones, como verificar si la instancia es nula o si la instancia 1 es igual a la instancia 2
Crea una instancia del pool y verificamos varias cosas, como por ejemplo que la instancia no sea nula o que la instancia uno es igual a la segunda.
Añade el codecov
Creamos una instancia en el pool y luego hacemos varias verificaciones, como por ejemplo que la instancia no sea nula o que la instancia uno sea igual que la segunda instancia.
There was a problem hiding this comment.
PR Overview
This PR updates and expands the test suite for the ReusablePool functionality, improving exception handling, verifying pool behavior (including LIFO order), and adding tests for the Reusable.util() method. It also updates the README to include editor information.
- Updated and expanded JUnit tests in ReusablePoolTest.java to cover all main functionalities and exception cases.
- Revised exception messages and added comprehensive cycle operations tests.
- Updated README.md with editor credits to properly acknowledge contributions.
Reviewed Changes
| File | Description |
|---|---|
| src/test/java/ubu/gii/dass/c01/ReusablePoolTest.java | Expanded tests for instance management, exception handling, and pool cycle operations. |
| README.md | Updated credits and added a Codecov badge to track test coverage. |
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/test/java/ubu/gii/dass/c01/ReusablePoolTest.java:176
- [nitpick] Consider using assertNotSame(r1, r2, "Should be different objects") instead of comparing with '!=' for clearer intent.
assertTrue(r1 != r2, "Should be different objects");
Co-authored-by: Copilot <[email protected]>
clopezno
left a comment
clopezno
left a comment
There was a problem hiding this comment.
Para mejorar la facilidad de mantenimiento y la comprensión sugiero ser coherentes con los comentarios de los métodos, formateo del código fuente y distribución de los casos de prueba en métodos.
| */ | ||
| @BeforeAll | ||
| public static void setUp(){ | ||
| public static void setUp() throws Exception{ |
There was a problem hiding this comment.
¿Qué sentido tiene añadir throws Exception?
| assertNotNull (instance1,"La instancia adquirida no debe ser nula"); | ||
| //Comprueba si la instancia 1 es igual que la instancia 2 | ||
| ReusablePool instance2 = ReusablePool.getInstance(); | ||
| assertSame(instance1, instance2); |
There was a problem hiding this comment.
Se están implementando dos casos de prueba en el mismo método testGetInstance. Otra alternativa es separar cada caso de prueba en métodos distintos testGetInstance_cu01instancia testGetInstance_cu02instanciaunica. Además, para mejorar la comprensión assetSame puede incorporar un mensaje en "Singleton devuelve siempre la misma instancia"
| assertNotNull(reusable,"El objeto reusable adquirido no debería ser nulo"); | ||
| // Verifica que el objeto adquirido es una instancia de la clase Reusable | ||
| assertTrue(reusable instanceof Reusable, "El objeto adquirido debe ser una instancia de Reusable"); | ||
| } catch (NotFreeInstanceException e) { |
There was a problem hiding this comment.
Comprobar el lanzamiento de excepciones es otro caso de prueba y se ha implementado en otro método de prueba. Es correcto. Para mejorar la comprensión sugiero ser coherentes, es decir, o todos los casos de prueba en el mismo método o todos en métodos separados. Además los pondría todos seguidos en el fichero.
| "Util string should contain the object's hashcode"); | ||
| assertTrue(utilString.contains(":Uso del objeto Reutilizable"), | ||
| "Util string should contain the expected message"); | ||
| } |
There was a problem hiding this comment.
Esta prueba esta fuera del contexto de prueba Reusablepool se debería eliminar de este fichero. Además, Reusable es una implementación de compromiso que no debería ser probada. Entiendo que se ha añadido para conseguir el 100% de cobertura del sistema. Si se quiere eliminar el seguimiento de su cobertura se puede indicar en el fichero de configuración de .codecov.yml añadiendo
ignore:
- "src/main/java/ubu/gii/dass/c01/Client.java"
- "src/main/java/ubu/gii/dass/c01/Reusable.java"
| assertEquals("Ya existe esa instancia en el pool.", | ||
| dupEx.getMessage(), | ||
| "DuplicatedInstanceException should have correct message"); | ||
| } |
There was a problem hiding this comment.
Nunca había visto estas pruebas de comprobar el mensaje de las excepciones. No aconsejo hacerlo ya que los mensajes pueden ser muy inestables y no interfieren en requisitos funcionales. Las pruebas se deberían definir para comprobar el funcionamiento correcto del sistema. La prueba fundamental es que se lanza la excepción.
5 participants
No description provided.