Saltar al contenido
Development

Una prueba que no demostró nada, y el bug que se suponía que iba a atrapar

Por Victor Da Luz
rustsqlitetestingdev-loggreenhouse

La revisión del repo que viene alimentando toda esta tanda de posts dejó un ítem sin abrir: un revoltijo de cinco hallazgos pequeños, todos de severidad BAJA, agrupados en un solo issue de “lote de limpieza” en vez de cinco separados. Un patrón repetido de mutex-unwrap, un comentario desactualizado, un campo de error hardcodeado, una inconsistencia de nombres, y una nota de copy de interfaz marcada explícitamente como “arreglar después.” Nada de esto iba a merecer su propia entrada de bitácora por mérito propio. Juntos valían una tarde.

Tres rápidos

Cuatro call sites en dos archivos hacían exactamente lo mismo para leer el estado compartido de vault-root de Greenhouse: state.vault_root.lock().expect("vault_root mutex poisoned"). Copiado y pegado suficientes veces como para que arreglar el patrón una vez significara algo distinto a arreglarlo en cada lugar donde aparecía. Como el valor protegido es un simple Option<PathBuf> que solo se clona hacia afuera o se reemplaza entero (nunca se muta en varios pasos mientras está tomado), recuperarse de un lock envenenado es en realidad seguro acá, no hay estado a medio escribir que un holder en pánico pudiera haber dejado atrás. Así que el campo pasó a ser privado, y dos métodos tomaron el control: AppState::vault_root() para leer, AppState::set_vault_root() para escribir, ambos usando lock().unwrap_or_else(PoisonError::into_inner) en vez de un expect a secas. Cuatro call sites se colapsaron en dos métodos con una sola política de manejo de envenenamiento en vez de cuatro copias del mismo criterio.

Un comentario en el router de nivel superior todavía describía el paso del selector de carpeta del onboarding en tiempo futuro, “once the folder-picker issue sets it,” “comes with its set_vault_root”, para una funcionalidad que ya estaba publicada y en uso diario desde hacía más de una semana. Los comentarios que describen trabajo como algo que vendrá no se revisan una vez que ese trabajo aterriza, nada lo obliga. Lo reescribí en tiempo presente, describiendo lo que el código hace ahora en vez de lo que iba a hacer.

Y una función, load_initial, era el único nombre en snake_case en un archivo Svelte que por lo demás era consistentemente camelCase. La renombré a loadInitial. El tipo de arreglo que toma diez segundos y le habría causado a otra persona un momento de “espera, ¿por qué esta es distinta?” cada vez que pasara por ahí leyendo.

El que en realidad era un bug

El cuarto ítem se veía igual de chico en el papel: una función auxiliar que parsea una cadena de estado de la base de datos reporta de qué columna del result-set vino cuando el parseo falla, y ese número de columna estaba hardcodeado a 2, correcto para el único llamador que lee el estado como la tercera columna seleccionada, incorrecto para un segundo llamador que selecciona el estado primero. Un problema cosmético en un mensaje de error que nadie vería a menos que algo más ya hubiera salido mal. Escribí una prueba para demostrar el arreglo: insertar una fila con un estado inválido, llamar a la consulta que selecciona el estado como columna 0, verificar que el error reporte la columna 0.

La prueba falló. No porque el arreglo estuviera mal, sino porque el bug que estaba arreglando en realidad no podía ocurrir por ese camino de código. La consulta en cuestión filtra con WHERE status IN ('vaulted', 'released') antes de que ninguna fila llegue al parser, y esos son exactamente los dos valores que siempre parsean con éxito. Si se le da un estado que el parser no reconocería, la propia cláusula WHERE de la consulta descarta la fila antes de que se intente parsear siquiera. El bug de columna equivocada era real, el 2 hardcodeado genuinamente estaba mal para ese llamador, pero era latente de una forma que ninguna prueba de integración podría haber observado jamás, porque el SQL alrededor impide estructuralmente que ese input fallido llegue a aparecer.

Esa es una forma distinta de “esto no se puede probar” comparada con las excusas habituales. No es intermitente, no es lenta, no está bloqueada detrás de infraestructura que no tengo en CI. Es un arreglo de corrección para un camino de código que hoy es demostrablemente inalcanzable, y que se mantiene arreglado y correcto si esa cláusula WHERE alguna vez se afloja, o si la consulta se reutiliza en algún lugar menos restrictivo. Como no podía demostrar la falla a través del camino de integración, en cambio probé la unidad real, llamé directamente a la función ahora parametrizada con ambos valores de columna y verifiqué que cada una reportara de vuelta lo que se le había dado. Menos dramático que una repro de punta a punta, pero es el nivel donde el arreglo en verdad vive: la función ahora recibe una columna, y usa la que se le da.

Reflexión

La lección no es “siempre escribir una prueba.” Escribí una, y me dijo algo cierto, solo que no lo que esperaba. La lección es que una prueba en rojo es información sin importar de qué lado del arreglo esté apuntando: puede significar que el arreglo está mal, o puede significar que el modelo mental de cómo ocurre el bug estaba mal. Vale la pena descubrir ambas cosas antes de mergear. Un lote tan chico podría haber entrado sin nada de esa fricción, cuatro ajustes a nivel de línea, publicar y listo, pero el ítem más chico del lote fue el que cambió lo que entendía sobre el código.

Lecturas relacionadas

Development

El guardián que nunca estuvo

Agregar una verificación de estado a una función de Rust hizo que ya no pudiera llamar a otra función de la misma manera, y esa restricción abrió en silencio un hueco que ninguna de las dos funciones tenía por sí sola.

Leer