[Django] #27079: LiveServerPort.test_port_bind() sets allow_thread_sharing to False before all server threads are done
Django
all server threads are done
-------------------------------+--------------------
Reporter: cjerdonek | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
Currently, `LiveServerPort.test_port_bind()` calls
`LiveServerTestCase.setUpClass()` and `LiveServerTestCase.tearDownClass()`
''twice'' (to create two live server threads -- see
[https://github.com/django/django/blob/e76981b43325da60b8a7475661df6cbfa7fda37e/tests/servers/tests.py#L133
here] for the code). On the tear-down side, this has the unwanted side
effect of setting `conn.allow_thread_sharing` to False before all live
server threads are done with their connections.
The test should really only be calling the full `setUpClass()` and
`tearDownClass()` once (but still doing the partial setup and tear-down
that it needs for the purposes of its test).
--
Ticket URL: <https://code.djangoproject.com/ticket/27079>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
all server threads are done
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* component: Uncategorized => Testing framework
* needs_tests: => 0
* needs_docs: => 0
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:1>
Django
all server threads are done
Reporter: cjerdonek | Owner: cjerdonek
Type: Bug | Status: assigned
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by cjerdonek):
* owner: nobody => cjerdonek
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:2>
Django
-----------------------------------+-------------------------------------
Reporter: cjerdonek | Owner: cjerdonek
Type: Bug | Status: assigned
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by cjerdonek):
[https://github.com/django/django/pull/7096 This] merged pull request
removed calling `tearDownClass()` twice (because it blocked the resolution
of that issue), so I'm retitling this ticket to reflect the updated
situation.
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:3>
Django
-----------------------------------+------------------------------------
Type: Bug | Status: new
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* owner: Chris Jerdonek => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:4>
Django
-----------------------------------+------------------------------------
Reporter: Chris Jerdonek | Owner: (none)
Type: Bug | Status: new
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jacob Walls):
Replying to [comment:3 Chris Jerdonek]:
> [https://github.com/django/django/pull/7096 This] merged pull request
removed calling `tearDownClass()` twice (because it blocked the resolution
of that issue), so I'm retitling this ticket to reflect the updated
situation.
Chris, is it the case that the changes you referred to were undone in
8c775391b78b2a4a2b57c5e89ed4888f36aada4b? Perhaps we could revert to the
original issue title (and include the test that follows it)?
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:5>
Django
-----------------------------------+------------------------------------
Reporter: Chris Jerdonek | Owner: (none)
Type: Bug | Status: new
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Chris Jerdonek):
> Chris, is it the case that the changes you referred to were undone in
8c775391b78b2a4a2b57c5e89ed4888f36aada4b? Perhaps we could revert to the
original issue title (and include the test that follows it)?
Nice observation, Jacob. More specifically, it was done
[https://github.com/django/django/commit/8c775391b78b2a4a2b57c5e89ed4888f36aada4b
#diff-
c0d7a27759b0649539ed852dde69da3527fc718af1806dc28b93697d3dd0ceb4L212-R212
in these lines] and below.
Do you want to go ahead with what you suggest? It could also help to
comment on that PR so the connection between the tickets is more
noticeable.
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:6>
Django
server threads are done
-----------------------------------+------------------------------------
Reporter: Chris Jerdonek | Owner: (none)
Type: Bug | Status: new
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jacob Walls):
Back to the original state of play after changes in #28478. My
understanding is that we should tailor the setup and cleanup in these two
tests rather than make extra calls to the class methods.
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:7>
Django
tearDownClass()
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* owner: (none) => Jacob Walls
* status: new => assigned
* type: Bug => Cleanup/optimization
Comment:
> Back to the original state of play after changes in #28478.
Actually, not quite the original state of play. `allow_thread_sharing` was
removed in #30171 in favor of a counter pattern which obviates the
reported bug here. I still think Chris's suggestion has value, but it's
more of a cleanup opportunity, if I'm not mistaken.
Having touched this a couple times I ought to just finish it off.
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:8>
Django
tearDownClass()
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/15007 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:9>
Django
tearDownClass()
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:10>
Django
tearDownClass()
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:11>
Django
tearDownClass()
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:12>
Django
tearDownClass()
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Jacob
Type: | Walls
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"def09bf4126d4886413adf7388882eca8e32576b" def09bf]:
{{{
#!CommitTicketReference repository=""
revision="def09bf4126d4886413adf7388882eca8e32576b"
Fixed #27079 -- Avoided multiple setUpClass()/tearDownClass() calls in
LiveServerTestCase tests.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:13>
Django
tearDownClass()
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: closed
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"534895f1ace819053f28463fd62b833ba2ced829" 534895f]:
{{{
#!CommitTicketReference repository=""
revision="534895f1ace819053f28463fd62b833ba2ced829"
[4.0.x] Fixed thread termination in servers.tests.LiveServerPort on Python
< 3.10.9.
TestCase.doClassCleanups() cannot be called on Python < 3.10.9 because
setUpClass()/tearDownClass() are called multiple times in
LiveServerTestCase tests (refs #27079).
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:14>
Django
tearDownClass()
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: closed
Component: Testing framework | Version: 1.10
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by GitHub <noreply@…>):
In [changeset:"d6e9ec40145b9edc65e8d0c65bd3f4ef8a7a27bb" d6e9ec4]:
{{{
#!CommitTicketReference repository=""
revision="d6e9ec40145b9edc65e8d0c65bd3f4ef8a7a27bb"
Refs #27079 -- Used addClassCleanup() in SeleniumTestCase.
Regression in def09bf4126d4886413adf7388882eca8e32576b.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27079#comment:15>