Opened 7 years ago

Closed 7 years ago

Last modified 4 years ago

#8835 closed (duplicate)

Tests fail with r8908

Reported by: jarrow Owned by: telenieko
Component: Testing framework 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:

Description

Tested with r8908 on Debian Etch, Sqlite3 and Python 2.4.4

======================================================================
FAIL: Doctest: regressiontests.extra_regress.models.__test__.API_TESTS
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.4/site-packages/django/test/_doctest.py", line 2180, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for regressiontests.extra_regress.models.__test__.API_TESTS
  File "/srv/lib/django-trunk/tests/regressiontests/extra_regress/models.py", line unknown line number, in API_TESTS

----------------------------------------------------------------------
File "/srv/lib/django-trunk/tests/regressiontests/extra_regress/models.py", line ?, in regressiontests.extra_regress.models.__test__.API_TESTS
Failed example:
    User.objects.extra(select={'extra_field': 1}).distinct()
Expected:
    [<User: fred>]
Got:
    [<User: normaluser>, <User: fred>]
----------------------------------------------------------------------
File "/srv/lib/django-trunk/tests/regressiontests/extra_regress/models.py", line ?, in regressiontests.extra_regress.models.__test__.API_TESTS
Failed example:
    User.objects.extra(select={'extra_field': 1}, order_by=['extra_field'])
Expected:
    [<User: fred>]
Got:
    [<User: normaluser>, <User: fred>]
----------------------------------------------------------------------
File "/srv/lib/django-trunk/tests/regressiontests/extra_regress/models.py", line ?, in regressiontests.extra_regress.models.__test__.API_TESTS
Failed example:
    User.objects.extra(select={'extra_field': 1}, order_by=['extra_field']).distinct()
Expected:
    [<User: fred>]
Got:
    [<User: normaluser>, <User: fred>]


----------------------------------------------------------------------
Ran 525 tests in 512.993s

FAILED (failures=1)

Change History (10)

comment:1 Changed 7 years ago by julien

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

That's probably to do with [8898]

comment:2 Changed 7 years ago by telenieko

It seems that was introduced by comments refactor (normaluser is created in it's tests). And the issue is not the test failling but the DB not being rolledback after the comments tests. Hence "normaluser" is still there, which shouldn't.

I'm pretty sure something like that happened before (remember SITE's cache?)

comment:3 Changed 7 years ago by telenieko

Malcolm, git points at you! ;)

marc@castor ~/dev/django % git bisect start HEAD 45df44cb56b0754fd2da763e9d431907cedc54c9
...
marc@castor ~/dev/django % git bisect run test-django 0 comment_tests extra_regress
...
04dbfae1df3a4fb989d69d45311dfc735e47ce6c is first bad commit
commit 04dbfae1df3a4fb989d69d45311dfc735e47ce6c
Author: mtredinnick <mtredinnick@bcc190cf-cafb-0310-a4f2-bffc1f526a37>
Date:   Wed Sep 3 03:48:25 2008 +0000

    Fixed #8819 -- Don't include two copies of extra-select columns in the query.
    This was triggered by r8794, but was, in fact, fairly fragile before then. The
    current fix is the correct way we should be doing this.
    
    
    git-svn-id: http://code.djangoproject.com/svn/django/trunk@8898 bcc190cf-cafb-0310-a4f2-bffc1f526a37

:040000 040000 07388d332878771536191f88cea36c29af5fe66f 0b8e5975c505e41d52d6eaaf6ebe96575d3713b8 M	django
:040000 040000 6a54c2f494b634be75cf4b24a8adab04db80c4ef d80c671e345477c04c36c634ff17ef7c30c95df8 M	tests
bisect run success
git bisect run test-django 0 comment_tests extra_regress  55,11s user 2,48s system 90% cpu 1:03,98 total

comment:4 Changed 7 years ago by telenieko

Forget my last comment...

comment:5 Changed 7 years ago by telenieko

  • Component changed from Uncategorized to Unit test system
  • Triage Stage changed from Unreviewed to Accepted

comment:6 Changed 7 years ago by telenieko

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

comment:7 Changed 7 years ago by telenieko

At this moment DocTests are left with the DB status previous to their call, meaning that if the previous test loaded a fixture or altered some data; The DocTest will have this data.

Unit Tests flush the database before being run. DocTests should do the same.
Adding a call to flush in DocTestRunner() (django/test/testcases.py:163) "call_command('flush', verbosity=0, interactive=False)" should be enough.

That way every app gets a fresh environment for doctests. (And previous apps cannot "leave" things in the database, which is what caused this ticket).

If not, "flush" should be called *atleast* in TestCase._post_teardown() (same file, L225). Loading a fixture in a TestCase should not affect the upcoming tests. It's done in TestCases as "flush" is called on _pre_setup() but if a DoctTest follows a TestCase...

So atleast DocTestRunner.init() or TestCase._post_teardown() should flush the database (It'd be better on init() to make sure all doctests have a clean DB).

diff --git a/django/test/testcases.py b/django/test/testcases.py
index dcab078..813a211 100644
--- a/django/test/testcases.py
+++ b/django/test/testcases.py
@@ -159,6 +159,7 @@ class OutputChecker(doctest.OutputChecker):
 
 class DocTestRunner(doctest.DocTestRunner):
     def __init__(self, *args, **kwargs):
+        call_command('flush', verbosity=0, interactive=False)
         doctest.DocTestRunner.__init__(self, *args, **kwargs)
         self.optionflags = doctest.ELLIPSIS

comment:8 Changed 7 years ago by jacob

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

Ah, nice job tracking this down, teleniko. This means that this is basically a duplicate of #5624; that'd make sure that doctests are properly flushed. For now I think we'll just have to deal with it.

comment:9 Changed 7 years ago by mtredinnick

(In [8930]) Changed the tests added in r8898 very slightly so that they don't return
unintended results. Doctests aren't self-contained yet when run as part of
the full suite. Refs #5624, #8835.

comment:9 Changed 4 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

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