Actividad 3 - Grupo 5

[JoseLlontop]

https://github.com/DanielUTN/vetsoft

[ivanduva]

Buenas!
Vemos varios problemas que deben corregir para aprobar la actividad:

• El más grave lo encontramos en el caso del issue de la raza de la mascota porque no se muestran los errores en el form, permite guardar y la app entonces se rompe. Además quedó duplicada la clase Pet y aparece la advertencia al correr runserver. En cuanto a los tests, no hay e2e y el test de integración no hace ningún assert.
• En cuanto al precio del producto, no se muestran los errores al editar y el mensaje de error cuando el precio es menor o igual a 0 es el mismo que cuando el precio es vacío. Además el test de unidad está mal porque no prueba que aparezca un error sino que no se cree el producto cuando se intenta crear con error. Debería probar que aparezca el error.
• En el caso de la dirección del provider, no hay tests específicos sobre la dirección sino que son generales sobre la clase Provider. Los tests están bien pero son más generales sobre la clase Provider. Cuando se soluciona un issue, se debería agregar tests relativos a la funcionalidad que describía ese issue.
• En cuanto a la especialidad del veterinario, falta el test de unidad que testee ese atributo específicamente. También notamos que el form de editar no muestra mensajes de error, lo cual también estaría bueno arreglar pero no es prioritario.
• En cuanto a la dosis del medicamento, vemos el mismo problema que el del precio del producto: el test de unidad no prueba que aparezca el error.

Cada problema debería solucionarlo el que se encargó del issue.
También vemos problemas de procedimiento que deberían mejorar en las próximas entregas:

• En las reviews no hay comentarios con relevancia. Como dijimos más de una vez, comentarios del estilo "Funciona bien" no aportan a la review. Para eso se pone el approve y listo. Además había errores claros para marcar, por ejemplo que se rompa el form de mascota o que falte un test. No es algo relativo al código que puede ser complicado de encontrar.
• Vimos que en casi todos los PR, otra persona que no fue el que lo creó realizó el merge con main. No le esquiven a la resolución de conflictos, si pasa esto de nuevo, puede ser causa de corrección. Cada persona tiene que resolver los conflictos que le aparezcan.
• Si van a estandarizar la nomenclatura de los branches, sigan todos el mismo formato. La mayoría tiene el formato feature/una-descripcion, pero casos como feature-agregaropcionesrazamascota o feature/testing_product no estarían siguiendo ese formato.
• Si bien no es la mayoría, vemos algunos commits malos. Por ejemplo: DanielUTN/vetsoft@7feba34 . Este commit no es atómico, resuelve mil cosas distintas. Además el mensaje "para actividad 3 poner cambios en otra rama" no significa nada con respecto al proyecto o al código. Un mensaje de commit debería contener una síntesis de la funcionalidad que resuelve ese commit. No tienen que hacer referencias a la consigna ni en los commits ni en el código, deberían manejarse con los issues.

Saludos!

[DanielUTN]

Hola Ivan, te confirmo que los problemas de la actividad ya fueron revisados. Cualquier cosa nos avisas.
Saludos

[ivanduva]

Buenas.
La entrega está fuera de término. Recuerden que como explicamos en las condiciones de aprobación, las correcciones tienen fecha límite en la siguiente entrega (en este caso el día 5/6). Además si entregan fuera de término 2 veces quedan afuera de la promoción. Por ahora tienen 1.
Van a tener que corregir nuevamente:

• El test de unidad de la especialidad del veterinario no es correcto porque está testeando la función Vet.objects.create que es algo nativo de Django. Deberían testear algo que hicieron ustedes, por ejemplo la función save_vet o update_vet o validate_vet. Algo interesante para testear es que tire el error correspondiente cuando la especialidad es vacía.
• El test de integración sobre la clase mascota es un test general que prueba que la mascota sea creada correctamente. No es un test específico sobre el atributo breed. El de unidad lo mismo. Además el anterior test tiene el mismo problema que el de la especialidad del veterinario, no es correcto testear la función Pet.objects.create. El e2e, lo mismo, no es específico sobre el atributo breed.

La última fecha límite de recuperatorio, tanto para esta como las demás actividades, será el último día de cursada antes de las vacaciones: Lunes 8 de Julio 23:59 hs.

Saludos

[JoseLlontop]

Buenas, ya se realizaron las correcciones.
https://github.com/DanielUTN/vetsoft
Saludos!

[ivanduva]

Buenas,
Seguimos viendo un problema de procedimiento que ya les mencionamos anteriormente y que nos parece fundamental.
En este PR: DanielUTN/vetsoft#38 alguien que no era la persona asignada hizo el merge con main para solucionar conflictos. Ya comentamos que cada uno debería solucionar conflictos y no esquivarle al tema porque podía ser causa de corrección. La resolución de conflictos es un proceso clave para el trabajo con git. Además hay otro commit que borra un test y tampoco es una buena manera de proceder. Justamente para eso está la revisión. Si el PR genera dudas, a lo sumo habría que agregar un comentario y generar un feedback con la persona responsable hasta llegar al código con la calidad esperada y funcionando correctamente.
Le pedimos a Daniel que reponga el test test_breed_choices que se borró y no estaba mal. Esto deberá hacerse en el mismo branch bugFix/testsmascota y en el caso de que hayan conflictos deberá solucionarlos el mismo. Luego, realizar el PR correspondiente. En el caso de que el equipo tenga dudas del PR, deberán solucionarlas con una buena review.

Saludos

[JoseLlontop]

Buenas, se realizo la corrección mencionada.
https://github.com/DanielUTN/vetsoft

Saludos!

[ivanduva]

Perfecto, la actividad 3 queda aprobada