[Django] #32445: LiveServerThreadTest.test_closes_connections() doesn't pass with non-in-memory SQLite

Django

unread,

Feb 14, 2021, 5:38:42 PM2/14/21

to django-...@googlegroups.com

#32445: LiveServerThreadTest.test_closes_connections() doesn't pass with non-in-
memory SQLite
-------------------------------------+-------------------------------------
Reporter: Chris | Owner: nobody
Jerdonek |
Type: | Status: new
Uncategorized |
Component: Testing | Version: 3.1
framework | Keywords:
Severity: Normal | SQLite,is_usable,LiveServerThread
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I noticed that `LiveServerThreadTest.test_closes_connections()` doesn't
seem to pass when using non-in-memory SQLite. It's only configured to be
skipped
[https://github.com/django/django/blob/3fa1ed53be370f4b1a94d78b56ff30d23b131623/django/db/backends/sqlite3/features.py#L75-L81
for in-memory SQLite]. It doesn't pass because the test has the
[https://github.com/django/django/blob/3fa1ed53be370f4b1a94d78b56ff30d23b131623/tests/servers/test_liveserverthread.py#L24
following line]:

{{{#!python
self.assertFalse(conn.is_usable())
}}}

But SQLite's `is_usable()` has a hard-coded return value of `True`:
https://github.com/django/django/blob/3fa1ed53be370f4b1a94d78b56ff30d23b131623/django/db/backends/sqlite3/base.py#L387-L388

I guess this means that Django's CI doesn't check non-in-memory SQLite. If
it's true that Django doesn't check this case, I'm not sure if that means
it doesn't need to be fixed.

--
Ticket URL: <https://code.djangoproject.com/ticket/32445>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Feb 15, 2021, 1:17:27 AM2/15/21

to django-...@googlegroups.com

#32445: LiveServerThreadTest.test_closes_connections() doesn't pass with non-in-
memory SQLite
-------------------------------------+-------------------------------------

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted

Comment:

> I guess this means that Django's CI doesn't check non-in-memory SQLite.

That's True, but I think it's still worth fixing. There is an interesting
[https://github.com/django/django/pull/7096#discussion_r74955788
discussion] about this test in the original PR. I think we should remove
`test_closes_connections()` from `django_test_skips()` and use
`@skipUnlessDBFeature('test_db_allows_multiple_connections')` instead.

--
Ticket URL: <https://code.djangoproject.com/ticket/32445#comment:1>

Django

unread,

Feb 15, 2021, 1:33:12 AM2/15/21

to django-...@googlegroups.com

#32445: LiveServerThreadTest.test_closes_connections() doesn't pass with non-in-
memory SQLite
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Chris Jerdonek):

Just to be totally clear, would
`@skipUnlessDBFeature('test_db_allows_multiple_connections')` mean that
the test is or isn't skipped with non-in-memory SQLite?

--
Ticket URL: <https://code.djangoproject.com/ticket/32445#comment:2>

Django

unread,

Feb 15, 2021, 1:41:46 AM2/15/21

to django-...@googlegroups.com

#32445: LiveServerThreadTest.test_closes_connections() doesn't pass with non-in-
memory SQLite
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

Replying to [comment:2 Chris Jerdonek]:

> Just to be totally clear, would
`@skipUnlessDBFeature('test_db_allows_multiple_connections')` mean that
the test is or isn't skipped with non-in-memory SQLite?

It will always be skipped on SQLite, so also with non-in-memory database.

--
Ticket URL: <https://code.djangoproject.com/ticket/32445#comment:3>

Django

unread,

Feb 15, 2021, 1:52:31 AM2/15/21

to django-...@googlegroups.com

#32445: LiveServerThreadTest.test_closes_connections() doesn't pass with non-in-
memory SQLite
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Chris Jerdonek):

> It will always be skipped on SQLite, so also with non-in-memory
database.

Okay, that's what I thought. Why do you think the test should always be
skipped on SQLite as opposed to making the test work also for non-in-
memory SQLite? It seems like making it work would be the better outcome
since it would cover more scenarios where the test makes sense.

--
Ticket URL: <https://code.djangoproject.com/ticket/32445#comment:4>

Django

unread,

Feb 15, 2021, 2:08:18 AM2/15/21

to django-...@googlegroups.com

#32445: LiveServerThreadTest.test_closes_connections() doesn't pass with non-in-
memory SQLite
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

Replying to [comment:4 Chris Jerdonek]:

> Okay, that's what I thought. Why do you think the test should always be
skipped on SQLite as opposed to making the test work also for non-in-
memory SQLite? It seems like making it work would be the better outcome
since it would cover more scenarios where the test makes sense.

Agreed, making the test work would be better, but I'm sure how to do this.
I would be happy to review a patch, if you have an idea.

--
Ticket URL: <https://code.djangoproject.com/ticket/32445#comment:5>

Django

unread,

Feb 15, 2021, 2:21:00 AM2/15/21

to django-...@googlegroups.com

#32445: LiveServerThreadTest.test_closes_connections() doesn't pass with non-in-
memory SQLite
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Chris Jerdonek):

Agreed, thank you. I have some ideas, but not 100% sure it will work out.

--
Ticket URL: <https://code.djangoproject.com/ticket/32445#comment:6>

Django

unread,

Feb 20, 2021, 11:35:18 PM2/20/21

to django-...@googlegroups.com

#32445: LiveServerThreadTest.test_closes_connections() doesn't pass with non-in-
memory SQLite
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Chris Jerdonek):

Okay, I came up with a solution for getting the test to work with non-in-
memory SQLite. The main idea is to use `TransactionTestCase` instead of
`TestCase`.

--
Ticket URL: <https://code.djangoproject.com/ticket/32445#comment:7>

Django

unread,

Feb 20, 2021, 11:35:38 PM2/20/21

to django-...@googlegroups.com

#32445: LiveServerThreadTest.test_closes_connections() doesn't pass with non-in-
memory SQLite
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Chris Jerdonek):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/32445#comment:8>

Django

unread,

Feb 22, 2021, 6:38:15 AM2/22/21

to django-...@googlegroups.com

#32445: LiveServerThreadTest.test_closes_connections() doesn't pass with non-in-
memory SQLite
-------------------------------------+-------------------------------------

Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned

Component: Testing framework | Version: 3.1
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
SQLite,is_usable,LiveServerThread | checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* owner: nobody => Chris Jerdonek
* status: new => assigned
* stage: Accepted => Ready for checkin

Comment:

[https://github.com/django/django/pull/14012 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/32445#comment:9>

Django

unread,

Feb 23, 2021, 12:35:25 AM2/23/21

to django-...@googlegroups.com

#32445: LiveServerThreadTest.test_closes_connections() doesn't pass with non-in-
memory SQLite
-------------------------------------+-------------------------------------

Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek

Cleanup/optimization | Status: closed

Component: Testing framework | Version: 3.1

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
SQLite,is_usable,LiveServerThread | checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"5c4c3e2d1f232df6de24741ca1df32442a01ebc3" 5c4c3e2d]:
{{{
#!CommitTicketReference repository=""
revision="5c4c3e2d1f232df6de24741ca1df32442a01ebc3"
Fixed #32445 -- Fixed LiveServerThreadTest.test_closes_connections() for
non-in-memory database on SQLite.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/32445#comment:10>

Reply all

Reply to author

Forward