Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9494 )
Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py script to handle exit_code for EE tests when no tests are collected.After this change return_code will be either 0 if no tests are expected to be collected (dry-run) and 1 if tests are expected to be collected ...................................................................... Patch Set 4: (10 comments) http://gerrit.cloudera.org:8080/#/c/9494/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9494/4//COMMIT_MSG@7 PS4, Line 7: IMPALA-5886 & IMPALA-4812 Update run-tests.py script to handle The commit convention is: IMPALA-XXX: Brief description <new line> Detailed description. http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py File tests/run-tests.py: http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@65 PS4, Line 65: class TestStatisticsPlugin: Use 2 spaces to follow Impala Python convention. http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@71 PS4, Line 71: # items represents the list of collected test items Use Python's docstring comment instead. http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@87 PS4, Line 87: Remove extra new line. http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@88 PS4, Line 88: plugin = TestStatisticsPlugin() Since we may be adding a new plugin in the future, it's probably better to inline TestStatisticsPlugin in line: 91. http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@96 PS4, Line 96: if "--collect-only" in args: In general, we use single quotes for strings in Python. http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@100 PS4, Line 100: if exit_code == 5: Some documentation on what exit_code 5 means? http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@232 PS4, Line 232: for i in range(1,len(sys.argv)): What's the purpose of copying all elements into a new list? http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@240 PS4, Line 240: collect_mode = True I don't see where collect_mode is declared or used. http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@243 PS4, Line 243: if '--collect-only' not in sys.argv: Isn't it just an else statement for the if statement in line: 239? -- To view, visit http://gerrit.cloudera.org:8080/9494 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150 Gerrit-Change-Number: 9494 Gerrit-PatchSet: 4 Gerrit-Owner: Nithya Janarthanan <njanartha...@cloudera.com> Gerrit-Reviewer: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Nithya Janarthanan <njanartha...@cloudera.com> Gerrit-Comment-Date: Wed, 07 Mar 2018 00:07:06 +0000 Gerrit-HasComments: Yes
