You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mladen Kovacevic (Code Review)" <ge...@cloudera.org> on 2016/10/05 08:15:24 UTC

[kudu-CR] Running kudu-spark tests alone fail finding build dir

Mladen Kovacevic has uploaded a new change for review.

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

Change subject: Running kudu-spark tests alone fail finding build dir
......................................................................

Running kudu-spark tests alone fail finding build dir

When running kudu-spark tests alone using 'mvn test', you end up with a
failure as follows:

03:03:30.930 [WARN - main] (TestUtils.java:108) Unable to find build dir!
myUrl=file:/Users/mladen/.m2/repository/org/apache/kudu/kudu-client/1.1.0-SNAPSHOT/kudu-client-1.1.0-SNAPSHOT-tests.jar

Essentially, the kudu-client jar is typically installed by maven, and
placed in your local machine's .m2 repository. The build dir lookup
logic in TestUtils assumes that the TestUtils class is in your kudu
source code git repo.  However, it won't be when it has been installed
by maven.

The fix here is to have kudu-spark find the location of its own scala
test files, and set the system property appropriately that will end up
overriding the lookup logic done in the kudu-client package.  Hence, it
should find the binaries for kudu-master, etc without a problem.

These changes also modify TestUtils.java slightly, to publicly make
available helper functions and constants so that we're not duplicating
code. This also makes it easier to adopt this strategy in other test
cases.

Change-Id: Ib452086cf57a3cb4249bb7c1f91c7572e55466ce
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
2 files changed, 42 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib452086cf57a3cb4249bb7c1f91c7572e55466ce
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic <ml...@gmail.com>

[kudu-CR] Running kudu-spark tests alone fail finding build dir

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

Change subject: Running kudu-spark tests alone fail finding build dir
......................................................................


Abandoned

After some back and forth offline, best to just leave this alone. The way to specify kudu binary directory is already documented in the java README.md which users can refer to.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ib452086cf57a3cb4249bb7c1f91c7572e55466ce
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic <ml...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic <ml...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] Running kudu-spark tests alone fail finding build dir

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

Change subject: Running kudu-spark tests alone fail finding build dir
......................................................................


Patch Set 2:

I forget: do we already have an easy way to override the "search for binaries" from the command line? maybe we should just make the error/exception message nicer when it can't find the binaries, like:

"Could not auto-detect location of Kudu binaries in build tree. To explicitly set a location, set $KUDU_BIN_DIR in your environment or ensure that the kudu server binaries are on your $PATH" or whatever. That should satisfy the ability to develop Java-ecosystem components and run the tests using a kudu-client in m2 so long as you have a working Kudu install on your system.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib452086cf57a3cb4249bb7c1f91c7572e55466ce
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic <ml...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] Running kudu-spark tests alone fail finding build dir

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

Change subject: Running kudu-spark tests alone fail finding build dir
......................................................................


Patch Set 2:

I'm not sure I agree with the premise of this patch.  Running the kudu-spark tests against a version of kudu-client in your .m2 is not a good thing.  The cached version in .m2 could be old, missing, or in any number of strange states since we build against snapshot versions.  I understand the sad state of the world with regards to running/testing a single submodule of a maven project, but I think this is going to cause more problems than it solves.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib452086cf57a3cb4249bb7c1f91c7572e55466ce
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic <ml...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] Running kudu-spark tests alone fail finding build dir

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

Change subject: Running kudu-spark tests alone fail finding build dir
......................................................................


Patch Set 2: -Code-Review

> I just wanted to make sure that once you pull down the image, if you go straight into kudu-spark, the build and running the testcases should just work. Today they don't.

This workflow is just not possible with multi-module projects with inter-dependencies, due to maven being broken by design.  All builds/tests need to get run from the directory of the top level pom (/java in our case).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib452086cf57a3cb4249bb7c1f91c7572e55466ce
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic <ml...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic <ml...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] Running kudu-spark tests alone fail finding build dir

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

Change subject: Running kudu-spark tests alone fail finding build dir
......................................................................


Patch Set 2:

It seems a little 'hackish' anyway looking for the base kudu-client test class's location, the going up the chain until you hit '.git'. Just thought I'd make it more containable just within the kudu-spark section was the intention.

Maybe its just a documentation change that's needed to explain how we're supposed to build and run tests smoothly?

I'd be happy to change this commit to better documentation. I'm just not sure what to write just yet.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib452086cf57a3cb4249bb7c1f91c7572e55466ce
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic <ml...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic <ml...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] Running kudu-spark tests alone fail finding build dir

Posted by "Mladen Kovacevic (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#2).

Change subject: Running kudu-spark tests alone fail finding build dir
......................................................................

Running kudu-spark tests alone fail finding build dir

When running kudu-spark tests alone using 'mvn test', you end up with a
failure as follows:

03:03:30.930 [WARN - main] (TestUtils.java:108) Unable to find build dir!
myUrl=file:/Users/mladen/.m2/repository/org/apache/kudu/kudu-client/1.1.0-SNAPSHOT/kudu-client-1.1.0-SNAPSHOT-tests.jar

Essentially, the kudu-client jar is typically installed by maven, and
placed in your local machine's .m2 repository. The build dir lookup
logic in TestUtils assumes that the TestUtils class is in your kudu
source code git repo.  However, it won't be when it has been installed
by maven.

The fix here is to have kudu-spark find the location of its own scala
test files, and set the system property appropriately that will end up
overriding the lookup logic done in the kudu-client package.  Hence, it
should find the binaries for kudu-master, etc without a problem.

These changes also modify TestUtils.java slightly, to publicly make
available helper functions and constants so that we're not duplicating
code. This also makes it easier to adopt this strategy in other test
cases.

Change-Id: Ib452086cf57a3cb4249bb7c1f91c7572e55466ce
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
2 files changed, 46 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/4630/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib452086cf57a3cb4249bb7c1f91c7572e55466ce
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic <ml...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] Running kudu-spark tests alone fail finding build dir

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

Change subject: Running kudu-spark tests alone fail finding build dir
......................................................................


Patch Set 2: Code-Review-1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib452086cf57a3cb4249bb7c1f91c7572e55466ce
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic <ml...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] Running kudu-spark tests alone fail finding build dir

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

Change subject: Running kudu-spark tests alone fail finding build dir
......................................................................


Patch Set 2:

So this is interesting. If I go under java/kudu-spark and from there run "mvn clean package -DskipTests", I can see it download the kudu-client to pop into .m2.

In this case, I actually modified kudu-client, so it would even fail to build kudu-spark.

I'd need to go under kudu-client, run "mvn install -DskipTests", then *my* version of kudu-client is put into .m2 and so re-running the build in kudu-spark directory works.

I just wanted to make sure that once you pull down the image, if you go straight into kudu-spark, the build and running the testcases should just work. Today they don't.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib452086cf57a3cb4249bb7c1f91c7572e55466ce
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic <ml...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic <ml...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No