Skip to content

bpo-44165: Add tests for sqlite3 SQL string length #26485

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

Closed
wants to merge 4 commits into from

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Jun 2, 2021

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Jun 2, 2021

This slows down the sqlite3 tests considerably:

# with this PR
$ ./python.exe -m test test_sqlite
0:00:00 load avg: 2.67 Run tests sequentially
0:00:00 load avg: 2.67 [1/1] test_sqlite

== Tests result: SUCCESS ==

1 test OK.

Total duration: 10.6 sec
Tests result: SUCCESS

# without this PR
$ ./python.exe -m test test_sqlite
0:00:00 load avg: 2.14 Run tests sequentially
0:00:00 load avg: 2.14 [1/1] test_sqlite

== Tests result: SUCCESS ==

1 test OK.

Total duration: 1.3 sec
Tests result: SUCCESS

I tried to mock the string length, but was unable to do so. What do you think, @pablogsal?

@erlend-aasland
Copy link
Contributor Author

Test time reduced with ~5 seconds after 3687fea, but it's still a massive slow-down.

@pablogsal
Copy link
Member

This slows down the sqlite3 tests considerably:

Oh :( I thought this was going to be faster. I think that 5 seconds is quite expensive for this check in particular. Unfortunately there is no easy way to go around PyUnicode_AsUTF8AndSize even with subclasses. Well, then I assume we would need to proceed without the extra tests. Sorry for the inconvenience

@erlend-aasland
Copy link
Contributor Author

No problem, it was a nice little excercise :) Perhaps we could disable it by default and use some kind of flag to decide if to run it?

@pablogsal
Copy link
Member

No problem, it was a nice little excercise :) Perhaps we could disable it by default and use some kind of flag to decide if to run it?

I was thinking about that but given si just checking this code path I am still not super enthusiastic. Normally those kind of test with -u test behaviour

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.

4 participants