Skip to content
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

bpo-43853: Expand test suite for SQLite UDF's #27642

Merged
merged 16 commits into from
Jan 26, 2022

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Aug 6, 2021

  • Test that strings that contain embedded NULL characters are passed to UDF's
  • Consolidate "test param" tests to reduce the amount of boilerplate code
  • Test more error conditions when building UDF arguments, and returning UDF values
  • Handle PyFloat_AsDouble() errors

https://bugs.python.org/issue43853

@erlend-aasland
Copy link
Contributor Author

FYI: Tests for aggregate functions are already in place (implemented in GH-27588).

@serhiy-storchaka, is the refactor commit acceptable? Code coverage is preserved.

@erlend-aasland erlend-aasland added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels Aug 6, 2021
Lib/sqlite3/test/userfunctions.py Outdated Show resolved Hide resolved
Lib/sqlite3/test/userfunctions.py Outdated Show resolved Hide resolved
Lib/sqlite3/test/userfunctions.py Outdated Show resolved Hide resolved
Lib/sqlite3/test/userfunctions.py Outdated Show resolved Hide resolved
@serhiy-storchaka
Copy link
Member

Ignore my comments about values which cause errors. I covered them in #27654. But please add more test cases for valid values of different types.

Modules/_sqlite/connection.c Show resolved Hide resolved
Lib/sqlite3/test/userfunctions.py Outdated Show resolved Hide resolved
Lib/sqlite3/test/userfunctions.py Outdated Show resolved Hide resolved
Lib/sqlite3/test/userfunctions.py Outdated Show resolved Hide resolved
Lib/sqlite3/test/userfunctions.py Outdated Show resolved Hide resolved
Lib/sqlite3/test/userfunctions.py Outdated Show resolved Hide resolved
@erlend-aasland
Copy link
Contributor Author

I've updated the PR. PTAL, @serhiy-storchaka.

@erlend-aasland erlend-aasland marked this pull request as draft September 14, 2021 21:44
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Sep 14, 2021

Marked as draft, as I need to resolve some potential issues created by GH-27884.

Resolved: only test arguments passed to UDF's

@erlend-aasland erlend-aasland marked this pull request as ready for review September 15, 2021 08:23
@erlend-aasland
Copy link
Contributor Author

@serhiy-storchaka, please take a new look.

@erlend-aasland
Copy link
Contributor Author

Go ahead and land it!

I can't; I don't have the commit bit :)

@gvanrossum
Copy link
Member

What?! I thought you did. You should. Who's mentoring you?

@gvanrossum gvanrossum merged commit 3eb3b4f into python:main Jan 26, 2022
@miss-islington
Copy link
Contributor

Thanks @erlend-aasland for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @erlend-aasland and @gvanrossum, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 3eb3b4f270757f66c7fb6dcf5afa416ee1582a4b 3.10

@miss-islington
Copy link
Contributor

Sorry @erlend-aasland and @gvanrossum, I had trouble checking out the 3.9 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 3eb3b4f270757f66c7fb6dcf5afa416ee1582a4b 3.9

@JelleZijlstra
Copy link
Member

I can't; I don't have the commit bit :)

I was surprised when I learned that too, I feel like I've seen your name around CPython a lot :)

@erlend-aasland
Copy link
Contributor Author

What?! I thought you did. You should. Who's mentoring you?

First @corona10, now @pablogsal 🙂

(BTW, I'll fix the backports.)

@erlend-aasland
Copy link
Contributor Author

I was surprised when I learned that too, I feel like I've seen your name around CPython a lot :)

I've been around a year or two, IIRC :)

@pablogsal
Copy link
Member

What?! I thought you did. You should. Who's mentoring you?

We have already offer the promotion a couple of times, we are working on it 😉

@erlend-aasland erlend-aasland deleted the sqlite-test-udf-str-with-null branch January 26, 2022 21:12
erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Jan 26, 2022
(cherry picked from commit 3eb3b4f)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jan 30, 2022
@bedevere-bot
Copy link

GH-31030 is a backport of this pull request to the 3.10 branch.

@erlend-aasland erlend-aasland removed the needs backport to 3.9 only security fixes label Jan 30, 2022
@erlend-aasland
Copy link
Contributor Author

Backporting to 3.9 from GH-31030.

JelleZijlstra pushed a commit that referenced this pull request Feb 21, 2022
…1030)

* [3.10] bpo-43853: Expand test suite for SQLite UDF's (GH-27642).
(cherry picked from commit 3eb3b4f)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>

* Fix test_func_return_too_large_int

GH-27613 (bpo 44839) was not backported, so exceptions differ between
main (3.11) and older versions.
erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Feb 25, 2022
erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Feb 26, 2022
miss-islington pushed a commit that referenced this pull request Mar 2, 2022
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants