-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #182 failing tests #197
Conversation
5487190
to
acff235
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GROSO Dodain!!! 👨💻 🥇 👾
de paso adelanté la validación de la carpeta assets que estaba metida dentro del método eventFor
Entiendo que esto cambió el comportamiento: antes fallaba cuando se conectaba el server y al levantar la consola, no?
Recuerdo de haber peleado con Ivo por eso, habría que hacer una buena prueba si eso falló.
¿está bien tener un doble listen?
No vi dónde está eso... no suena bien, pero si no se queja y funciona como queremos 🤷
|
||
if (debug) timeEnd(successDescription('Run finalized successfully')) | ||
|
||
if (!game) { | ||
fileLogger.info({ message: `${programIcon} Program executed ${programFQN} on ${project}`, timeElapsed: timeMeasurer.elapsedTime(), ok: true }) | ||
process.exit(0) | ||
} | ||
|
||
return ioGame |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yo en un momento que vimos este error pensé en algo así, pero no me gustaba principalmente porque quiero que ese io se levante desde una native de game (y que a la consola le de lo mismo que sea desde un program, game o REPL).
Pero podemos dejarlo así y después hacer el cambio (para mí sería mejor que devolviera el interpreter, y buscar el io en el objeto game) 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
me gusta pero no estoy seguro de cómo ese ioGame
vive dentro del intérprete. +1 a salir con ésto y mejorarlo más adelante.
// Be careful, if you are in developing mode | ||
// and some of these tests fail it will lead to exit code 13 | ||
// because an active session of the dynamic diagram | ||
// will remain running in background |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😮
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hice la prueba: levanté un juego en el 3000 y corrí los tests y falla correctamente el test de game porque dice que el puerto 3000 está en uso. Pero no tiene mayores problemas, los otros tests pasan. Cerré el juego, volví a correr los tests y pasaron perfectamente. Así que me animé a eliminar esos comentarios tremendistas.
A mí me parecía un feature, pero podemos volverlo para atrás si quieren dejar corriendo el juego aunque sea con las imágenes rotas. De paso probamos el test para comprobar que falle.
El doble listen está en la función |
Resumen, ver #182
diagram.test.ts
)dynamicDiagram.test.ts
que sí prueba toda la interacción del repl con el socket del diagrama dinámico, agregamos un finally para que cierre el server al final, ya que si tiraba error elserver.close()
no se ejecutaba y los otros tests que dependían de tener el puerto 3000 libre fallabanrun.test.ts
, ya que el test del game confiaba en chai, a partir de recibir una funcióndone
como parámetro. No obstante, necesitamos cerrar elioGame
. Al principio fui por la propuesta de agregar un parámetrodryRun
en las opciones del run, pero después se me ocurrió devolver elioGame
en la promesa. Con eso el test pudo poner unawait
, cerrar elioGame
a mano y luego testear que el juego anduvo correctamente. En caso de error logueamos el mensaje para tener una buena experiencia de desarrolladore. Para testear game creé un ejemplo más, y de paso adelanté la validación de la carpeta assets que estaba metida dentro del métodoeventFor
.La duda que me queda es acá:
¿está bien tener un doble listen?