Opened 9 months ago

Closed 9 months ago

#35226 closed Bug (fixed)

Dynamically created connection are disallowed in SimpleTestCase and subclasses.

Reported by: Florian Apolloner Owned by: Mariusz Felisiak
Component: Testing framework Version: dev
Severity: Release blocker Keywords:
Cc: Florian Apolloner, David Wobrock Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Mariusz Felisiak)

While working on connection pooling for postgres I ran into the following regression (since 8fb0be3500cc7519a56985b1b6f415d75ac6fedb):

from django.db import connection
from django.test import TestCase


class Test(TestCase):
    def test_something(self):
        new_connection = connection.copy("asd")
        with new_connection.cursor() as cursor:
            print(cursor.execute("SELECT 1").fetchall())

On 5.0 this works fine, on main this fails with:

django.test.testcases.DatabaseOperationForbidden: Database threaded connections to 'asd' are not allowed in this test. Add 'asd' to test_regression.Test.databases to ensure proper test isolation and silence this failure.

I cannot really add this to databases since this is a dynamically created database. I am opening this as release blocker so it doesn't get lost (I might be holding it wrong though)

Change History (7)

comment:1 by Mariusz Felisiak, 9 months ago

Description: modified (diff)

TBH, I don't see how we could handle this and keep 8fb0be3500cc7519a56985b1b6f415d75ac6fedb. The main question is, do we want to support something like this? Creating a copy of the connection copy is tricky, and if you change the alias in the meantime it becomes even more complicated.

comment:2 by Florian Apolloner, 9 months ago

Well I will see if I can get around it without an alias change in the pool tests, but testing the pooling stuff is tricky without creating completely new connections :D

EDIT:// I was able to work around it for my tests, we will see if the full testsuite on jenkins agrees with my changes.

Last edited 9 months ago by Florian Apolloner (previous) (diff)

comment:3 by Mariusz Felisiak, 9 months ago

We could ignore dynamically created connections, e.g.:

  • django/test/testcases.py

    diff --git a/django/test/testcases.py b/django/test/testcases.py
    index 51b07ae50d..768ebd7d52 100644
    a b class SimpleTestCase(unittest.TestCase):  
    280280                self.connection is None
    281281                and self.alias not in cls.databases
    282282                and self.alias != NO_DB_ALIAS
     283                and self.alias in connections
    283284            ):
    284285                # Connection has not yet been established, but the alias is not allowed.
    285286                message = cls._disallowed_database_msg % {

What do you think?

comment:4 by Florian Apolloner, 9 months ago

That seems like a workable solution to me if it doesn't conflict with what you have been trying to achieve in the first place 👍

in reply to:  4 comment:5 by Mariusz Felisiak, 9 months ago

Cc: David Wobrock added
Owner: changed from nobody to Mariusz Felisiak
Status: newassigned
Summary: Copying a connection with a new alias failsDynamically created connection are disallowed in SimpleTestCase and subclasses.
Triage Stage: UnreviewedAccepted
Version: 5.0dev

Replying to Florian Apolloner:

That seems like a workable solution to me if it doesn't conflict with what you have been trying to achieve in the first place 👍

Thanks, let's try.

comment:6 by Mariusz Felisiak, 9 months ago

Has patch: set

comment:7 by GitHub <noreply@…>, 9 months ago

Resolution: fixed
Status: assignedclosed

In 5f637a8a:

Fixed #35226 -- Reallowed executing queries for dynamically created connections.

Regression in 8fb0be3500cc7519a56985b1b6f415d75ac6fedb.

Thanks Florian Apolloner for the report.

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