TreinaDev / td11-portfoliorrr

4 stars 0 forks source link

[Fix] Resolve bug do teste `ActiveStorage::Blob` #153

Closed eliseuramos93 closed 9 months ago

eliseuramos93 commented 9 months ago

Essa PR aborda e resolve #152

O teste spec/system/profile_photo/user_edits_profile_photo_spec.rb:21 falhava intermitentemente, e não tinhamos conhecimento do motivo exato desse problema. Ele foi implementado verificando se o banco de dados possuía apenas um anexo após o usuário atualizar sua foto de perfil, e realizava uma contagem dos Blobs de ActiveStorage para validar se o anexo anterior era removido automaticamente através do update.

Segundo a documentação oficial do Rails sobre ActiveStorage, ao utilizar o método :has_one_attached para anexar um arquivo à um model sem nenhuma opção para seus dependentes, o anexo anterior será expurgado assim que o registro anterior for removido.

image

has_one_attached está sendo utilizado da forma descrita acima no modelo Profile (atualmente linha 19 de app/models/profile.rb), portanto o comportamento padrão do Rails já deve ser esperado. E o teste já valida que existe apenas um anexo em todo banco de dados, não existindo mais o anterior. Então, a linha contendo expect(ActiveStorage::Blob.count).to eq 1 é apenas uma validação do comportamento padrão do Rails, e não de uma lógica do app Portfoliorrr.

A suspeita é que esse seja um bug semelhante ao que acontecia com o Capybara ao usar expect(current_path).to eq route_path, no qual mesmo com a lógica correta e ação acontecendo normalmente ao usar a app, dentro do ambiente de testes não havia tempo suficiente para current_path ser gerado, retornando um falso negativo.

Por isso, esse PR remove a linha que testa a contagem de Blobs.

akaninja commented 9 months ago

Eu diria até que fazer essa contagem não é necessária. Já que você está trocando de uma pra outra, seria 1 de qualquer jeito (porque é has_one, não has_many). Se você estivesse deletando anexos e queria confirmar com absoluta certeza que foi apagado, dai talvez esse teste fosse interessante. Mas mesmo assim, na minha opinião, bastaria o teste de view para esse caso. :)

eliseuramos93 commented 9 months ago

Eu diria até que fazer essa contagem não é necessária. Já que você está trocando de uma pra outra, seria 1 de qualquer jeito (porque é has_one, não has_many). Se você estivesse deletando anexos e queria confirmar com absoluta certeza que foi apagado, dai talvez esse teste fosse interessante. Mas mesmo assim, na minha opinião, bastaria o teste de view para esse caso. :)

Esse teste foi bem "insegurança de junior feat. não entender completamente o has_one_attached", porque queríamos ter algo que nos desse segurança de que não estaríamos poluindo o banco de dados com várias fotos de perfil que não eram mais usadas. Mas agora já fica o aprendizado pra quando lidar com o ActiveStorage no futuro :)