Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[test] packaging: add java test project #29297

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Vagrantfile
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ export BATS=/project/build/bats
export BATS_UTILS=/project/build/packaging/bats/utils
export BATS_TESTS=/project/build/packaging/bats/tests
export PACKAGING_ARCHIVES=/project/build/packaging/archives
export PACKAGING_TESTS=/project/build/packaging/tests
VARS
cat \<\<SUDOERS_VARS > /etc/sudoers.d/elasticsearch_vars
Defaults env_keep += "ZIP"
Expand All @@ -347,6 +348,7 @@ Defaults env_keep += "BATS"
Defaults env_keep += "BATS_UTILS"
Defaults env_keep += "BATS_TESTS"
Defaults env_keep += "PACKAGING_ARCHIVES"
Defaults env_keep += "PACKAGING_TESTS"
SUDOERS_VARS
chmod 0440 /etc/sudoers.d/elasticsearch_vars
SHELL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class VagrantTestPlugin implements Plugin<Project> {
static List<String> UPGRADE_FROM_ARCHIVES = ['rpm', 'deb']

private static final PACKAGING_CONFIGURATION = 'packaging'
private static final PACKAGING_TEST_CONFIGURATION = 'packagingTest'
private static final BATS = 'bats'
private static final String BATS_TEST_COMMAND ="cd \$PACKAGING_ARCHIVES && sudo bats --tap \$BATS_TESTS/*.$BATS"
private static final String PLATFORM_TEST_COMMAND ="rm -rf ~/elasticsearch && rsync -r /elasticsearch/ ~/elasticsearch && cd ~/elasticsearch && ./gradlew test integTest"
Expand All @@ -58,6 +59,7 @@ class VagrantTestPlugin implements Plugin<Project> {

// Creates custom configurations for Bats testing files (and associated scripts and archives)
createPackagingConfiguration(project)
project.configurations.create(PACKAGING_TEST_CONFIGURATION)

// Creates all the main Vagrant tasks
createVagrantTasks(project)
Expand Down Expand Up @@ -134,10 +136,12 @@ class VagrantTestPlugin implements Plugin<Project> {
}

private static void createCleanTask(Project project) {
project.tasks.create('clean', Delete.class) {
description 'Clean the project build directory'
group 'Build'
delete project.buildDir
if (project.tasks.findByName('clean') == null) {
Copy link
Member

Choose a reason for hiding this comment

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

++. Because if it has the java plugin this isn't needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I figured it would be good to not couple it to the expectation that the project also has the java plugin applied

project.tasks.create('clean', Delete.class) {
description 'Clean the project build directory'
group 'Build'
delete project.buildDir
}
}
}

Expand All @@ -164,6 +168,12 @@ class VagrantTestPlugin implements Plugin<Project> {
from project.configurations[PACKAGING_CONFIGURATION]
}

File testsDir = new File(packagingDir, 'tests')
Copy copyPackagingTests = project.tasks.create('copyPackagingTests', Copy) {
into testsDir
from project.configurations[PACKAGING_TEST_CONFIGURATION]
}

Task createVersionFile = project.tasks.create('createVersionFile', FileContentsTask) {
dependsOn copyPackagingArchives
file "${archivesDir}/version"
Expand Down Expand Up @@ -214,7 +224,7 @@ class VagrantTestPlugin implements Plugin<Project> {

Task vagrantSetUpTask = project.tasks.create('setupPackagingTest')
vagrantSetUpTask.dependsOn 'vagrantCheckVersion'
vagrantSetUpTask.dependsOn copyPackagingArchives, createVersionFile, createUpgradeFromFile
vagrantSetUpTask.dependsOn copyPackagingArchives, copyPackagingTests, createVersionFile, createUpgradeFromFile
vagrantSetUpTask.dependsOn copyBatsTests, copyBatsUtils
}

Expand Down Expand Up @@ -373,10 +383,14 @@ class VagrantTestPlugin implements Plugin<Project> {
packagingTest.dependsOn(batsPackagingTest)
}

// This task doesn't do anything yet. In the future it will execute a jar containing tests on the vm
Task groovyPackagingTest = project.tasks.create("vagrant${boxTask}#groovyPackagingTest")
groovyPackagingTest.dependsOn(up)
groovyPackagingTest.finalizedBy(halt)
Task groovyPackagingTest = project.tasks.create("vagrant${boxTask}#groovyPackagingTest", VagrantCommandTask) {
command 'ssh'
boxName box
environmentVars vagrantEnvVars
dependsOn up, setupPackagingTest
finalizedBy halt
args '--command', 'set -e; for jar in $PACKAGING_TESTS/*.jar; do java -jar $jar; done'
}

TaskExecutionAdapter groovyPackagingReproListener = createReproListener(project, groovyPackagingTest.path)
groovyPackagingTest.doFirst {
Expand Down
36 changes: 34 additions & 2 deletions qa/vagrant/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,27 @@
* under the License.
*/

apply plugin: 'elasticsearch.vagrantsupport'
apply plugin: 'elasticsearch.vagrant'
plugins {
id 'groovy'
id 'application'
id 'com.github.johnrengelman.shadow' version '2.0.2'
id 'elasticsearch.build'
id 'elasticsearch.vagrantsupport'
id 'elasticsearch.vagrant'
}

dependencies {
compile localGroovy()
compile "org.hamcrest:hamcrest-all:${versions.hamcrest}"
runtime project(':libs:elasticsearch-core') // needs to be on the classpath for JarHell
packagingTest project(path: project.path, configuration: 'shadow')
}

shadowJar {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it'd be simpler not to make an uber jar at all and instead execute a main class with classpath. I see that you iterate so you can run multiple jar files so you'd lose that. And you'd have to communicate the main class name to the plugin. I don't know if that is worth it just not to have to build the uber jar, but I think it is worth thinking about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. The iterating over multiple jars thing isn't really super important, I just figured "all the jars in this directory get executed" is a simple way to reason about it. It also gives a flexible way (the packagingTest configuration) to choose which test projects get included

Similarly, using an uberjar isn't necessarily the goal but I did want however it works to be easy to run by hand inside the test VM like we can with bats (since iterating through gradle on the host is time consuming)

I'll think about this more (also would like to hear thoughts from @rjernst on this)

Copy link
Member

Choose a reason for hiding this comment

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

I would stay away from shading if we can. What about having the executable jar contain a classpath entry, and having a simple "test" script be generated which invokes the jar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll give that a try. After thinking about it a little, dumping the test jars and all their dependencies into a directory and doing java -cp /project/packaging/tests MainClass is still really simple

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rjernst I pushed c16706d to have it copy the test jars and their deps separately (rather than an uberjar) and have the test projects specify a main class

dependencies {
exclude(project(":libs:elasticsearch-core")) // isn't needed in tests
}
}

List<String> plugins = []
for (Project subproj : project.rootProject.subprojects) {
Expand All @@ -39,3 +58,16 @@ setupPackagingTest {
expectedPlugins.setText(plugins.join('\n'), 'UTF-8')
}
}

mainClassName = 'org.elasticsearch.packaging.PackagingMain'

tasks.test.enabled = false
Copy link
Member

Choose a reason for hiding this comment

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

Why does this use elasticsearch.build if all these tasks are just disabled? Should this just be a plain groovy project, or is there something else that is needed? If something else, please leave a comment in this file explaining the reasoning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We discussed this - I was able to get most of the precommit tasks passing, now only test and the license checks are disabled


tasks.dependencyLicenses.enabled = false
tasks.dependenciesInfo.enabled = false

tasks.thirdPartyAudit.enabled = false

tasks.forbiddenApis.enabled = false
tasks.forbiddenApisMain.enabled = false
tasks.forbiddenApisTest.enabled = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.packaging

/**
* This class doesn't do anything yet
*/
class PackagingMain {
static void main(String[] args) {}
}