Michael Brown has posted comments on this change. Change subject: IMPALA-3326: TestShowCreateTable switched to use unique_database ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/2768/1/tests/metadata/test_show_create_table.py File tests/metadata/test_show_create_table.py: Line 60: def __run_show_create_table_test_case(self, test_file_name, vector, unique_database): Should we prefer a different name other than unique_database for this parameter? 1. The method can be given any database name 2. It might be a good practice to avoid naming formal parameters the same as our fixtures. Line 86: test_case = ShowCreateTableTestCase(test_section, test_file_name, unique_database) Fix pending L60 decision. Line 190: EXPECTED_DB_NAME = "show_create_table_test_db" I'm not comfortable with this name either, from the standpoint that it's simply a placeholder for string replacement. The real, expected name is derived via str.replace() L207. Unfortunately it's hard for me to give a better example for a name here. I'll think about it. Thoughts? Line 207: self.expected_result = expected_result.replace( : ShowCreateTableTestCase.EXPECTED_DB_NAME, test_db_name) Fix pending L190 decision. -- To view, visit http://gerrit.cloudera.org:8080/2768 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4855a3a6ff0cea9f6cb3f8433e8705762afefe7d Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Harrison Sheinblatt <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-HasComments: Yes
