Skip to content

Conversation

@lresende
Copy link
Member

@lresende lresende commented Apr 19, 2016

What changes were proposed in this pull request?

Create a maven profile for executing the docker integration tests using maven
Remove docker integration tests from main sbt build
Update documentation on how to run docker integration tests from sbt

How was this patch tested?

Manual test of the docker integration tests as in :
mvn -Pdocker-integration-tests -pl :spark-docker-integration-tests_2.11 compile test

Other comments

Note that the the DB2 Docker Tests are still disabled as there is a kernel version issue on the AMPLab Jenkins slaves and we would need to get them on the right level before enabling those tests. They do run ok locally with the updates from PR #12348

@lresende
Copy link
Member Author

@JoshRosen how would I go about updating pr build to run the integration tests in Jenkins only ? Should we run the tests after the ./dev/run-tests-jenkins ?

@SparkQA
Copy link

SparkQA commented Apr 19, 2016

Test build #56281 has finished for PR 12508 at commit 4552964.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 20, 2016

Test build #56299 has finished for PR 12508 at commit b8b2521.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Apr 20, 2016

I wouldn't mind this, just because these tests always fail on a machine without docker, which is a little surprising.

@lresende
Copy link
Member Author

@JoshRosen @rxin ping

@SparkQA
Copy link

SparkQA commented Apr 21, 2016

Test build #56562 has finished for PR 12508 at commit 5867866.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@vanzin
Copy link
Contributor

vanzin commented Apr 21, 2016

Would a better solution be to only run these tests if docker is installed? Kinda like what RPackageUtilsSuite.scala does for R (see calls to assume).

@srowen
Copy link
Member

srowen commented Apr 21, 2016

That's a decent idea if it's not too complex. If so, these could remain ignored for now per @JoshRosen but at least that would make these not fail for anyone without docker.

@lresende
Copy link
Member Author

@vanzin @srowen I wouldn't recommend running when docker is installed, as this would run on the Jenkins slaves, and @JoshRosen disable these because they were failing intermittently. Having a way to explicitly ask for the tests to run would give more flexibility, where we could have a daily or weekly build that would run these tests, etc

@SparkQA
Copy link

SparkQA commented Apr 27, 2016

Test build #57100 has finished for PR 12508 at commit a925f33.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Apr 30, 2016

I think we could provide a profile to activate Docker tests, to provide a little more indirection in case it requires more Maven config later.

@JoshRosen what do you think?

@JoshRosen
Copy link
Contributor

I'm fine guarding this behind a Maven profile and running them nightly.

@srowen
Copy link
Member

srowen commented May 3, 2016

@JoshRosen this may be a dumb question, but does that mean we'd then need to modify the Jenkins jobs' config to specify the new profile? that's not something I am able to do, I think. or is the config actually in the repo and I am just not finding it?

@lresende
Copy link
Member Author

lresende commented May 3, 2016

The SparkPullRequestBuilder Build basically calls ./dev/run-tests-jenkins while the nightly snapshot build is running some scripts from @pwendell. So I don't believe that there is any other code on Spark repository that needs to be changed, but if we want to enable the docker tests for nightly builds, we have to chat with Patrick.

lresende added 3 commits May 4, 2016 13:41
This reverts commit 90933e2.

With the changes to remove docker from main build, disabling them is not
necessary as there might be ocasions we actually want to run these tests.
@SparkQA
Copy link

SparkQA commented May 4, 2016

Test build #57796 has finished for PR 12508 at commit b626312.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 4, 2016

Test build #57799 has finished for PR 12508 at commit 099e105.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented May 5, 2016

OK, this LGTM. This re-enables tests Josh disabled, but then of course they're not enabled by default because they're behind a profile. Eventually we need to re-enable them for testing, and that requires a change to Jenkins config. I don't know how to do that, but, I suppose we don't want to do that right now anyway if the tests are temporarily disabled? then seems fine to merge.


override val db = new DatabaseOnDocker {
override val imageName = "wnameless/oracle-xe-11g:latest"
override val imageName = "wnameless/oracle-xe-11g:14.04.4"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is probably a good idea, to fix the version, but just checking this was intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srowen Yes, the other tests were already with fixed docker image version, but Oracle one that was introduce recently was not. Having a fixed one enables us to have consistency on the backend version, and also avoid timeout issues on builds trying to download new version of the docker image. Please let me know if you want me to use a different jira to make this change more explicit.

@srowen
Copy link
Member

srowen commented May 6, 2016

OK, I will merge this to master and to 2.0. @JoshRosen this means that re-enabling Docker tests will be a config change in Jenkins rather than a code change. Let me make sure the master build is happy and then will merge back to 2.0, just to be sure.

@asfgit asfgit closed this in a03c5e6 May 6, 2016
asfgit pushed a commit that referenced this pull request May 6, 2016
## What changes were proposed in this pull request?

Create a maven profile for executing the docker integration tests using maven
Remove docker integration tests from main sbt build
Update documentation on how to run docker integration tests from sbt

## How was this patch tested?

Manual test of the docker integration tests as in :
mvn -Pdocker-integration-tests -pl :spark-docker-integration-tests_2.11 compile test

## Other comments

Note that the the DB2 Docker Tests are still disabled as there is a kernel version issue on the AMPLab Jenkins slaves and we would need to get them on the right level before enabling those tests. They do run ok locally with the updates from PR #12348

Author: Luciano Resende <[email protected]>

Closes #12508 from lresende/docker.

(cherry picked from commit a03c5e6)
Signed-off-by: Sean Owen <[email protected]>
@lresende lresende deleted the docker branch May 7, 2016 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants