Opened 13 years ago

Closed 2 years ago

Last modified 2 years ago

#15691 closed Bug (worksforme)

TEST_DEPENDENCIES doesn't use TEST_NAME in circular dependency detection.

Reported by: slinkp@… Owned by: nobody
Component: Testing framework Version: 1.2
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As far as I can tell, TEST_DEPENDENCIES checks for circular dependencies by looking at the NAME key, ignoring TEST_NAME if it exists.

This means that if you have some aliases that use the same NAME but different TEST_NAMEs, it will bail out reporting a circular dependency even though there isn't one. Example:

DATABASES = {
    'default': {
        'NAME': 'mydb',
         ...
        'TEST_NAME': 'test_mydb',
        'TEST_DEPENDENCIES': ['users'],
    },
    'users': {
        'NAME': 'mydb',
        ...
        'TEST_NAME': 'test_users',
        'TEST_DEPENDENCIES': [],
    },

It's reasonable to ask why you'd have multiple database configs with the same NAME. On OpenBlock we've been doing that as our default setup, because most of our dev rigs and deployments use only one physical database, but when running tests we want to be sure that multi-database setups work.

Attachments (2)

15691.diff (969 bytes ) - added by Jason Peddle 13 years ago.
TEST_NAME, fallback to NAME
issue15691.zip (18.0 KB ) - added by Carlton Gibson 2 years ago.
Test project trying to reproduce circa 2022.

Download all attachments as: .zip

Change History (11)

comment:1 by slinkp@…, 13 years ago

fwiw, our config hasn't caused any problems to date on Django 1.2.3. I noticed this problem because tests started failing if we tried upgrading to 1.2.4 or 1.2.5, and I traced it to the addition of TEST_DEPENDENCIES.

comment:2 by Luke Plant, 13 years ago

Type: Bug

comment:3 by Luke Plant, 13 years ago

Severity: Normal

comment:4 by Jacob, 13 years ago

milestone: 1.4
Triage Stage: UnreviewedAccepted

comment:5 by anonymous, 13 years ago

Easy pickings: unset
Has patch: set
UI/UX: unset

test_db_signature in db/backends/[oracle/]creation.py was not using TEST_NAME. Diff attached that gives precedence to TEST_NAME, then NAME

by Jason Peddle, 13 years ago

Attachment: 15691.diff added

TEST_NAME, fallback to NAME

in reply to:  5 comment:6 by Jason Peddle, 13 years ago

Replying to anonymous:

test_db_signature in db/backends/[oracle/]creation.py was not using TEST_NAME. Diff attached that gives precedence to TEST_NAME, then NAME

Forgot to sign my name.

comment:7 by Jacob, 12 years ago

milestone: 1.4

Milestone 1.4 deleted

comment:8 by Tim Graham, 11 years ago

Needs tests: set

This needs a test.

comment:9 by Carlton Gibson, 2 years ago

Resolution: worksforme
Status: newclosed

This doesn't reproduce for me on latest main.

These settings:

DATABASES = {
    'default': {
        'ENGINE': 'django.db.backends.sqlite3',
        'NAME': BASE_DIR / 'db.sqlite3',
        'TEST': {
            'NAME': BASE_DIR / 'test_mydb',
            'DEPENDENCIES': ['users'],
        },
    },
    'users': {
        'ENGINE': 'django.db.backends.sqlite3',
        'NAME': BASE_DIR / 'db.sqlite3',
        'TEST': {
            'NAME': BASE_DIR / 'test_users',
            'DEPENDENCIES': [],
        },
    },
}

This model:

from django.db import models

class DefaultModel(models.Model):
    name = models.TextField()

This test:

from django.test import TestCase

from .models import DefaultModel

class TheTests(TestCase):
    databases = '__all__'

    def test_basics(self):
        o = DefaultModel(name="Default")
        o.save()

        o = DefaultModel(name="Users")
        o.save(using="users")

        d = DefaultModel.objects.get()
        u = DefaultModel.objects.using('users').get()

        self.assertEqual(d, u)
        self.assertNotEqual(d.name, u.name)

All runs without error.

I'll upload the project zip in case someone wants to play with it.

...checks for circular dependencies by looking at the NAME key, ignoring TEST_NAME if it exists...

This doesn't look right, at least how the code is now. The check compares aliases, not NAME or TEST['NAME'].

I'll close as worksforme but happy to re-open if someone can provide a reproduce.

Last edited 2 years ago by Carlton Gibson (previous) (diff)

by Carlton Gibson, 2 years ago

Attachment: issue15691.zip added

Test project trying to reproduce circa 2022.

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