impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Brown (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3898: Add a pytest skipif decorator based on presence of Impala LZO.
Date Fri, 29 Jul 2016 17:48:29 GMT
Michael Brown has posted comments on this change.

Change subject: IMPALA-3898: Add a pytest skipif decorator based on presence of Impala LZO.
......................................................................


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/3782/4/tests/common/environ.py
File tests/common/environ.py:

PS4, Line 51:   build_type_dir = build_type_arg_search_result.groups()[0].lower()
Whoops. Thanks.


Line 168: 
Why the extra line?


PS4, Line 251:   """Return Boolean based on whether Impala appears to be built with LZO."""
Based on what we found yesterday, the more accurate comment would be something like

  """Return Boolean based on whether Impala can dynamically load LZO support."""


PS4, Line 260: IMPALA_BUILT_WITH_LZO = _is_impalalzo_present()  # Boolean
Based on what we found yesterday, a more accurate name for this would be something like IMPALA_SUPPORTS_LZO
.


http://gerrit.cloudera.org:8080/#/c/3782/4/tests/conftest.py
File tests/conftest.py:

PS4, Line 176: @pytest.fixture(autouse=True)
Since we're enabling this for autouse, can you run a core build and ensure there's not a noticeable
increase in total run time?


PS4, Line 202:       pytest.skip()
Can you add an argument to skip() with the reason for skipping?


-- 
To view, visit http://gerrit.cloudera.org:8080/3782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If61a7799205cd00d440196303a42db32c522f5b1
Gerrit-PatchSet: 4
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@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message