David Knupp has posted comments on this change. Change subject: IMPALA-4510: Selectively filter args for metric verification tests ......................................................................
Patch Set 2: (7 comments) Thanks for the review. http://gerrit.cloudera.org:8080/#/c/5135/2//COMMIT_MSG Commit Message: PS2, Line 9: impala-pytest > impala-py.test Done PS2, Line 10: separate individual > You can just use one of these two words. :) Done Line 28: > Please mention the testing you performed for this patch. Done http://gerrit.cloudera.org:8080/#/c/5135/2/tests/run-tests.py File tests/run-tests.py: Line 73: Modify and return the command line arguments that will be passed to pytest. > Can you also talk about what log_base_name and valid_dirs need to be for a Done PS2, Line 107: # We need to account for the fact that '--foo bar' and '--foo=bar' might : # be supplied by the user. : raw_args = itertools.chain(*[arg.split('=') for arg in sys.argv[1:]]) > Crafty. It would help a reader to show a simple before/after example of wha Done PS2, Line 110: filtered_args = [] > The name here is strange to me. These args aren't being filtered, but in fa Done PS2, Line 113: try: : pytest.config.getvalue(arg.strip('-')) # Raises ValueError if invalid arg : filtered_args += [arg, str(raw_args.next())] : except ValueError: : continue > This is related to the L75 docstring comment above: it might make more sens Done -- To view, visit http://gerrit.cloudera.org:8080/5135 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I069172f44c1307d55f85779cdb01fecc0ba1799e Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-HasComments: Yes
