Jim Apple has posted comments on this change. Change subject: IMPALA-4735: Upgrade pytest in python env to version 2.9.2. ......................................................................
Patch Set 3: (10 comments) Thank you for making this patch http://gerrit.cloudera.org:8080/#/c/5640/3//COMMIT_MSG Commit Message: PS3, Line 7: 2.9.2 Why not 3.0.5? Line 13: In addition to bumping the version of pytest (and related modules), It might make more sense to split these changes up. Line 15: showed up from pytest re: the imported TestMatrix and TestDimension Could you put in the commit message the sed script (or whatever) you used to do the change? Line 24: Tested by doing a standard (non-exhaustive) test run on centos 6.4 Presumably that checks that the tests that run do not fail. How did you verify that this patch does not accidentally disable any tests? Is there a pytest dry-run mode that just lists the tests? You could compare the lists before and after this patch. http://gerrit.cloudera.org:8080/#/c/5640/3/tests/common/kudu_test_suite.py File tests/common/kudu_test_suite.py: Line 61: cls.ImpalaTestMatrix.add_dimension(create_uncompressed_text_dimension(cls.get_workload())) long line http://gerrit.cloudera.org:8080/#/c/5640/3/tests/custom_cluster/test_permanent_udfs.py File tests/custom_cluster/test_permanent_udfs.py: Line 48: cls.ImpalaTestMatrix.add_dimension(create_uncompressed_text_dimension(cls.get_workload())) long line http://gerrit.cloudera.org:8080/#/c/5640/3/tests/experiments/test_targeted_perf.py File tests/experiments/test_targeted_perf.py: Line 33: cls.ImpalaTestMatrix.add_constraint(lambda v: v.get_value('exec_option')['batch_size'] == 0) long line, here and elsewhere http://gerrit.cloudera.org:8080/#/c/5640/3/tests/run-tests.py File tests/run-tests.py: PS3, Line 101: we need to account for Why? What bad thing could happen otherwise? PS3, Line 103: random "arbitrary", not "random" PS3, Line 143: Because of the way our repo is organized. This doesn't explain a lot as it stands. I'm also not sure if it is a continuation of the above sentence or the beginning of the next one. -- To view, visit http://gerrit.cloudera.org:8080/5640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I40d129e0e63ca5bee126bac6ac923abb3c7e0a67 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-HasComments: Yes
