-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Convert discovery-* from integTest to [yaml | java]RestTest or internalClusterTest #60084
Changes from 3 commits
e295cba
632da91
d3cb054
01114f9
1a6f9ed
8e3cc82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,15 +22,14 @@ import org.elasticsearch.gradle.MavenFilteringHack | |
import org.elasticsearch.gradle.info.BuildParams | ||
import org.elasticsearch.gradle.test.AntFixture | ||
import org.elasticsearch.gradle.test.RestIntegTestTask | ||
import org.elasticsearch.gradle.test.rest.YamlRestTestPlugin | ||
|
||
import static org.elasticsearch.gradle.PropertyNormalization.IGNORE_VALUE | ||
|
||
apply plugin: 'elasticsearch.standalone-rest-test' | ||
apply plugin: 'elasticsearch.rest-test' | ||
apply plugin: 'elasticsearch.rest-resources' | ||
apply plugin: 'elasticsearch.yaml-rest-test' | ||
|
||
dependencies { | ||
testImplementation project(':plugins:discovery-ec2') | ||
yamlRestTestImplementation project(':plugins:discovery-ec2') | ||
} | ||
|
||
restResources { | ||
|
@@ -45,13 +44,13 @@ Map<String, Object> expansions = [ | |
'expected_nodes': ec2NumberOfNodes | ||
] | ||
|
||
processTestResources { | ||
processYamlRestTestResources { | ||
inputs.properties(expansions) | ||
MavenFilteringHack.filter(it, expansions) | ||
} | ||
|
||
// disable default test task, use spezialized ones below | ||
integTest.enabled = false | ||
// disable default yamlRestTest task, use spezialized ones below | ||
yamlRestTest.enabled = false | ||
|
||
/* | ||
* Test using various credential providers (see also https://docs.aws.amazon.com/sdk-for-java/v2/developer-guide/credentials.html): | ||
|
@@ -66,19 +65,26 @@ integTest.enabled = false | |
*/ | ||
['KeyStore', 'EnvVariables', 'SystemProperties', 'ContainerCredentials', 'InstanceProfile'].forEach { action -> | ||
AntFixture fixture = tasks.create(name: "ec2Fixture${action}", type: AntFixture) { | ||
dependsOn compileTestJava | ||
env 'CLASSPATH', "${-> project.sourceSets.test.runtimeClasspath.asPath}" | ||
dependsOn compileYamlRestTestJava | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should really be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done 01114f9 |
||
env 'CLASSPATH', "${-> project.sourceSets.yamlRestTest.runtimeClasspath.asPath}" | ||
executable = "${BuildParams.runtimeJavaHome}/bin/java" | ||
args 'org.elasticsearch.discovery.ec2.AmazonEC2Fixture', baseDir, "${buildDir}/testclusters/integTest${action}-1/config/unicast_hosts.txt" | ||
args 'org.elasticsearch.discovery.ec2.AmazonEC2Fixture', baseDir, "${buildDir}/testclusters/yamlRestTest${action}-1/config/unicast_hosts.txt" | ||
} | ||
|
||
tasks.create(name: "integTest${action}", type: RestIntegTestTask) { | ||
tasks.create(name: "yamlRestTest${action}", type: RestIntegTestTask) { | ||
dependsOn fixture, project(':plugins:discovery-ec2').bundlePlugin | ||
} | ||
SourceSetContainer sourceSets = project.getExtensions().getByType(SourceSetContainer.class); | ||
SourceSet yamlRestTestSourceSet = sourceSets.getByName(YamlRestTestPlugin.SOURCE_SET_NAME) | ||
"yamlRestTest${action}" { | ||
runner { | ||
setTestClassesDirs(yamlRestTestSourceSet.getOutput().getClassesDirs()) | ||
setClasspath(yamlRestTestSourceSet.getRuntimeClasspath()) | ||
} | ||
} | ||
check.dependsOn("yamlRestTest${action}") | ||
|
||
check.dependsOn("integTest${action}") | ||
|
||
testClusters."integTest${action}" { | ||
testClusters."yamlRestTest${action}" { | ||
numberOfNodes = ec2NumberOfNodes | ||
plugin project(':plugins:discovery-ec2').bundlePlugin.archiveFile | ||
|
||
|
@@ -91,27 +97,27 @@ integTest.enabled = false | |
} | ||
|
||
// Extra config for KeyStore | ||
testClusters.integTestKeyStore { | ||
testClusters.yamlRestTestKeyStore { | ||
keystore 'discovery.ec2.access_key', 'ec2_integration_test_access_key' | ||
keystore 'discovery.ec2.secret_key', 'ec2_integration_test_secret_key' | ||
} | ||
|
||
// Extra config for EnvVariables | ||
testClusters.integTestEnvVariables { | ||
testClusters.yamlRestTestEnvVariables { | ||
environment 'AWS_ACCESS_KEY_ID', 'ec2_integration_test_access_key' | ||
environment 'AWS_SECRET_ACCESS_KEY', 'ec2_integration_test_secret_key' | ||
} | ||
|
||
// Extra config for SystemProperties | ||
testClusters.integTestSystemProperties { | ||
testClusters.yamlRestTestSystemProperties { | ||
systemProperty 'aws.accessKeyId', 'ec2_integration_test_access_key' | ||
systemProperty 'aws.secretKey', 'ec2_integration_test_secret_key' | ||
} | ||
|
||
// Extra config for ContainerCredentials | ||
ec2FixtureContainerCredentials.env 'ACTIVATE_CONTAINER_CREDENTIALS', true | ||
|
||
testClusters.integTestContainerCredentials { | ||
testClusters.yamlRestTestContainerCredentials { | ||
environment 'AWS_CONTAINER_CREDENTIALS_FULL_URI', | ||
{ "http://${-> tasks.findByName("ec2FixtureContainerCredentials").addressAndPort}/ecs_credentials_endpoint" }, IGNORE_VALUE | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,14 +24,12 @@ import org.elasticsearch.gradle.test.AntFixture | |
|
||
import static org.elasticsearch.gradle.PropertyNormalization.IGNORE_VALUE | ||
|
||
apply plugin: 'elasticsearch.standalone-rest-test' | ||
apply plugin: 'elasticsearch.rest-test' | ||
apply plugin: 'elasticsearch.rest-resources' | ||
apply plugin: 'elasticsearch.yaml-rest-test' | ||
|
||
final int gceNumberOfNodes = 3 | ||
|
||
dependencies { | ||
testImplementation project(':plugins:discovery-gce') | ||
yamlRestTestImplementation project(':plugins:discovery-gce') | ||
} | ||
|
||
restResources { | ||
|
@@ -42,26 +40,26 @@ restResources { | |
|
||
/** A task to start the GCEFixture which emulates a GCE service **/ | ||
task gceFixture(type: AntFixture) { | ||
dependsOn compileTestJava | ||
env 'CLASSPATH', "${-> project.sourceSets.test.runtimeClasspath.asPath}" | ||
dependsOn compileYamlRestTestJava | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, we should depending on the source set runtime classpath here, not just the compile task. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done 01114f9 |
||
env 'CLASSPATH', "${-> project.sourceSets.yamlRestTest.runtimeClasspath.asPath}" | ||
executable = "${BuildParams.runtimeJavaHome}/bin/java" | ||
args 'org.elasticsearch.cloud.gce.GCEFixture', baseDir, "${buildDir}/testclusters/integTest-1/config/unicast_hosts.txt" | ||
args 'org.elasticsearch.cloud.gce.GCEFixture', baseDir, "${buildDir}/testclusters/yamlRestTest-1/config/unicast_hosts.txt" | ||
} | ||
|
||
Map<String, Object> expansions = [ | ||
'expected_nodes': gceNumberOfNodes | ||
] | ||
|
||
processTestResources { | ||
processYamlRestTestResources { | ||
inputs.properties(expansions) | ||
MavenFilteringHack.filter(it, expansions) | ||
} | ||
|
||
integTest { | ||
yamlRestTest { | ||
dependsOn gceFixture, project(':plugins:discovery-gce').bundlePlugin | ||
} | ||
|
||
testClusters.integTest { | ||
testClusters.yamlRestTest { | ||
numberOfNodes = gceNumberOfNodes | ||
plugin project(':plugins:discovery-gce').bundlePlugin.archiveFile | ||
// use gce fixture for Auth calls instead of http://metadata.google.internal | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like one step forward and one step back. I assume this is also just to appease our testing conventions stuff? If so I feel like this should not be a failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup. However, it is a bit more rare to have no unit tests (at least until i start converting the qa projects). For this one we will probably want to toggle the enable/disable the test task if the directory exists or not. I can look into that for a follow up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we are going to have this all over the place for the qa projects that contain only rest tests.