bpo-40823: Use loadTestsFromTestCase() iso. makeSuite() in sqlite3 tests

[erlend-aasland]

https://bugs.python.org/issue40823

[berkerpeksag]

I don't have a strong objection to this, but if we are going to drop Check prefix, I'd prefer using test_* instead of test*.

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[erlend-aasland]

I don't have a strong objection to this, but if we are going to drop Check prefix, I'd prefer using test_* instead of test*.

No problem. I'll rebase onto master and fix the naming.

[erlend-aasland]

PTAL, @berkerpeksag. Rebased onto master with snake case method naming. IMHO, it looks really nice now.

[erlend-aasland]

I have made the requested changes; please review again

[bedevere-bot]

Thanks for making the requested changes!

@berkerpeksag: please review the changes made to this pull request.

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[erlend-aasland]

I have made the requested changes; please review again

[bedevere-bot]

Thanks for making the requested changes!

@berkerpeksag: please review the changes made to this pull request.

[erlend-aasland]

GitHub messed up the encoding again! See #20538 (comment). I must remember no to use the suggestion feature for these files. We should normalise the encodings:

$ file Lib/sqlite3/test/*.py Lib/sqlite3/test/__init__.py: empty Lib/sqlite3/test/backup.py: Python script text executable, ASCII text Lib/sqlite3/test/dbapi.py: Python script text executable, ISO-8859 text Lib/sqlite3/test/dump.py: Python script text executable, ASCII text Lib/sqlite3/test/factory.py: Python script text executable, ISO-8859 text Lib/sqlite3/test/hooks.py: Python script text executable, ISO-8859 text Lib/sqlite3/test/regression.py: Python script text executable, ISO-8859 text Lib/sqlite3/test/transactions.py: Python script text executable, ISO-8859 text Lib/sqlite3/test/types.py: Python script text executable, UTF-8 Unicode text Lib/sqlite3/test/userfunctions.py: Python script text executable, UTF-8 Unicode text 

[erlend-aasland]

The same thing happened in #20448, and we ended up just converting Lib/sqlite3/test/userfunctions.py to UTF-8 in the same PR. I'd say we convert the rest right away. Agreed, @berkerpeksag?

[berkerpeksag]

The same thing happened in #20448, and we ended up just converting Lib/sqlite3/test/userfunctions.py to UTF-8 in the same PR. I'd say we convert the rest right away. Agreed, @berkerpeksag?

I failed to see what's wrong here and #20448 has lots of comments. Why do you want to convert encodings of these files?

[erlend-aasland]

I failed to see what's wrong here and #20448 has lots of comments. Why do you want to convert encodings of these files?

Commit fd4f651 was auto-generated by GitHub from the "suggestion" I set up in #20538 (comment). As you notice from the diff, GitHub silently converted the file from ISO-8859 encoding to UTF-8 encoding (ignoring the #-*- coding: iso-8859-1 -*- header), in addition to applying the "suggestion" I set up. Over at #20448 the same thing happened; Victor Stinner set up a "suggestion", which I applied, and then GitHub silently converted Lib/sqlite3/test/userfunctions.py from ISO-8859 to UTF-8. Instead of reverting the encoding change GitHub made, we decided to just remove the #-*- coding: iso-8859-1 -*- header, since GitHub obviously dislikes ISO-8859.

[berkerpeksag]

Ah, I see now, thanks for the explanation! Personally, I prefer reverting the commit generated by GitHub and apply the suggested change manually. I don't like when companies try to impose their own practices (it doesn't mean they are bad practices) on a perfectly fine piece of code :)

[erlend-aasland]

… or we can revert the encoding change and create an issue for normalising the encodings.

Ah, I see now, thanks for the explanation! Personally, I prefer reverting the commit generated by GitHub and apply the suggested change manually. I don't like when companies try to impose their own practices (it doesn't mean they are bad practices) on a perfectly fine piece of code :)

No problem. I'll open an issue for normalising the file encodings. Consistence FTW :)

[erlend-aasland]

Reverted in f93a63b. PTAL, @berkerpeksag

[berkerpeksag]

LGTM!