Error con los turnos

[CimimUxMaio]

@Tp-Tacs/back @Pato94

Buenas :hand:

Lamentablemente seguimos teniendo el mismo bug. Si bien lo que nos mencionaste en #44 (@Pato94) era correcto, era un error que podia molestarnos en algun momento, no terminó de resolver el problema.

En la branch ErrorTurnos agregamos un par de tests en el GameControllerTests que logran capturar estos errores, que pasan "a veces".

Lo que nos llama la atencion es que hacemos tests muy similares con el modelo (GameTests) y estos no tienen ningun inconveniente.

Refactorizamos la logica de los turnos abstrayendola a un TurnManager, pero no tuvimos exito, sigue ocurriendo lo mismo.

No creemos que tenga que ver con mongo, o por lo menos no deberia, ya que la parte que toca la base de los tests esta mockeada.

Usando el debugger de Intellij pudimos identificar que es lo que hace que fallen los tests pero no la causa. Lo que sucede cuando fallan los tests es que los games que se usan en los mismos, algunas veces, se inicializan asignando todas sus towns a un unico jugador, cuando deberian repartirlos equitativamente.

Primero pensamos que podria ser un error en esa parte del codigo, la asignacion de towns; pero no encontramos nada raro, es mas si el error estuviera en esa parte del codigo, tambien deberian fallar los tests del modelo (GameTests). Por ejemplo los relacionados con la creacion del game, cambios de turno (pegandole directamente al modelo), asignacion de los towns, pero estos tests dan siempre verde.

[dpeccia]

@Pato94 agrego como aclaración que los tests que a veces fallan y a veces no son los de la inner class FinishTurn dentro del archivo GameControllerTest.kt Algo extraño que encontramos también es que si comentas todos los demás tests de ese archivo y comentas todos los games que no sean el game1, no pudimos lograr que fallen esos tests

[Pato94]

Checkouteo este branch e informo

[Pato94]

Bien, costó, pero encontré la(s) causa(s):

1- Comparten muchísimo estado entre tests, JUnit corre lo que está dentro de @BeforeEach en cada prueba, pero todas esas variables que ustedes inicializan como val game = Game(...) corren una vez sola, y el estado resultante de un tests afecta a los que corren después (Por ejemplo un game podría terminar FINISHED como parte de un test, y seguiría así para el siguiente test). No tiene una solución lineal, separen los tests en mas archivos, o inicialicen las variables dentro del @BeforeEach (pueden usar lateinit var para no lidiar con nullables). Como el orden de corrida de los tests no siempre es el mismo, compartir estado es una receta para los fallos aleatorios.

2- Usan shuffled, y se dá un problema bastante mas sutil: Supongan el siguiente caso

val game1 = Game(listOf(user1, user2), listOf(town1, town2))
// town1.owner = user1
// town2.owner = user2
val game2 = Game(listOf(user1, user3), listOf(town2, town3))
// town2.owner = user1
// town3.owner = user3

Como resultado de estas dos lineas, town1 y town2 son de user1, cuando pase el turno va a ser declarado ganador, y cualquier siguiente finishTurn va a tirar un error.

Son dos problemas de como eligieron hacer tests, y no encontré ningún problema de lógica. No se me ocurren casos donde estos problemas se den en producción.

Saludos

[dpeccia]

Gracias @Pato94 ! Nos estaba costando bastante encontrar que era, en estos dias lo corregimos