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

Fixed broken unit test test_config.py and test_naive_completion.py #260

Merged
merged 2 commits into from
Aug 2, 2019

Conversation

geleems
Copy link

@geleems geleems commented Jul 31, 2019

Fixed broken unit test test_config.py and test_naive_completion.py

  • test test_config.py was using wrong API for creating temporary directory.
  • test_naive_completion.py: these tests were using wrong expected values and wrong API for creating set from completer.get_completions() method
    • test_empty_string_completion
    • test_select_keyword_completion
    • test_function_name_completion
    • test_column_name_completion
    • test_paths_completion
    • test_alter_well_known_keywords_completion

@geleems geleems requested a review from pensivebrian July 31, 2019 01:12
@geleems geleems self-assigned this Jul 31, 2019
Copy link
Member

@pensivebrian pensivebrian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure these tests pass running with Python 2.7

@geleems
Copy link
Author

geleems commented Aug 1, 2019

I found there is 1 test failure in test_naive_completion.py when I ran with Python 2.7. It turned out there is a bug in MssqlCompleter.find_matches() method. When Python 2, the method trys to create Completion object with ASCII string where only unicode string is accepted. All strings in Python 3 are unicode, so it was not blown up on Python 3.

@geleems geleems requested a review from pensivebrian August 1, 2019 02:21
@geleems geleems merged commit fdd541d into master Aug 2, 2019
@geleems geleems deleted the gelee/tf1 branch August 7, 2019 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants