Added globalization unit tests for various charset including GB18030 #293
Conversation
tests/test_globalization.py
Outdated
| from mssqltestutils import create_mssql_cli_client, shutdown | ||
|
|
||
|
|
||
| # All tests require a live connection to an AdventureWorks2014 database with a hardcoded test server. |
There was a problem hiding this comment.
These tests don't depend on the AdventureWorks2014 schema, so we can remove that reference. #Closed
tests/test_globalization.py
Outdated
| shutdown(client) | ||
|
|
||
|
|
||
| def get_unit_str(self, chars, unit_len = 50): |
There was a problem hiding this comment.
Can we rename this to get_object_name or something more descriptive? Plus add some comments noting this is used to create localized objects. #Closed
There was a problem hiding this comment.
get_unit_str() is removed. #Closed
tests/test_globalization.py
Outdated
| def get_unit_str(self, chars, unit_len = 50): | ||
| curr_index = 0 | ||
| while curr_index < len(chars): | ||
| next_index = min(curr_index + unit_len, len(chars)) |
There was a problem hiding this comment.
It may be more simple to use the substring operator if the character is greater than unit_len. #Closed
There was a problem hiding this comment.
With that approach, it seems we do not even need generator. I removed get_unit_str(). Fixed. #Closed
tests/test_globalization.py
Outdated
| shutdown(client) | ||
|
|
||
|
|
||
| def get_unit_str(self, chars, unit_len = 50): |
There was a problem hiding this comment.
Can we rename unit_len to max_string_length? #Closed
| assert is_error == False | ||
|
|
||
| select_query = u"SELECT * FROM {0};".format(table_name) | ||
| for rows, columns, status, statement, is_error in client.execute_query(select_query): |
There was a problem hiding this comment.
can we assert execute_query returned the expected number of tuples? If it returns 0, it seems like it would pass. #Closed
There was a problem hiding this comment.
Maybe you meant 'if it returns 2, it is pass'? Because we inserts 2 tuples in the tests. #Closed
There was a problem hiding this comment.
What if it returns 2 rows like this?:
('??', 1)
('??', 2)
'??' indicates broken encoding. #Closed
There was a problem hiding this comment.
You should assert that too. #Closed
There was a problem hiding this comment.
Added back the assert statements. #Closed
tests/test_globalization.py
Outdated
| # All tests require a live connection to an AdventureWorks2014 database with a hardcoded test server. | ||
| # Make modifications to mssqlutils.create_mssql_cli_client() to use a different server and database. | ||
| # Please Note: These tests cannot be run offline. | ||
| class GlobalizationTests(unittest.TestCase): |
There was a problem hiding this comment.
Maybe rename to GlobalizationResultSetTests? #Closed
| assert is_error == False | ||
|
|
||
| select_query = u"SELECT * FROM {0};".format(table_name) | ||
| for rows, columns, status, statement, is_error in client.execute_query(select_query): |
There was a problem hiding this comment.
You should assert that too. #Closed
tests/test_globalization.py
Outdated
| for rows, columns, status, statement, is_error in client.execute_query(setup_query): | ||
| assert is_error == False | ||
|
|
||
| select_query = u"SELECT * FROM {0};".format(table_name) |
There was a problem hiding this comment.
Instead of using a select *, can you specify the column name? This way, we make sure we encode the correctly sent over json/rpc as input. #Closed
There was a problem hiding this comment.
Added column names to select statement #Closed
tests/test_globalization.py
Outdated
| max_string_length = 50 | ||
| for chars in chars_list: | ||
| while len(chars) > 0: | ||
| next_index = min(len(chars), max_string_length) |
There was a problem hiding this comment.
nit: next_index isn't that descriptive. How test_string_length? #Closed
There was a problem hiding this comment.
changed to test_string_length as requested. #Closed
| select_query = u"SELECT * FROM {0};".format(table_name) | ||
| for rows, columns, status, statement, is_error in client.execute_query(select_query): | ||
| assert is_error == False | ||
| assert len(rows) == 2 |
There was a problem hiding this comment.
Can we assert the value in the result set matches the input string? #Closed
There was a problem hiding this comment.
Added back the input value assert statements. #Closed
| setup_query = u"CREATE TABLE {0} ({1} nvarchar(MAX), {2} int);"\ | ||
| u"INSERT INTO {0} VALUES (N'value_{3}1', 1);"\ | ||
| u"INSERT INTO {0} VALUES (N'value_{3}2', 2);"\ | ||
| .format(table_name, col1_name, col2_name, test_str) |
There was a problem hiding this comment.
format [](start = 25, length = 6)
Can we insert the entire chars string? #Closed
There was a problem hiding this comment.
tests/test_globalization.py
Outdated
| try: | ||
| client = create_mssql_cli_client() | ||
| max_string_length = 50 | ||
| for chars in chars_list: |
There was a problem hiding this comment.
chars [](start = 16, length = 5)
nit, but important: chars and test_str are vague. Can you rename these to be more descriptive?
There was a problem hiding this comment.
synced offline. Changed to a little more descriptive words.
tests/test_globalization.py
Outdated
| max_string_length = 50 | ||
| for chars in chars_list: | ||
| while len(chars) > 0: | ||
| test_string_length = min(len(chars), max_string_length) |
There was a problem hiding this comment.
I think some comments here describing this loop would be good.
Added globalization unit tests for various charset including GB18030