Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#16645 closed Bug (fixed)

OldFormForXTests.test_image_field fails under Oracle

Reported by: aaugustin Owned by: nobody
Component: Forms Version: 1.3
Severity: Release blocker Keywords: oracle
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Attachments (1)

16645.patch (1.4 KB) - added by aaugustin 3 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 3 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 3 years ago by aaugustin

  • Keywords oracle added

comment:3 Changed 3 years ago by aaugustin

I can't reproduce this under Oracle 10. The CI server is running Oracle 11, it may explain the difference.

comment:4 Changed 3 years ago by aaugustin

The problem is in modeltests/model_forms/tests.py, in a test for ImageField, at line 1320:

        self.assertEqual(instance.image.name, None)

In the same file, there's an equivalent test for FileField, at line 1192 — but it's written slightly differently:

        self.assertEqual(instance.file.name, '')

I suggest to make the test for ImageField similar to the test for FileField. See attached patch.

With this patch, the tests still pass under SQLite, MySQL, PostgreSQL and Oracle 10. I don't have access to Oracle 11, but it's very likely that the change will fix the failing test; the CI will tell us.

Changed 3 years ago by aaugustin

comment:5 Changed 3 years ago by aaugustin

Also, this patch enforces a best practice, namely not using null=True on text-based fields.

comment:6 Changed 3 years ago by aaugustin

IRC discussion about this patch:

2011-08-28 15:36:35 <cramm> about the test failure with Oracle 11 related to a nullable filefield
2011-08-28 15:37:18 <cramm> I've been reviewing it. I don't feel comfortable with adapting the tests to workaround that
2011-08-28 15:38:37 <cramm> I konw there is some recommendation in the charfield docs discouraging using null=True, but we have a handful of models with FileField's and ImageField with null=tru in out tests, and they work correctly
2011-08-28 15:39:57 <cramm> ... and a bunch of models with other CharField-derived fields also with null=True. In some tests we even test for ORM behavior when confronted with the null=Tree
2011-08-28 15:40:42 <mYk> cramm: the problem here is that we test that an ImageField with no value has its name set to ''
2011-08-28 15:40:47 <cramm> So, I'd prefer to see first why things don't fail in 10g and fail in 11g
2011-08-28 15:41:03 <mYk> but since the field is nullable the result should be None
2011-08-28 15:41:47 <mYk> the docs don't say exactly how <file>.name is supposed to behave, but basically, it's the string stored in the db
2011-08-28 15:42:46 <mYk> in short, I agree that I didn't nail the root cause, and the expected behavior of <file>.name is underdefined
2011-08-28 15:43:09 <cramm> I gree with the latter :)
Version 0, edited 3 years ago by aaugustin (next)

comment:7 Changed 3 years ago by ikelly

I am able to reproduce the failure using Oracle 10g. This kind of thing has come up before in the tests. The usual fix is to predicate the expected output on the "interprets_empty_strings_as_nulls" feature.

comment:8 Changed 3 years ago by ikelly

  • Resolution set to fixed
  • Status changed from new to closed

In [16919]:

Fixed #16645: fixed a broken test to work in Oracle.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.