Repository navigation
Add explicit test for sqlite3.Row.keys() #108558
Description
Activity
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?
Reacted by Erlend E. AaslandGreat, have a go at it! :) I'll get a ping when you create the PR.
@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
- added a commit that references this issue
on Aug 29, 2023 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.Reacted by Edward Schauman-HaighOkay 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 :)
- changed the title
[-]Add tests for sqlite3.Row.keys()[/-][+]Add explicit test for sqlite3.Row.keys()[/+]on Aug 29, 2023 2 remaining items
BTW;
test_sqlite_row_as_dictimplicitly already testedkeys(), 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
Reacted by Erlend E. AaslandThank you for your test @EddInSverige, but other tests look unneeded to me.
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.
Reacted by Edward Schauman-Haigh
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