Skip to content

gh-108617: Extend interactive session tests for sqlite3 #108556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Aug 28, 2023

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Aug 28, 2023

Thanks, this is very nice. Are you ok with landing #108551 first? Either way, one of the PRs will have to resolve conflicts.

@serhiy-storchaka
Copy link
Member Author

If it is compatible with #108551. But since it only calls communicate() once per session, there should not be issues.

@serhiy-storchaka
Copy link
Member Author

On other hand, if the CLI outputs some farewell message, there will be conflicts. Also, #108551 does not test that all output is not hold until the program ends. In case of some issues I will need to partially revert your changes and simulate a subprocess with a thread.

@erlend-aasland
Copy link
Contributor

On other hand, if the CLI outputs some farewell message, there will be conflicts. Also, #108551 does not test that all output is not hold until the program ends. In case of some issues I will need to partially revert your changes and simulate a subprocess with a thread.

OTOH, we're simply testing the underlying code.InteractiveConsole here; perhaps it would be better to add tests to test_code instead of implicitly testing that it works though the sqlite3 CLI.

@erlend-aasland
Copy link
Contributor

Good use of the count() sequence API. It is a good improvement.

@erlend-aasland erlend-aasland changed the title Extend interactive session tests for sqlite3 gh-108617: Extend interactive session tests for sqlite3 Aug 29, 2023
@serhiy-storchaka serhiy-storchaka merged commit ecb2bf0 into python:main Aug 29, 2023
@serhiy-storchaka serhiy-storchaka deleted the sqlite3-test-interactive branch August 29, 2023 10:20
@bedevere-bot
Copy link

There's a new commit after the PR has been approved.

@erlend-aasland: please review the changes made to this pull request.

@erlend-aasland erlend-aasland added the needs backport to 3.12 only security fixes label Aug 29, 2023
@miss-islington
Copy link
Contributor

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-108626 is a backport of this pull request to the 3.12 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 29, 2023
@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Aug 29, 2023
Yhg1s pushed a commit that referenced this pull request Aug 29, 2023
…8556) (#108626)

gh-108617: Extend interactive session tests for sqlite3 (GH-108556)
(cherry picked from commit ecb2bf0)

Co-authored-by: Serhiy Storchaka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants