Skip to content
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

Un test que no envía un mensaje a assert no está rompiendo #272

Open
fdodino opened this issue Sep 7, 2024 · 3 comments
Open

Un test que no envía un mensaje a assert no está rompiendo #272

fdodino opened this issue Sep 7, 2024 · 3 comments
Labels
component: interpreter enhancement New feature or request priority: medium A "should" issue, whenever we have the time to solve it

Comments

@fdodino
Copy link
Contributor

fdodino commented Sep 7, 2024

En Wollok Xtext teníamos agregada la funcionalidad de que un test falle si al final de la ejecución no había enviado ningún mensaje a assert. Esto me sigue pareciendo piola porque hace que el CI falle y le llegue un mail al alumne.

Opiniones? @asanzo @PalumboN @ivojawer @lspigariol @npasserini

Reproducción

armar un archivo testExample.wtest como el que se forma de un wollok init y escribir

test "sin assert" {
    2 + 2
}

Lo ejecutás y no falla. Sí muestra el mensaje de warning (el validador está implementado)

Screenshot 2024-09-07 at 8 39 51 AM
@fdodino fdodino added priority: low A nice to have, or not so important issue enhancement New feature or request labels Sep 7, 2024
@PalumboN
Copy link
Contributor

PalumboN commented Sep 7, 2024

No me acordaba que en Xtext el test fallaba si assert no recibía ningún mensaje 🤔

Ok 👍

@fdodino
Copy link
Contributor Author

fdodino commented Sep 7, 2024

image

fetivamente!

@asanzo
Copy link
Contributor

asanzo commented Sep 7, 2024

TL;DR: estoy de acuerdo, sigan sigan.

Yo veo 2 casos a considerar:

  1. Me olvidé de hacer el assert o no sabía que tenía que hacerlo (en cuyo caso está bueno que falle). Pero en este caso quizás el mensaje podría decir algo así como "Todo test debe verificar algo" además de decir "no le mandaste un mensaje a assert", que es medio críptico per sé para quien está aprendiendo. Pero es un detalle.

  2. Lo que quiero probar es que efectivamente no estalle, en cuyo caso sirve el assert.doesNotThrow y la discusión que tuvimos está en Agregar a los tests el assert "esto no debe fallar" (doesNotThrowException) wollok-language#82 . Si vamos por este lado, el error al fallar por falta de mensaje al assert debería incluir algún hint de esto quizás?

Meh, en fin, me inclino por abordar el caso 1 creo.

Si además podemos detectar con un warning los assert.that(1=1) o los assert.that(true) quizás ese sea el lugar para poner la descripción de "probaste el doesNotThrowException?"

En cualquier caso avanzar en primer instancia por replicar el comportamiento de wollok xtext me parece bien

Les quiero.

@PalumboN PalumboN added priority: medium A "should" issue, whenever we have the time to solve it component: interpreter and removed priority: low A nice to have, or not so important issue labels Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: interpreter enhancement New feature or request priority: medium A "should" issue, whenever we have the time to solve it
Projects
None yet
Development

No branches or pull requests

3 participants