classification
Title: [sqlite3] refactor threading tests
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.11
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: erlendaasland Nosy List: erlendaasland, pablogsal
Priority: low Keywords: patch

Created on 2021-06-15 23:22 by erlendaasland, last changed 2021-06-20 19:26 by pablogsal. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 26748 merged erlendaasland, 2021-06-15 23:23
Messages (3)
msg395901 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-06-15 23:22
The threading tests (ThreadTests) in Lib/sqlite3/test/dbapi.py has a lot of code duplication, and none of the test methods use the test.support.threading_helper.reap_threads helper. Also, some branches are not covered by this test class. Suggesting the following:

1. Rewrite ThreadTests with a _run_test() helper method that does the heavy lifting
2. Add test.support.threading_helper.reap_threads to _run_test()
3. Use _run_test() in all threading tests
4. Add test case for sqlite3.Connection.set_trace_callback
5. Add test case for sqlite3.Connection.create_collation
msg395943 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-06-16 15:54
The current code duplication has several negative impacts, IMO:

- Readability
  Currently the ThreadTests class spans 173 lines. Refactored, it spans 51 lines (should fit in a screenful), making it easy to see what's actually being tested.
  Currently, the tests differ by only one line; the remaining 15 lines are boilerplate code. That's 15 lines of boilerplate code per test method.

- Maintainability
  Currently, if the boilerplate code needs tuning (or bugfixing), we must adjust it in _all_ the test methods. For example, adding the @threading_helper.reap_threads decorator to every single test. After refactoring, the boilerplate code is refactored out; adding the reap threads decorator needs only be done in one place; modifying the boilerplate code needs only be done in one place.

- Fewer lines of code; improved maintainability
  diff stat:
    1 file changed, 37 insertions(+), 140 deletions(-)
msg396190 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-06-20 19:26
New changeset 5f0fc30de46d41dccf04096df12664fc0a684ca2 by Erlend Egeberg Aasland in branch 'main':
bpo-44430: Refactor `sqlite3` threading tests (GH-26748)
https://github.com/python/cpython/commit/5f0fc30de46d41dccf04096df12664fc0a684ca2
History
Date User Action Args
2021-06-20 19:26:46pablogsalsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-06-20 19:26:43pablogsalsetmessages: + msg396190
2021-06-16 15:54:29erlendaaslandsetmessages: + msg395943
2021-06-15 23:23:42erlendaaslandsetkeywords: + patch
stage: patch review
pull_requests: + pull_request25333
2021-06-15 23:22:29erlendaaslandcreate