Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#13362 closed (fixed)

bug8245 test fails on 1.1.X/Python2.3 after r12957

Reported by: kmtracey Owned by: brosner
Component: contrib.admin Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


The fix for #11957 doesn't appear to work on Python2.3:

test_bug_8245 (regressiontests.bug8245.tests.Bug8245Test) ... FAIL

FAIL: test_bug_8245 (regressiontests.bug8245.tests.Bug8245Test)
Traceback (most recent call last):
  File "C:\u\kmt\django\branch1.1.X\tests\regressiontests\bug8245\", line 29, in test_bug_8245
  File "C:\bin\Python23\lib\", line 270, in fail
    raise self.failureException, msg
AssertionError: autodiscover should have raised a "Bad admin module" error.

Ran 1 test in 0.094s

FAILED (failures=1)

This test app has a broken admin module: attempting to import it raises an exception. Thus admin.autodiscover() raises an exception. Prior to r12957, the autodiscover code prevented a 2nd attempt to load admin modules if a first attempt failed to complete successfully, so the expected behavior was that a 2nd attempt to run autodiscover() would not raise an exception even though the first one did. r12957 changed things so that the loads will be re-attempted, and now the expected behavior is that the 2 successive calls to autodiscover() will raise the same exception.

However, on Python 2.3, the 2nd call to __import__ the broken admin module doesn't, in fact, raise an exception. This looks to me like a Python problem and my initial inclination is therefore to just skip this test if we are running on Python 2.3. (Perhaps there is some way to force Python2.3 to raise the exception again, but I'm not inclined to spend the investigation time required to figure that out for sure one way or the other.)

I'd be interested in others' opinions on how to deal with this.

Change History (7)

comment:1 Changed 5 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 5 years ago by brosner

  • Owner changed from nobody to brosner
  • Status changed from new to assigned

It isn't immediately obvious to me either. I'll take a look into this and find the best solution today (which may be to do what Karen suggests).

comment:3 Changed 5 years ago by aaugustin

This is actually documented in Python 2.4 release notes. Here is the relevant excerpt of

When importing a module M raises an exception, Python no longer leaves M in sys.modules. Before 2.4a2 it did, and a subsequent import of M would succeed, picking up a module object from sys.modules reflecting as much of the initialization of M as completed before the exception was raised. Subsequent imports got no indication that M was in a partially- initialized state, and the importers could get into arbitrarily bad trouble as a result (the M they got was in an unintended state, arbitrarily far removed from M's author's intent). Now subsequent imports of M will continue raising exceptions (but if, for example, the source code for M is edited between import attempts, then perhaps later attempts will succeed, or raise a different exception).

So this is definitely a bug in Python 2.3. I do not know Django's policy in such cases.

comment:4 Changed 5 years ago by brosner

Considering the nature of the fix for #11957 was to improve error handling I am okay with this functionality being broken on 2.3. I appreciate your bit of investigation aaugustin. I'll commit a fix to the test to ignore 2.3.

comment:5 Changed 5 years ago by russellm

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

(In [13021]) Fixed #13362 -- Disabled the test for bug #8245 under Python 2.3 due to differences in exception handling.

comment:6 Changed 5 years ago by russellm

(In [13022]) [1.1.X] Fixed #13362 -- Disabled the test for bug #8245 under Python 2.3 due to differences in exception handling.

Backport of r13021 from trunk.

comment:7 Changed 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top