Repository navigation
Remove PyHive-derived DB API tests repeated on the same code path #1267
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
1b0b876
Drop the removed aio test_unicode from NOTICE (GH-1260)
laughingman7743 9e81393
Remove PyHive-derived fetch tests repeated on the same code path (GH-…
laughingman7743 9e652a5
Check values through AsyncS3FSCursor and keep the PyHive attribution …
laughingman7743 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Independent review (relayed), finding 2 (P2), same reviewer and session as the review on
tests/pyathena/s3fs/test_async_cursor.py: deleting the whole reference totests/pyathena/aio/sqlalchemy/test_base.pyleaves recognizable PyHive material unlisted. Itstest_basic_queryrunsSELECT * FROM one_rowand checks one row,number_of_rows == 1, and one column, as PyHive'sSqlAlchemyTestCase.test_basic_querydoes. Replace the obsolete example with the surviving test instead of removing the file's attribution.Author verification: confirmed; the aio
test_reflect_schemasalso checks the test schema anddefaultas PyHive's does. Checking the same question for the DB API tests found thattest_arraysize(arraysize = 5,SELECT * FROM many_rows LIMIT 20,len(fetchmany()) == 5) and the aiotest_fetchoneremain PyHive's test cases in the cursor tests of the other cursor classes, which NOTICE did not mention before this change either.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repaired in 8e85cb2: NOTICE states that
tests/pyathena/aio/sqlalchemy/test_base.pyportstest_basic_queryandtest_reflect_schemasto the asynchronous dialect, and that the cursor tests of the other cursor classes under tests/pyathena/ repeat some of PyHive's DB API test cases, such astest_arraysizeandtest_fetchone. Self-review of the repair: both statements were checked against the remaining tests and PyHive ae3c9a96 (SqlAlchemyTestCase.test_basic_query/test_reflect_schemas,DBAPITestCase.test_arraysize/test_fetchone); the PR body now describes the NOTICE change.