gh-108550: Speed up sqlite3 tests#108551
Conversation
Refactor the CLI so we can easily invoke it and mock command-line arguments. Adapt the CLI tests so we no longer have to launch a separate process. Disable the busy handler for all concurrency tests; we have full control over the order of the SQLite C API calls, so we can safely do this. The sqlite3 test suite now completes ~10 times faster than before.
|
I ran this some hundred times locally: $ ./python.exe -m test test_sqlite3 -F
Raised RLIMIT_NOFILE: 256 -> 1024
0:00:00 load avg: 4.23 Run tests sequentially
0:00:00 load avg: 4.23 [ 1] test_sqlite3
Warning -- files was modified by test_sqlite3
Warning -- Before: []
Warning -- After: ['default.profraw']
Warning -- files was modified by test_sqlite3
Warning -- Before: []
Warning -- After: ['default.profraw']
0:00:00 load avg: 4.23 [ 2/1] test_sqlite3 -- test_sqlite3 failed (env changed)
[...]
0:03:25 load avg: 5.57 [694/1] test_sqlite3(env changes is because I have profiling enabled by default on my local build) |
|
@AlexWaygood, want to have a quick look at the |
|
cc. @serhiy-storchaka if you want to have a look |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Nice.
But it was possible to rewrite InteractiveSession tests in such way because you do not test interactivity. For example, that the prompt is displayed before any input. That the continuation line prompt is displayed before finishing the multiline command. In future we can build readline support in the REPL and add tests for history or completion.
Is it possible to support multiple communicate()?
|
Yeah, InteractiveSession had flaws, and it is suboptimal currently. I thought realine support was baked into the interactive console class from |
It should be possible, but I think that is out of scope for this PR. |
|
On other hand, it is not necessary. There is already a test for initial prompt. We can just add a new test for incomplete multiline input and check secondary prompts. See #108556. |
We already do, no? |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
There are many ways to break the interactive session, but keep the changed tests passed:
- Read input not line by line, by the whole content. It delays any interaction until stdin be closed.
- Buffer stdout, or stderr, or both until the program ends.
So I am not sure about this change. Maybe simulating a subprocess with a thread can mitigate the problem, but we need an analogue of blocking PipeInputStream for this.
There are no such problems with non-interactive tests.
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
This PR seems to risk running stale. I'm opening a more narrowed PR for the busy timeout optimisation instead. |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
I approve this change, but I afraid that we will need to revert some of them in future.
Yes, I also believe we need to further rework the interactive tests. I'm not even sure we need all of them. |
|
Thanks for the review, Serhiy. |
This comment was marked as outdated.
This comment was marked as outdated.
|
Sorry, @erlend-aasland, I could not cleanly backport this to |
Refactor the CLI so we can easily invoke it and mock command-line arguments. Adapt the CLI tests so we no longer have to launch a separate process. Disable the busy handler for all concurrency tests; we have full control over the order of the SQLite C API calls, so we can safely do this. The sqlite3 test suite now completes ~8 times faster than before. (cherry picked from commit 0e8b3fc) Co-authored-by: Erlend E. Aasland <erlend@python.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
GH-108566 is a backport of this pull request to the 3.12 branch. |
Disable the busy handler for all concurrency tests; we have full control over the order of the SQLite C API calls, so we can safely do this. test_sqlite3.test_transactions now completes ~10 times faster than before. Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Disable the busy handler for all concurrency tests; we have full control over the order of the SQLite C API calls, so we can safely do this. test_sqlite3.test_transactions now completes ~10 times faster than before. Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
GH-108567 is a backport of this pull request to the 3.11 branch. |
AlexWaygood
left a comment
There was a problem hiding this comment.
Post-merge review: LGTM too (though I'm no expert on testing interactive REPLs)!
Execution time for me on Windows went down from 3.8s to 548ms :D
Yay! 😃 |
Disable the busy handler for all concurrency tests; we have full control over the order of the SQLite C API calls, so we can safely do this. test_sqlite3.test_transactions now completes ~10 times faster than before. Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
gh-108550: Speed up sqlite3 tests (GH-108551) Refactor the CLI so we can easily invoke it and mock command-line arguments. Adapt the CLI tests so we no longer have to launch a separate process. Disable the busy handler for all concurrency tests; we have full control over the order of the SQLite C API calls, so we can safely do this. The sqlite3 test suite now completes ~8 times faster than before. (cherry picked from commit 0e8b3fc) Co-authored-by: Erlend E. Aasland <erlend@python.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
…honGH-108618) (cherry picked from commit c884784) Co-authored-by: Erlend E. Aasland <erlend@python.org>
Refactor the CLI so we can easily invoke it and mock command-line
arguments. Adapt the CLI tests so we no longer have to launch a
separate process.
Disable the busy handler for all concurrency tests; we have full
control over the order of the SQLite C API calls, so we can safely
do this.
The sqlite3 test suite now completes ~8 times faster than before.