impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jim Apple (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4735: Upgrade pytest in python env to version 2.9.2.
Date Fri, 20 Jan 2017 21:57:08 GMT
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 <dknupp@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message