Strange platform-specific test failure in OldFormForXTests since r16366

[Sasha Romijn]

Since r16366, which fixes #811, I see a failure on one of my boxes for model_forms.OldFormForXTests.test_media_on_modelform. But, it only occurs when running the full suite - not when just running model_forms. The bug has also not been seen by any other testers, so this might be some obscure race condition.

The failure I get is:

======================================================================
FAIL: test_media_on_modelform (modeltests.model_forms.tests.OldFormForXTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/erik/trunk/tests/modeltests/model_forms/tests.py", line 1377, in test_media_on_modelform
    self.assertEqual(f.errors, {'field': [u'Enter only digits separated by commas.']})
AssertionError: {'field': [u'Enter a valid IPv4 or IPv6 address.']} != {'field': [u'Enter only digits separated by commas.']}

[Sasha Romijn]

This is a bit over my head, so it would be nice if someone else could have a look. I have gotten close to the problem, but not close enough.

Summary

In some way, the CommaSeparatedIntegerField throws an error which is configured in the GenericIPAddressField, but only under very specific test conditions.
Beyond this, it's difficult for me to debug, because I know little about the ORM and how fields and their error messages are processed. I can also not think of a testcase that would make this visible on more platforms. I am guessing that this is not really a race condition, because on the same platform, any variety of tests (as long as it includes the two above) produces the failure.

Environment

I can reproduce the problem by running the model_forms.OldFormForXTests.test_media_on_modelform validation.GenericIPAddressFieldTests.test_correct_v6_ip_passes. Running just the media_on_modelform test does not cause any problems. Replacing test_correct_v6_ip_passes with any of the other tests in that testcase has the same result.

This succeeds on my Mac OS X, running Python 2.6.1 (r261). The system on which I have the failure runs 2.6.5 (r265).

What I can figure out

As the test says, the failure occurs in line 1377 of tests.py. This expects the "Enter only digits separated by commas" error but gets the "Enter a valid IPv4 or IPv6 address". This comes from validators.py line 163 - it's not the string from line 160. So, this is the string that eventually gets set in GenericIPAddressField.default_error_messages.

The validation is run against a CommaSeparatedIntegerForm, which is a plain modelform based on the CommaSeparatedInteger model, declared in test models. This contains a plain CommaSeparatedIntegerField.

I have opened a pdb session just before the failing assert:

# Yes, f.errors really contains the wrong error message
(Pdb) f.errors
{'field': [u'Enter a valid IPv4 or IPv6 address.']}

# If we create a fresh form instance, it has the same issue
(Pdb) CommaSeparatedIntegerForm({'field': '1a,2'}).errors
{'field': [u'Enter a valid IPv4 or IPv6 address.']}

# If we create a fresh model_instance, it has the same issue too -
# this issue is unrelated to the form or modelform
(Pdb) csi = CommaSeparatedInteger(field="1a,2")
(Pdb) csi.full_clean()
*** ValidationError: {'field': [u'Enter a valid IPv4 or IPv6 address.']}

So, the problem consistently occurs for any instance of CommaSeparatedInteger. Looking deeper into full_clean() it will at some point call clean() on the field.

# So, is it really calling an instance of CommaSeparatedIntegerField?
(Pdb) csi._meta.fields[1]
<django.db.models.fields.CommaSeparatedIntegerField: field>

# And what if we talk to this instance directly? Same result. So the problem
# is somewhere with this field instance.
(Pdb) csi._meta.fields[1].clean("1a,2", csi)
*** ValidationError: [u'Enter a valid IPv4 or IPv6 address.']

I can also confirm it is not just due to the loading of the GenericIPAddressTestModel, the following code works as expected, throwing django.core.exceptions.ValidationError: {'field': [u'Enter only digits separated by commas.']}.

from tests.modeltests.model_forms.models import *
from tests.modeltests.validation.models import *

try:
        GenericIPAddressTestModel(generic_ip="1.2.3.410").full_clean()
except:
        pass

CommaSeparatedInteger(field="2a,2").full_clean()

[Harro]

Did you try running the tests with the bisect option for those specific tests? Might give you a clearer idea what other test makes that one fail

[Sasha Romijn]

Replying to hvdklauw:

Did you try running the tests with the bisect option for those specific tests? Might give you a clearer idea what other test makes that one fail

Eh, unless I understand --bisect, that doesn't help once you've narrowed the problem down to "these two tests in combination". That's how far I am already.

[Karen Tracey]

In [16390]:

Refs #16234: Avoid GenericIPAddressField clobbering Field default_error_messages dict.

[Karen Tracey]

I was seeing this on Ubuntu 10.04, Python 2.6.5 (not tried anywhere else); r16390 fixes it for me. I'm pretty sure that's the right fix but not entirely certain; I'm a little confused that seeing this seems to be limited to only some systems. The new GenericIPAddressField is modifying self.default_error_messages in __init__ but it has not declared that attribute at the class level, so the change is affecting the parent Field class. At first I thought to override just a single key in that dict the subclass should copy the dict, but on further looking I think this code:

https://code.djangoproject.com/browser/django/trunk/django/db/models/fields/__init__.py#L115

takes care of gathering up all the key/values specified in all superclasses. But it's late and I could be missing something, so I put refs in the commit in stead of fixes. Anyone more awake than me feel free to review and either close this as fixed or explain what the right fix should be.

[Karen Tracey]

And thanks very much erikr for the details on which specific tests together caused the failure.

[Sasha Romijn]

Very nice find. Resolved for me too in r16390. But indeed, it still leaves a few questions open.

Using r16389, I managed to produce an even tinier test script that triggers the problem. My problem before was import order, with the correct import order, it'll go wrong even without ever instantiating a GenericIPAddressField:

So, this produces the "Enter a valid IPv4..." exception before your fix

from tests.modeltests.validation.models import *
from tests.modeltests.model_forms.models import *

CommaSeparatedInteger(field="2a,2").full_clean()

I can understand why the import order matters, but not why this even happens when not instantiating GenericIPAddressField. After all, the code setting the default error messages is being called from init().

If GenericIPAddressfield.init() is being called, I can sort of understand this happening - but still none of it explains why I can't reproduce this on my Mac OS X install, and neither could anyone else at the sprints...

[Claude Paroz]

If no further issues have been reported here, let's consider this ticket fixed.