Michael Ho has posted comments on this change. Change subject: IMPALA-3608: Updates pytest to allow multiple exception messages ......................................................................
Patch Set 1: (6 comments) I ran some tests locally which throw exception. The following sand box build is in progress but I will start a new one: http://sandbox.jenkins.cloudera.com/job/impala-private-build-and-test/3206/ http://gerrit.cloudera.org:8080/#/c/3205/1//COMMIT_MSG Commit Message: Line 7: IMPALA-3608: Allow multiple different exception messages > Can you specify this change is for test infrastructure, or end to end tests Done http://gerrit.cloudera.org:8080/#/c/3205/1/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: Line 192: def verify_exceptions(self, expected_strs, actual_str, use_db): > I think this should be a private method (prefixed with _), or perhaps scope Oh. Didn't know we have the convention of prefixing private method with '_' Good to know. Function is now prefixed with two '_'. Line 197: actual_str = actual_str.replace('\n', '') > Why not either of: Good point. Chose the latter one. Line 199: """ : In error messages, some paths are always qualified and some are not. : So, allow both $NAMENODE and $FILESYSTEM_PREFIX to be used in CATCH. : """ > This should be a comment, not a docstring. Done http://gerrit.cloudera.org:8080/#/c/3205/1/tests/util/test_file_parser.py File tests/util/test_file_parser.py: Line 177: subsection_str = '\n'.join([line for line in lines[1:-1]]) > It would add some readability to store lines[1:-1] as a variable. Plus you Done Line 204: for line in lines[1:-1]: : parsed_sections['CATCH'].append(line) > What about: Done -- To view, visit http://gerrit.cloudera.org:8080/3205 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie6d81fd3ae601f565b575edfeefff7c5a6c07974 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-2.6.0_5.8.0 Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-HasComments: Yes
