Skip to content

Add explicit test for sqlite3.Row.keys() #108558

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.

Sign up for GitHub

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

Jump to bottom
Closed
serhiy-storchaka opened this issue Aug 28, 2023 · 9 comments · Fixed by #108578
Closed

Add explicit test for sqlite3.Row.keys() #108558

serhiy-storchaka opened this issue Aug 28, 2023 · 9 comments · Fixed by #108578
Labels
3.11 only security fixes 3.12 only security fixes 3.13 bugs and security fixes easy tests Tests in the Lib/test dir topic-sqlite3

Comments

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Aug 28, 2023 •
edited by bedevere-bot
Loading

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Proposal:

Seems, there are no tests for sqlite3.Row.keys().

Linked PRs
@serhiy-storchaka serhiy-storchaka added tests Tests in the Lib/test dir 3.11 only security fixes easy 3.12 only security fixes topic-sqlite3 3.13 bugs and security fixes labels Aug 28, 2023
@erlend-aasland erlend-aasland mentioned this issue Aug 28, 2023
Closed
@erlend-aasland erlend-aasland moved this to TODO: Feature in sqlite3 issues Aug 28, 2023
@EddInSverige
Copy link
Contributor

Hey @erlend-aasland / @serhiy-storchaka - I'm a test engineer and would love to contribute to Python. Been using it for 9+ years. Any chance of taking a look at this issue?

@erlend-aasland
Copy link
Contributor

Great, have a go at it! :) I'll get a ping when you create the PR.

@EddInSverige EddInSverige mentioned this issue Aug 28, 2023
Merged
@EddInSverige
Copy link
Contributor

EddInSverige commented Aug 28, 2023 •
edited
Loading

@erlend-aasland I added a single test just to check the whole build/test workflow etc - happy to add more tomorrow. Does the PR itself look okay?

Edit: I guess master is not what we merge into lol, what's the working branch to merge into? Pulled master for this work

EddInSverige added a commit to EddInSverige/cpython that referenced this issue Aug 29, 2023
@EddInSverige
Merge branch 'python:main' into pythongh-108558-fix
bdb16be
@erlend-aasland
Copy link
Contributor

erlend-aasland commented Aug 29, 2023 •
edited
Loading

Edit: I guess master is not what we merge into lol, what's the working branch to merge into? Pulled master for this work

