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

Fix misspecified tests using MagicMock().called_with() #1072

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

austinweisgrau
Copy link
Collaborator

@austinweisgrau austinweisgrau commented Jun 3, 2024

See issue #1071

The tests which use this method must be rewritten - some of them fail once the misspecified method is fixed, and will need to be rewritten to actually test what they intend to test.

TO DO:

  • Fix remaining twilio tests
  • Fix salesforce tests
  • Fix documentation recommending use of called_with()

BLOCKING:

  • In python 3.12 it's no longer syntactical to use MagicMock().called_with(), so these fixes must be made before the python 3.12 migration can be made (Support for python3.12 #1070)

Prior implementation ran `assert ...MagicMock.called_with(...)`

`called_with` is not a real test method on MagicMock, so it returns a
MagicMock instance by default, which is truthy and evaluates to true
by assert. This test wasn't checking for anything and would pass no
matter what actually happened in the code.

These new tests check for the intended methods being called.
@austinweisgrau austinweisgrau changed the title Work in Progress - Fix misspecified tests using MagicMock().called_with() Fix misspecified tests using MagicMock().called_with() Jun 21, 2024
Copy link
Collaborator

@shaunagm shaunagm left a comment

Choose a reason for hiding this comment

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

This looks good to me - thanks for catching & fixing these!

@shaunagm shaunagm merged commit c49f631 into move-coop:main Jun 25, 2024
14 checks passed
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.

2 participants