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/12/02 16:39:02 UTC

[kudu-CR] java tests: Also search for build bindir from cwd

Hello Dan Burkert,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/5328

to review the following change.

Change subject: java tests: Also search for build bindir from cwd
......................................................................

java tests: Also search for build bindir from cwd

Previously, the logic to find the bindir depended on the location of the
TestUtils class file. In case that doesn't work, such as when the class
is installed in the Maven repository, this patch adds a fallback that
searches for the source root starting from the current working
directory.

This is useful for developers who want to build and test submodules that
depend on the kudu-client test classes without rebuilding all of
kudu-client itself, for example using `mvn install -pl <submodule>`.

Change-Id: Ie2ca92c2d1d54fd04f9300642de478a189e8d611
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java
1 file changed, 29 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/5328/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5328
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie2ca92c2d1d54fd04f9300642de478a189e8d611
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>

[kudu-CR] java tests: Also search for build bindir from cwd

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: java tests: Also search for build bindir from cwd
......................................................................


Patch Set 1:

No, the concern is that developers will unwittingly run tests against a version of kudu-client in their .m2, which can lead to extremely hard to debug issues.  Maven multi-module projects are broken by design, but I don't believe this hack is an acceptable solution.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2ca92c2d1d54fd04f9300642de478a189e8d611
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] java tests: Also search for build bindir from cwd

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: java tests: Also search for build bindir from cwd
......................................................................


Patch Set 1:

This patch allows the following pattern:

 $ mvn clean install -pl kudu-flume-sink -am -DskipTests # recompile deps
 $ mvn clean install -pl kudu-flume-sink # fast edit-run-test cycle

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2ca92c2d1d54fd04f9300642de478a189e8d611
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] java tests: Also search for build bindir from cwd

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has abandoned this change.

Change subject: java tests: Also search for build bindir from cwd
......................................................................


Abandoned

OK, I'll abandon since we have a workaround. I'll post a separate CR to document this inline so we don't keep submitting the same code review.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ie2ca92c2d1d54fd04f9300642de478a189e8d611
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] java tests: Also search for build bindir from cwd

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: java tests: Also search for build bindir from cwd
......................................................................


Patch Set 1:

Is the concern w/ this patch that we will somehow merge something broken? If so, doesn't jenkins protect us against that?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2ca92c2d1d54fd04f9300642de478a189e8d611
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] java tests: Also search for build bindir from cwd

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: java tests: Also search for build bindir from cwd
......................................................................


Patch Set 1:

I've previously rejected this patch, because I think the premise of running tests against an installed kudu client jar is flawed.  See my reasoning here: https://gerrit.cloudera.org/#/c/4630/.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2ca92c2d1d54fd04f9300642de478a189e8d611
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No