The development branch is called main (currently 3.13 development). Apart from that, we've got release branches (3.12, 3.11, etc.). You're free to choose your own branching scheme when developing. I used to name bug/feature branches using the issue number, but nowadays I group them using a simple naming scheme instead: sqlite/* for sqlite3 stuff, clinic/* for Argument Clinic stuff, docs/* for random documentation stuff, and for everything else, I just make up a name there and then :)

Note: all PRs are squashed into a single merge commit in main, so you don't have to worry about the PR/branch history; it is ok if it is messy! See also the devguide.

@EddInSverige
Copy link
Contributor

Okay great! So I guess my PR is all good then?

If so I'll dig out some more test issues and have a go at them :)

@erlend-aasland erlend-aasland changed the title Add tests for sqlite3.Row.keys() Add explicit test for sqlite3.Row.keys() Aug 29, 2023
@erlend-aasland
Copy link
Contributor

Okay great! So I guess my PR is all good then?

Indeed it is :)

If so I'll dig out some more test issues and have a go at them :)

Great!

BTW; test_sqlite_row_as_dict implicitly already tested keys(), so there is no increase in test coverage post this change.

@erlend-aasland erlend-aasland linked a pull request Aug 29, 2023 that will close this issue
gh-108558: Improve sqlite3 row factory tests #108578
Merged
@github-project-automation github-project-automation bot moved this from TODO: Feature to Done in sqlite3 issues Aug 29, 2023
erlend-aasland added a commit that referenced this issue Aug 29, 2023
@EddInSverige @erlend-aasland
gh-108558: Improve sqlite3 row factory tests (#108578)
6eaddc1 
Add test_sqlite_row_keys() to explicitly test sqlite3.Row.keys().

Cleanups:
- Reduce test noise by converting docstrings to regular comments
- Reduce boilerplate code by adding a setUp() method to RowFactoryTests

Co-authored-by: Erlend E. Aasland <erlend@python.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Aug 29, 2023
@EddInSverige @erlend-aasland @miss-islington
pythongh-108558: Improve sqlite3 row factory tests (pythonGH-108578)
edd2f57 
Add test_sqlite_row_keys() to explicitly test sqlite3.Row.keys().

Cleanups:
- Reduce test noise by converting docstrings to regular comments
- Reduce boilerplate code by adding a setUp() method to RowFactoryTests

(cherry picked from commit 6eaddc1)

Co-authored-by: Edward Schauman-Haigh <142528725+EddInSverige@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend@python.org>
This was referenced Aug 29, 2023
Merged
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Aug 29, 2023
@EddInSverige @erlend-aasland @miss-islington
pythongh-108558: Improve sqlite3 row factory tests (pythonGH-108578)
b185615 
Add test_sqlite_row_keys() to explicitly test sqlite3.Row.keys().

Cleanups:
- Reduce test noise by converting docstrings to regular comments
- Reduce boilerplate code by adding a setUp() method to RowFactoryTests

(cherry picked from commit 6eaddc1)

Co-authored-by: Edward Schauman-Haigh <142528725+EddInSverige@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend@python.org>
erlend-aasland added a commit that referenced this issue Aug 29, 2023
@miss-islington @EddInSverige @erlend-aasland
[3.11] gh-108558: Improve sqlite3 row factory tests (GH-108578) (#108616
cc12c96 
)

Add test_sqlite_row_keys() to explicitly test sqlite3.Row.keys().

Cleanups:
- Reduce test noise by converting docstrings to regular comments
- Reduce boilerplate code by adding a setUp() method to RowFactoryTests

(cherry picked from commit 6eaddc1)

Co-authored-by: Edward Schauman-Haigh <142528725+EddInSverige@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend@python.org>
Yhg1s pushed a commit to Yhg1s/cpython that referenced this issue Aug 29, 2023
@EddInSverige @erlend-aasland @Yhg1s
pythongh-108558: Improve sqlite3 row factory tests (pythonGH-108578)
aa1d4d4 
Add test_sqlite_row_keys() to explicitly test sqlite3.Row.keys().

Cleanups:
- Reduce test noise by converting docstrings to regular comments
- Reduce boilerplate code by adding a setUp() method to RowFactoryTests

(cherry picked from commit 6eaddc1)

Co-authored-by: Edward Schauman-Haigh <142528725+EddInSverige@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend@python.org>
@EddInSverige EddInSverige mentioned this issue Aug 29, 2023
Closed
@EddInSverige
Copy link
Contributor

BTW; test_sqlite_row_as_dict implicitly already tested keys(), so there is no increase in test coverage post this change.

I added a second PR to cover the edge case that the SQL query didn't return any results and a few other behaviours

@erlend-aasland erlend-aasland moved this from Done to In Progress in sqlite3 issues Aug 29, 2023
@serhiy-storchaka
Copy link
Member Author

Thank you for your test @EddInSverige, but other tests look unneeded to me.

@erlend-aasland
Copy link
Contributor

Thank you for your test @EddInSverige, but other tests look unneeded to me.

After looking more closely, I also see that we've got these cases covered already. Thanks, @EddInSverige, but I agree with Serhiy that #108628 is not needed.

@github-project-automation github-project-automation bot moved this from In Progress to Done in sqlite3 issues Aug 29, 2023
Yhg1s pushed a commit that referenced this issue Aug 29, 2023
@miss-islington @EddInSverige @erlend-aasland
[3.12] gh-108558: Improve sqlite3 row factory tests (GH-108578) (#108615
1478a38 
)

* gh-108558: Improve sqlite3 row factory tests (GH-108578)

Add test_sqlite_row_keys() to explicitly test sqlite3.Row.keys().

Cleanups:
- Reduce test noise by converting docstrings to regular comments
- Reduce boilerplate code by adding a setUp() method to RowFactoryTests

(cherry picked from commit 6eaddc1)

Co-authored-by: Edward Schauman-Haigh <142528725+EddInSverige@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend@python.org>

* Fix backport

---------

Co-authored-by: Edward Schauman-Haigh <142528725+EddInSverige@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.11 only security fixes 3.12 only security fixes 3.13 bugs and security fixes easy tests Tests in the Lib/test dir topic-sqlite3
Projects
sqlite3 issues
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

gh-108558: Improve sqlite3 row factory tests EddInSverige/cpython
3 participants
@serhiy-storchaka @erlend-aasland @EddInSverige