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

ajustando metodo is_valid como único #260

Merged
merged 2 commits into from
Nov 24, 2023

Conversation

antoniamaia
Copy link
Member

@antoniamaia antoniamaia commented Oct 19, 2023

Descrição

Fiz todas as alterações, mas ainda dá erro 😢 deixando em draft pra vc ver onde cheguei 😄

Mudanças Propostas

Checklist de Revisão

  • Eu li o Contributing.md
  • Os testes foram adicionados ou atualizados para refletir as mudanças (se aplicável).
  • Foi adicionada uma entrada no changelog / Meu PR não necessita de uma nova entrada no changelog.
  • A documentação em português foi atualizada ou criada, se necessário.
  • Se feita a documentação, a atualização do arquivo em inglês.
  • Eu documentei as minhas mudanças no código, adicionando docstrings e comentários. Instruções
  • O código segue as diretrizes de estilo e padrões de codificação do projeto.
  • Todos os testes passam. Instruções
  • O Pull Request foi testado localmente. Instruções
  • Não há conflitos de mesclagem.

Comentários Adicionais (opcional)

Issue Relacionada

Closes #259

"brutils.license_plate._is_valid_old_format", return_value=False
) as mock__is_valid_old_format:
# When licence plate old format isn't valid, returns False
self.assertIs(is_valid("123456", "old_format"), False)
Copy link
Contributor

@cuducos cuducos Nov 7, 2023

Choose a reason for hiding this comment

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

Na função você utilizou "old format" — com espaço ao invés de _. Por isso não chama o mock como esperado aqui : )

Comment on lines 91 to 95
with patch(
"brutils.license_plate._is_valid_old_format"
) as mock__is_valid_old_format:
# When license plate as mercosul is_valid, returns True
self.assertIs(is_valid("ABC4E67"), True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Como a função is_valid retorna _is_valid_old_format(…) or _is_valid_mercosul(…), se não retornar algo q se pareça com False, nunca vamos chegar no _is_valid_mercosul(…).

Como vc não definiu o retorno do mock de _is_valid_old_format, ele retorna um objeot Mock() que não se parece com False. Por isso nunca executamos a função do Mercosul como esperado : )

Suggested change
with patch(
"brutils.license_plate._is_valid_old_format"
) as mock__is_valid_old_format:
# When license plate as mercosul is_valid, returns True
self.assertIs(is_valid("ABC4E67"), True)
with patch(
"brutils.license_plate._is_valid_old_format", return_value=False
) as mock__is_valid_old_format:
# When license plate as mercosul is_valid, returns True
self.assertIs(is_valid("ABC4E67"), True)

@antoniamaia antoniamaia marked this pull request as ready for review November 15, 2023 20:37
@antoniamaia antoniamaia requested review from a team as code owners November 15, 2023 20:37

# Checks if function _is_valid_mercosul is called with the
# correct argument
# mock__is_valid_mercosul.assert_called_once_with("ABC4E67") <--
Copy link
Member Author

Choose a reason for hiding this comment

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

@camilamaia mock outra vez fundindo o meu motor 🤯 deixei essas coisas feias (<---) nas linhas comentadas que dão sempre o mesmo Assertion error.. quando é pra ser chamado, (e é chamado pq eu printei 💅🏼 ) ele diz que não foi chamado nenhuma vez, e o contrário a mesma coisa. Quando estava procurando documentação, em algum lugar (bem obcuro) disse que o mock pode ter um side effect quando usado muitas vezes, e utilizar essa função reset ajudaria. Para esse caso realmente funcionou, mas quis colocar tudo aqui pra vc acompanhar todo o rolê :)


# Checks if function _is_valid_mercosul is called with the
# correct argument
# mock__is_valid_mercosul.assert_called_once_with("ABC4E67") <--
Copy link
Member Author

Choose a reason for hiding this comment

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

@camilamaia aqui :)

mock__is_valid_mercosul.assert_called_once_with("ABC-123")
# Checks if function _is_valid_old_format is called with the
# correct argument
# mock__is_valid_old_format.assert_called_once_with("123-456")<--
Copy link
Member Author

Choose a reason for hiding this comment

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

@camilamaia aqui :)

Copy link
Member

@camilamaia camilamaia left a comment

Choose a reason for hiding this comment

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

Boa! Acho que com o pair que a gente fez, arrumando os mocks de um jeito mais claro, ficou mais suave para entender tb. My bad de ter criado tudo numa confusão só.

…#275)

Bumps [identify](https://github.com/pre-commit/identify) from 2.5.31 to 2.5.32.
- [Commits](pre-commit/identify@v2.5.31...v2.5.32)

---
updated-dependencies:
- dependency-name: identify
  dependency-type: indirect
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Copy link

codecov bot commented Nov 24, 2023

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (785116a) 100.00% compared to head (619796a) 86.02%.
Report is 1 commits behind head on main.

❗ Current head 619796a differs from pull request most recent head e2ccbec. Consider uploading reports for the commit e2ccbec to get more accurate results

Files Patch % Lines
brutils/license_plate.py 15.00% 17 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              main     #260       +/-   ##
============================================
- Coverage   100.00%   86.02%   -13.98%     
============================================
  Files           10       10               
  Lines          268      272        +4     
============================================
- Hits           268      234       -34     
- Misses           0       38       +38     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@antoniamaia antoniamaia merged commit 52f0088 into brazilian-utils:main Nov 24, 2023
6 checks passed
@antoniamaia antoniamaia deleted the 259 branch November 24, 2023 14:05
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.

Ajustar método is_valid para placas de carro para ser único
3 participants