You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mike Percy (Code Review)" <ge...@cloudera.org> on 2016/11/25 13:58:08 UTC
[kudu-CR] cmake: Throw cmake error if unit test does not exist
Hello Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/5222
to review the following change.
Change subject: cmake: Throw cmake error if unit test does not exist
......................................................................
cmake: Throw cmake error if unit test does not exist
Previously, if the unit test file did not exist, CMake would not notice
and the problem was difficult to debug at test runtime.
Change-Id: I6c7d048f44624c8406198a098e0a7ecb44f8a08e
---
M CMakeLists.txt
1 file changed, 3 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/5222/1
--
To view, visit http://gerrit.cloudera.org:8080/5222
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6c7d048f44624c8406198a098e0a7ecb44f8a08e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
[kudu-CR] cmake: Throw cmake error if unit test does not exist
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged.
Change subject: cmake: Throw cmake error if unit test does not exist
......................................................................
cmake: Throw cmake error if unit test does not exist
Previously, if the unit test file did not exist, CMake would not notice
and the problem was difficult to debug at test runtime.
Change-Id: I6c7d048f44624c8406198a098e0a7ecb44f8a08e
Reviewed-on: http://gerrit.cloudera.org:8080/5222
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M CMakeLists.txt
1 file changed, 3 insertions(+), 1 deletion(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/5222
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6c7d048f44624c8406198a098e0a7ecb44f8a08e
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] cmake: Throw cmake error if unit test does not exist
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5222
to look at the new patch set (#2).
Change subject: cmake: Throw cmake error if unit test does not exist
......................................................................
cmake: Throw cmake error if unit test does not exist
Previously, if the unit test file did not exist, CMake would not notice
and the problem was difficult to debug at test runtime.
Change-Id: I6c7d048f44624c8406198a098e0a7ecb44f8a08e
---
M CMakeLists.txt
1 file changed, 3 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/5222/2
--
To view, visit http://gerrit.cloudera.org:8080/5222
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c7d048f44624c8406198a098e0a7ecb44f8a08e
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] cmake: Throw cmake error if unit test does not exist
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: cmake: Throw cmake error if unit test does not exist
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5222/1/CMakeLists.txt
File CMakeLists.txt:
Line 658: else()
> Would be nice to say that neither REL_TEST_NAME nor REL_TEST_NAME.cc were f
Done
PS1, Line 659: {.cc}
> Are these braces redundant?
they were intended to indicate that the .cc is optional, but I like your idea for the error message so let's go with that.
--
To view, visit http://gerrit.cloudera.org:8080/5222
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c7d048f44624c8406198a098e0a7ecb44f8a08e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] cmake: Throw cmake error if unit test does not exist
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: cmake: Throw cmake error if unit test does not exist
......................................................................
Patch Set 2:
This is what the error output looked like when I defined a non-existent test called consistency-itestxxxx:
CMake Error at CMakeLists.txt:659 (message):
Neither consistency-itestxxxx nor consistency-itestxxxx.cc were found in
/home/mpercy/src/kudu/src/kudu/integration-tests/
Call Stack (most recent call first):
src/kudu/integration-tests/CMakeLists.txt:53 (ADD_KUDU_TEST)
-- Configuring incomplete, errors occurred!
See also "/home/mpercy/src/kudu/build/dynclang/CMakeFiles/CMakeOutput.log".
FAILED: /home/mpercy/src/kudu/thirdparty/installed/common/bin/cmake -H/home/mpercy/src/kudu -B/home/mpercy/src/kudu/build/dynclang
--
To view, visit http://gerrit.cloudera.org:8080/5222
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c7d048f44624c8406198a098e0a7ecb44f8a08e
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] cmake: Throw cmake error if unit test does not exist
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: cmake: Throw cmake error if unit test does not exist
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/5222
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c7d048f44624c8406198a098e0a7ecb44f8a08e
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] cmake: Throw cmake error if unit test does not exist
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: cmake: Throw cmake error if unit test does not exist
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5222/1/CMakeLists.txt
File CMakeLists.txt:
Line 658: else()
Would be nice to say that neither REL_TEST_NAME nor REL_TEST_NAME.cc were found.
PS1, Line 659: {.cc}
Are these braces redundant?
--
To view, visit http://gerrit.cloudera.org:8080/5222
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c7d048f44624c8406198a098e0a7ecb44f8a08e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes