Code

Opened 3 years ago

Closed 3 years ago

#16694 closed Bug (fixed)

The test suite no longer runs under Oracle since r16678

Reported by: aaugustin Owned by: nobody
Component: Database layer (models, ORM) Version: 1.3
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

http://ci.django-cms.org/job/Django/database=oracle,python=python2.7/190/console

I don't know why the problem happens only with Oracle, but I reproduce it on my own machines.

Attachments (1)

16694.patch (1.1 KB) - added by aaugustin 3 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 3 years ago by aaugustin

In fact, it's certainly a consequence of this:

the Oracle backend coerces the null=True option on fields that have the empty string as a possible value

https://docs.djangoproject.com/en/dev/ref/databases/#null-and-empty-strings

comment:2 Changed 3 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Accepted

*sigh* Anytime anybody claims Oracle supports SQL, people should feel free to laugh at them. Their NULL handling has been broken forever, in multiple places, and continues to hurt people (like us) trying to write portable SQL. This particular issue is because incorrectly Oracle compares NULL and "" to be equal and the Django backend tries to make things behave uniformly with respect to that.

It would be nice to avoid having to omit that test entirely (since null=True is entirely incorrect when written on a model pk field), but that means checking for explicit null=True versus when Oracle backend adds it implicitly. Alternative is to skip the test for the Oracle backend (which hurts in the multi-db case, I suspect).

comment:3 Changed 3 years ago by mtredinnick

On the grounds that it can't make things worse, I'm about to commit a one-liner that should fix this by not doing that validation in the "empty string is the same as NULL" case. I'll check the CI server in a day or two when it's had a chance to run the Oracle tests (:-D) and see if that helped.

comment:4 Changed 3 years ago by mtredinnick

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

In [16681]:

Modify validity check from r16678 slightly to work with Oracle.

We now skip this particular check on Oracle backends. Change is based on
a backend feature check, so it also works with other backends that may
treat NULL as equal to an empty string.

Fixes #16694 (with luck).

comment:5 Changed 3 years ago by aaugustin

  • Has patch set
  • Resolution fixed deleted
  • Status changed from closed to reopened

The test suite now runs :) -- the CI server isn't *that* slow; since I've put Oracle's tablespaces in a ramfs it's purely CPU-bound.

However, the test added in r16678 fails under Oracle, because the validation doesn't happen :(

http://ci.django-cms.org/job/Django/database=oracle,python=python2.7/193/testReport/modeltests.invalid_models.tests/InvalidModelTestCase/test_invalid_models/

Patch attached, tested with Oracle 10g.

NB: I'm reopening this ticket instead of creating a new one because it's a follow-up on the same issue and all the history is on this page.

Changed 3 years ago by aaugustin

comment:6 Changed 3 years ago by mtredinnick

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

In [16686]:

Fix test from r16678 and r16681 properly for Oracle.

Fixes #16694, with thanks to aagustin for the Oracle testing and
tweaking.

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.