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

Mejora comportamiento comando test con parametros -f -d -t #162

Merged
merged 4 commits into from
Jun 15, 2024

Conversation

ivojawer
Copy link
Contributor

@ivojawer ivojawer commented Jun 7, 2024

Arregla varias cosas que surgen a partir de uqbar-project/wollok-lsp-ide#168

  • El parametro -f ahora espera el path relativo al archivo (e.g. src/pepita.wtest)
  • Si no encuentra el package o describe que esta buscando ahora no matchea ningun test y muestra un log con la causa
image

Copy link
Contributor

@fdodino fdodino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Piola!!! 🚀 🥇

const possibleTargets = getBaseNode(environment, filter, options).descendants.filter(getTestFilter(filter, options))
let possibleTargets: Test[]
try {
possibleTargets = getBaseNode(environment, filter, options).descendants.filter(getTestFilter(filter, options))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lo aprobé y me quedé pensando si por algún motivo no pusiste el resto del código acá, o bien si no querés llamar a otra función interna...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No 😅, la verdad que no lo pense tanto, como el resto del codigo no podia generar ese error no me cambiaba mucho. Lo muevo adentro del try.

o bien si no querés llamar a otra función interna...

Uh no entendi dodain, que funcion llamaria?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sería algo como todo lo que iría en el try

try {
   xxx() // función que obtiene possibleTargets y hace todo lo que ahora está post-catch
} catch ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh okay ya entendi

No tengo una opinion fuerte al respecto pero esa funcion xxx no seria simplemente getTarget? Que ganamos que moverlo a otra funcion? Y que nombre le pondriamos si no es getTarget?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lo que ganás generalmente es que evitás el problema de la indentación anidada, no mucho pero si el método tiene 88 líneas es medio un embole poder capturar el try/catch cuando lo estás leyendo (por supuesto, podés colapsarlo). Igual no digo que hagas un método privado, lo que sí no me gusta es tener un try/catch específico para eso a menos que quieras que cualquier otro error salte.

@fdodino
Copy link
Contributor

fdodino commented Jun 10, 2024

Hay que acordarse de releasear wollok-lsp-ide con este cambio para que funcionen los tests...

Copy link
Contributor

@PalumboN PalumboN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Al toke rokeeeee 🐶 💯 🚥

@PalumboN PalumboN merged commit c023d06 into master Jun 15, 2024
1 check passed
@PalumboN PalumboN deleted the test-parameters-err-handling branch June 15, 2024 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants