-
Notifications
You must be signed in to change notification settings - Fork 31
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
Mocks returned by QuerySetMock should return self when QuerySet-generating methods are called #15
Comments
I encountered the same problem and managed to solve it, at least for my use case, in 50f7e70 |
Sorry it's taken a while to get back to this. Your change breaks the existing tests:
|
@henrikalmer If you're currently using your solution, can you take a look at my latest PR and verify that your uses still pass? Instead of just returning a new iterator, I return a new copy of the QuerySetMock for most QuerySet-returning methods. |
@boydjj I've tried the latest PR and it seems to work with well, with one exception. I need to be able to add custom queryset returning methods that correspond to methods on a custom manager. I've added a settings variable to do this locally and if you wish I could submit a pull request. |
I solved this issue in my code by using mock and return_value:
|
For complicated views, where several QuerySet-generating methods (e.g., filter() or order_by()) are called, the SharedMock returned by QuerySetMock breaks application code. Here's an example::
So mocking out a QuerySet with a SharedMock fails in any chain-calling application code.
I must admit I'm not quite clear on why this is happening at the moment, because at least on first glance it seems like self should already be returned. But I'm considering a patch that would ensure that such methods always return self, at least for QuerySetMock. Would such a patch be useful to folks? Is there something I'm missing in my use that would allow me to fix the issue otherwise? And, finally, is there a use case I'm not considering where such a patch would break the current behavior?
The text was updated successfully, but these errors were encountered: