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

Fixed #26778 -- Fixed ModelSignal.connect() weak argument. #6802

Merged
merged 1 commit into from Jun 19, 2016

Conversation

timgraham
Copy link
Member

pass
signals.pre_init.connect(callback)
signals.pre_init.disconnect(callback)
self.assertTrue(ref.called)
Copy link
Member

Choose a reason for hiding this comment

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

I'd use ref.assert_called_once_with() here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't sure how to get the appropriate arguments:

ref.assert_called_once_with(callback)

AssertionError: Expected call: ref(<function callback at 0x7f1de47ec500>)
Actual call: ref(<function callback at 0x7f1de47ec500>, 
<bound method ModelSignal._remove_receiver of
<django.db.models.signals.ModelSignal object at 0x7f1de706d4d0>>)

Copy link
Member

Choose a reason for hiding this comment

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

From looking at the code the call could differ from Python 2 to 3 and is really an implementation detail which is not worth testing after all.

My initial reflexion was more about the fact assert_not_called() was used below instead of self.assertFalse(ref.called) but now I realize there's no assert_called() method.

LGTM

@charettes
Copy link
Member

LGTM otherwise.

@timgraham timgraham merged commit 9bf8d50 into django:master Jun 19, 2016
@timgraham timgraham deleted the 26778 branch June 19, 2016 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants