-
-
Notifications
You must be signed in to change notification settings - Fork 7.2k
✅ Fix incorrect mocking in unit tests (issue #1780) #1781
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I suggest removing several lines to simplify tests (I tested it locally - it doesn't break tests).
Currently if you replace session.exec(select(1))
in this line with pass
, this assertion will not fail as it's expected.
This PR fixes mocking the Session
and after applying it, that test will not only ensure that there was no exception in init, but also that select(1)
was actually called.
The same for https://github.com/fastapi/full-stack-fastapi-template/blob/master/backend/app/tests/scripts/test_test_pre_start.py
@vicaya, thank you!
Could you please take a look at my suggested changes? (done)
Resolve fastapi#1780
daa3017
to
5321f5b
Compare
It seems we now have some conflicts here 🤔 😔 |
This pull request has a merge conflict that needs to be resolved. |
Fix issue #1780: the 2
test_init_successful_connection
tests were basically noops due to misuse of MagicMock.The errors went undetected until python 3.12+ tightened up the MagicMock implementation.