Adar Dembo has posted comments on this change.

Change subject: Bump test timeout for flex_partitioning-itest
......................................................................


Patch Set 1:

(3 comments)

Test failure is weird; perhaps an oom kill on the Gradle daemon?

http://gerrit.cloudera.org:8080/#/c/7814/1/CMakeLists.txt
File CMakeLists.txt:

Line 673:     set(ARG_TIMEOUT 900)
Can we avoid defining this default value both here and in run-test.sh? Given 
that we want to support running tests at multiple levels (running the binary 
directly, running it through run-test.sh, running it through ctest, running it 
through dist-test) this may not be possible.

If you don't use the ctest timeout, you could condition all of the environment 
changes on ARG_TIMEOUT, which means the test timeout will be some custom value 
(if set in CMakeLists.txt) or the default value from run-test.sh (if not). Then 
there won't be a need to define this here.


Line 707:   # Set the ctest timeout to be a bit longer than the timeout we pass 
to
Why bother with the ctest timeout at all, given that the run-test.sh timeout 
(respected by every test, dist-test or otherwise) is shorter? Are you worried 
about deadlocks/hangs induced by run-test.sh's post-processing?


Line 714:   if("${CUR_TEST_ENV}" STREQUAL "NOTFOUND")
does if(NOT CUR_TEST_ENV) work?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I07661415d3500dfa5d7984b720b7a6f14597cc19
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

Reply via email to