A guarda que nunca esteve lá

O engine do Greenhouse tem um punhado de funções que movem um projeto pelo seu ciclo de vida: promover uma ideia, registrar um toque, avançar de estágio, guardar no vault, lançar. Cada uma delas lê o estado atual do item antes de decidir o que fazer. Nenhuma checava se esse estado era de fato um ponto de partida sensato.

É estranho notar isso em código que está em produção há semanas. Funcionava, porque a UI só oferecia essas ações a partir do lugar certo - um botão “Vault” só aparece num item que você pode guardar. Mas as funções do engine em si não impunham isso. Chamar vault_item duas vezes no mesmo item (um duplo clique, uma nova tentativa, um chamador futuro que não é a UI atual) e ela alegremente re-carimbava o timestamp de vaulting, movendo silenciosamente a meta de quanto tempo algo estava dormente. Chamar promote_idea num projeto que já está três estágios à frente, e ela o resetaria de volta à estaca zero.

Adicionar as checagens foi a parte fácil

Quatro funções, quatro guardas de uma linha, todas usando um tipo de erro que já existia. Rejeitar o status de partida errado, retornar cedo, pronto. Escrevi a guarda, escrevi um teste que guarda algo no vault duas vezes e checa que o timestamp não mudou, e passei para a próxima função.

A interessante foi a quinta função, a que o issue nem mencionava.

A função que chamava outra função parou de conseguir

Avançar um projeto para o próximo estágio faz duas coisas: registra um toque (você trabalhou nisso, é por isso que está se movendo) e move a pasta para o diretório do novo estágio. Isso eram dois commits separados no banco: registrar o toque e, num segundo passo, mover o estágio. Se o segundo passo falhasse, o primeiro já tinha ido para frente. O projeto afundaria para o fundo da lista de trabalho como se você tivesse acabado de trabalhar nele, sem de fato ter ido a lugar nenhum.

Corrigir isso significava combinar as duas escritas numa única transação. Simples o bastante, exceto que a etapa “registrar um toque” não era uma escrita de banco inline nesta função - era uma chamada para a função dedicada de registro de toque, que abre e comita sua própria transação internamente. Você não pode aninhar uma transação SQLite dentro de outra na mesma conexão. Então, no momento em que escrevi uma única transação envolvendo as duas operações, não conseguia mais chamar aquela outra função. Tive que entrar e copiar suas duas instruções de banco individuais diretamente para o novo bloco combinado.

Foi aí que ficou interessante: a função de registro de toque era uma das quatro nas quais eu tinha acabado de adicionar uma guarda. Chamá-la costumava ser o motivo pelo qual avançar um estágio era seguro contra um status de partida ruim - se o item não estivesse no estado certo, a própria chamada de toque recusaria e pararia tudo. Assim que parei de chamá-la e passei a incluir suas instruções brutas inline, essa proteção não veio junto. Nunca ficou escrito em lugar nenhum como “esta função é segura porque delega para aquela outra.” Simplesmente acontecia de ser verdade, até que uma mudança de encanamento por um motivo não relacionado a fez deixar de ser verdade, silenciosamente.

A correção foi escrever a mesma guarda uma segunda vez, diretamente na função que não podia mais emprestá-la da vizinha. O que significou que essa função precisava de uma checagem que o relato original do bug nunca pediu, porque o relato foi escrito antes de alguém rastrear o que combinar essas duas escritas realmente exigiria.

Testando uma falha sem mentir pra você mesmo sobre isso

Parte desse trabalho significava provar que um movimento de pasta falho deixa tudo intocado, e que uma escrita de arquivo falha depois de um insert bem-sucedido no banco não vaza uma linha órfã. Os dois precisam de um jeito confiável de fazer uma operação de filesystem falhar dentro de um teste.

O truque óbvio é tornar uma pasta somente leitura e tentar escrever nela. Já usei isso antes e funciona bem, até que não funciona: se o teste por acaso rodar como root, os bits de permissão deixam de importar, a escrita passa mesmo assim, e o teste passa pelo motivo completamente errado. Parece verde. Não está testando nada.

A correção que de fato se sustenta não importa quem esteja rodando o teste: colocar o tipo errado de coisa no caminho, em vez das permissões erradas. Quer que uma chamada de criação de diretório falhe? Coloque um arquivo comum ali primeiro - você não consegue transformar um arquivo num diretório não importa quem você seja. Quer que uma escrita de arquivo falhe? Coloque um diretório ali no lugar. Root não ganha isenção especial de “isso não é um diretório.”

Errei isso uma vez no processo, de um jeito que vale mencionar porque é uma armadilha fácil: uma operação no código sob teste move uma pasta inteira para o lugar com um rename, em vez de escrever diretamente nela. Coloquei meu arquivo bloqueador direto no destino, e o teste falhou - mas pelo motivo errado. Renomear uma pasta para cima de um destino existente e não vazio falha por conta própria, antes mesmo do trecho de código que eu de fato queria testar rodar. A correção foi colocar o arquivo bloqueador dentro da pasta que estava sendo movida, para que ele viajasse junto com o rename e só causasse problema quando de fato chegasse onde eu queria interromper algo.

O que ficou comigo

O padrão recorrente aqui não é “adicionar validação,” é “a correção de uma função às vezes depende de como ela é chamada, não só do que ela faz.” Combinar as escritas de duas funções numa única transação é uma mudança de encanamento - não parece que deveria afetar comportamento. Mas se uma dessas funções estava silenciosamente agindo como guardiã da outra, remover a chamada remove a guarda, e nada no sistema de tipos avisa que isso aconteceu. O único jeito que encontrei de pegar isso foi perguntando, função por função, “o que aconteceria agora se eu chamasse isso a partir de um estado que ela não esperava” - e essa é uma pergunta que vale a pena fazer de novo toda vez que um refactor muda quem chama quem, não só uma vez quando as guardas são escritas pela primeira vez.