[Django] #26122: Weird test regression in 1.8.5
Django
-----------------------------------+--------------------
Reporter: knyghty | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------
Smallest example I could get to:
{{{
# forms.py
from django import forms
from django.contrib.auth import get_user_model
class InlineEditEmailForm(forms.ModelForm):
class Meta:
model = get_user_model()
fields = ['email']
# views.py
import copy
from django.http import HttpResponse
from .forms import InlineEditEmailForm
def edit_user_email(request):
user = request.user
old_user_data = copy.copy(user)
form = InlineEditEmailForm(data=request.POST, instance=user)
if form.is_valid():
new_user_data = form.cleaned_data
user = form.save()
if old_user_data.email != new_user_data['email']:
response = HttpResponse()
response.status_code = 200
return response
response = HttpResponse()
response.status_code = 400
return response
# tests.py
from django.contrib.auth import get_user_model
from django.test import TestCase
class EditUserEmailTestCase(TestCase):
def test_post(self):
get_user_model().objects.create_user('test', 'x...@example.com',
'pw')
data = {'email': 'te...@example.com'}
self.client.login(username='test', password='pw')
response = self.client.post('/', data)
self.assertEqual(response.status_code, 200)
}}}
* Works in 1.8.4, doesn't work in 1.8.5+ or 1.9.
* Works when using a browser, only fails during tests.
* Changing first line of the view to {{{user =
get_user_model().objects.create_user(...)}}} makes the test pass again.
--
Ticket URL: <https://code.djangoproject.com/ticket/26122>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: knyghty | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.9
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by knyghty):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
New description:
Smallest example I could get to:
{{{#!python
# views.py
import copy
from django.http import HttpResponse
from .forms import InlineEditEmailForm
class EditUserEmailTestCase(TestCase):
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/26122#comment:1>
Django
Reporter: knyghty | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.9
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
All the changes in 1.8.5 are documented
[https://docs.djangoproject.com/en/1.8/releases/1.8.5/ in the release
notes]. At first glance, #25431 looks like it might be the cause, though I
didn't study it long enough to say why. Can you provide any more analysis
with that information?
--
Ticket URL: <https://code.djangoproject.com/ticket/26122#comment:2>
Django
Reporter: knyghty | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.9
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by knyghty):
I managed to bisect it to this commit:
https://github.com/django/django/commit/35355a4ffedb2aeed52d5fe3034380ffc6a438db
But that's all I have to go on.
--
Ticket URL: <https://code.djangoproject.com/ticket/26122#comment:3>
Django
---------------------------+--------------------------------------
Reporter: knyghty | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* cc: benjaminjkraft (added)
* version: 1.9 => 1.8
* component: Testing framework => Utilities
Comment:
It looks like your bisection is correct and `copy.copy()` no longer has
the same behavior with `SimpleLazyObject` (`request.user` in your code).
Let's see if the author of the pickling patch, Ben, can advise.
--
Ticket URL: <https://code.djangoproject.com/ticket/26122#comment:4>
Django
---------------------------+--------------------------------------
Type: Bug | Status: new
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by benjaminjkraft):
Oh, yeah, I can see now how the dummy `__getstate__` might break this,
because it seems like if there's no `__copy__`, `copy` will use
`__getstate__` but not `__reduce__` -- sorry I didn't realize before.
(This stuff is not very well documented so I'll need to look more closely
to be sure that that's the issue but it definitely seems plausible.)
I'm not entirely sure, but the simplest way to solve this is probably just
writing a `__copy__` method for `LazyObject`, either by just proxying the
wrapped object's `__copy__` or by doing something similar to the
`__deepcopy__` implementation. I'm not immediately sure which one of
those is more correct.
--
Ticket URL: <https://code.djangoproject.com/ticket/26122#comment:5>
Django
Reporter: knyghty | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/26122#comment:6>
Django
Reporter: knyghty | Owner:
| benjaminjkraft
Type: Bug | Status: assigned
Component: Utilities | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by benjaminjkraft):
* status: new => assigned
* owner: nobody => benjaminjkraft
* has_patch: 0 => 1
Comment:
Fixed in https://github.com/django/django/pull/6030, along with regression
tests (similar to those for deepcopy, which I also improved a bit). This
reverts to the behavior that matches deepcopy of not initializing the
object if it hasn't already been initialized (instead we just create
another object with the same initializer). Incidentally, this seems to
have been broken in various ways in other past versions; the tests I added
fail even before 35355a4.
--
Ticket URL: <https://code.djangoproject.com/ticket/26122#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: knyghty | Owner:
| benjaminjkraft
Component: Utilities | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"13023ba86746980aace2341ba32a9419e7567751" 13023ba8]:
{{{
#!CommitTicketReference repository=""
revision="13023ba86746980aace2341ba32a9419e7567751"
Fixed #26122 -- Fixed copying a LazyObject
Shallow copying of `django.utils.functional.LazyObject` or its subclasses
has
been broken in a couple of different ways in the past, most recently due
to
35355a4.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26122#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: knyghty | Owner:
| benjaminjkraft
Type: Bug | Status: closed
Component: Utilities | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"79c3950562a3f9c9d6d5e37990dfa9eb84052de7" 79c39505]:
{{{
#!CommitTicketReference repository=""
revision="79c3950562a3f9c9d6d5e37990dfa9eb84052de7"
[1.8.x] Fixed #26122 -- Fixed copying a LazyObject
Shallow copying of `django.utils.functional.LazyObject` or its subclasses
has
been broken in a couple of different ways in the past, most recently due
to
35355a4.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26122#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: knyghty | Owner:
| benjaminjkraft
Type: Bug | Status: closed
Component: Utilities | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"dee5896b55385d2c66d8c1b8386604868f7fc6b4" dee5896]:
{{{
#!CommitTicketReference repository=""
revision="dee5896b55385d2c66d8c1b8386604868f7fc6b4"
[1.9.x] Fixed #26122 -- Fixed copying a LazyObject
Shallow copying of `django.utils.functional.LazyObject` or its subclasses
has
been broken in a couple of different ways in the past, most recently due
to
35355a4.
Backport of 13023ba86746980aace2341ba32a9419e7567751 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26122#comment:9>