Skip to content

Commit

Permalink
Testfixtures allow a single service only (#46780)
Browse files Browse the repository at this point in the history
This PR adds some restrictions around testfixtures to make sure the same service ( as defiend in docker-compose.yml ) is not shared between multiple projects.
Sharing would break running with --parallel.

Projects can still share fixtures as long as each has it;s own service within.
This is still useful to share some of the setup and configuration code of the fixture.

Project now also have to specify a service name when calling useCluster to refer to a specific service.
If this is not the case all services will be claimed and the fixture can't be shared.
For this reason fixtures have to explicitly specify if they are using themselves ( fixture and tests in the same project ).
  • Loading branch information
alpar-t committed Sep 23, 2019
1 parent ef0b757 commit 5fd7505
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,65 @@
*/
package org.elasticsearch.gradle.testfixtures;

import org.gradle.api.GradleException;
import org.gradle.api.NamedDomainObjectContainer;
import org.gradle.api.Project;

import java.util.HashMap;
import java.util.Map;
import java.util.Optional;

public class TestFixtureExtension {

private final Project project;
final NamedDomainObjectContainer<Project> fixtures;
final Map<String, String> serviceToProjectUseMap = new HashMap<>();

public TestFixtureExtension(Project project) {
this.project = project;
this.fixtures = project.container(Project.class);
}

public void useFixture() {
useFixture(this.project.getPath());
}

public void useFixture(String path) {
addFixtureProject(path);
serviceToProjectUseMap.put(path, this.project.getPath());
}

public void useFixture(String path, String serviceName) {
addFixtureProject(path);
String key = getServiceNameKey(path, serviceName);
serviceToProjectUseMap.put(key, this.project.getPath());

Optional<String> otherProject = this.findOtherProjectUsingService(key);
if (otherProject.isPresent()) {
throw new GradleException(
"Projects " + otherProject.get() + " and " + this.project.getPath() + " both claim the "+ serviceName +
" service defined in the docker-compose.yml of " + path + "This is not supported because it breaks " +
"running in parallel. Configure dedicated services for each project and use those instead."
);
}
}

private String getServiceNameKey(String fixtureProjectPath, String serviceName) {
return fixtureProjectPath + "::" + serviceName;
}

private Optional<String> findOtherProjectUsingService(String serviceName) {
return this.project.getRootProject().getAllprojects().stream()
.filter(p -> p.equals(this.project) == false)
.filter(p -> p.getExtensions().findByType(TestFixtureExtension.class) != null)
.map(project -> project.getExtensions().getByType(TestFixtureExtension.class))
.flatMap(ext -> ext.serviceToProjectUseMap.entrySet().stream())
.filter(entry -> entry.getKey().equals(serviceName))
.map(Map.Entry::getValue)
.findAny();
}

private void addFixtureProject(String path) {
Project fixtureProject = this.project.findProject(path);
if (fixtureProject == null) {
throw new IllegalArgumentException("Could not find test fixture " + fixtureProject);
Expand All @@ -42,6 +87,20 @@ public void useFixture(String path) {
);
}
fixtures.add(fixtureProject);
// Check for exclusive access
Optional<String> otherProject = this.findOtherProjectUsingService(path);
if (otherProject.isPresent()) {
throw new GradleException("Projects " + otherProject.get() + " and " + this.project.getPath() + " both " +
"claim all services from " + path + ". This is not supported because it breaks running in parallel. " +
"Configure specific services in docker-compose.yml for each and add the service name to `useFixture`"
);
}
}

boolean isServiceRequired(String serviceName, String fixtureProject) {
if (serviceToProjectUseMap.containsKey(fixtureProject)) {
return true;
}
return serviceToProjectUseMap.containsKey(getServiceNameKey(fixtureProject, serviceName));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import com.avast.gradle.dockercompose.ComposeExtension;
import com.avast.gradle.dockercompose.DockerComposePlugin;
import com.avast.gradle.dockercompose.ServiceInfo;
import com.avast.gradle.dockercompose.tasks.ComposeUp;
import org.elasticsearch.gradle.OS;
import org.elasticsearch.gradle.SystemPropertyCommandLineArgumentProvider;
Expand Down Expand Up @@ -58,9 +59,6 @@ public void apply(Project project) {
ext.set("testFixturesDir", testfixturesDir);

if (project.file(DOCKER_COMPOSE_YML).exists()) {
// the project that defined a test fixture can also use it
extension.fixtures.add(project);

Task buildFixture = project.getTasks().create("buildFixture");
Task pullFixture = project.getTasks().create("pullFixture");
Task preProcessFixture = project.getTasks().create("preProcessFixture");
Expand Down Expand Up @@ -106,6 +104,7 @@ public void apply(Project project) {
configureServiceInfoForTask(
postProcessFixture,
project,
false,
(name, port) -> postProcessFixture.getExtensions()
.getByType(ExtraPropertiesExtension.class).set(name, port)
);
Expand Down Expand Up @@ -144,6 +143,7 @@ public void apply(Project project) {
configureServiceInfoForTask(
task,
fixtureProject,
true,
(name, host) ->
task.getExtensions().getByType(SystemPropertyCommandLineArgumentProvider.class).systemProperty(name, host)
);
Expand All @@ -165,14 +165,23 @@ private void conditionTaskByType(TaskContainer tasks, TestFixtureExtension exten
);
}

private void configureServiceInfoForTask(Task task, Project fixtureProject, BiConsumer<String, Integer> consumer) {
private void configureServiceInfoForTask(
Task task, Project fixtureProject, boolean enableFilter, BiConsumer<String, Integer> consumer
) {
// Configure ports for the tests as system properties.
// We only know these at execution time so we need to do it in doFirst
TestFixtureExtension extension = task.getProject().getExtensions().getByType(TestFixtureExtension.class);
task.doFirst(new Action<Task>() {
@Override
public void execute(Task theTask) {
fixtureProject.getExtensions().getByType(ComposeExtension.class).getServicesInfos()
.forEach((service, infos) -> {
.entrySet().stream()
.filter(entry -> enableFilter == false ||
extension.isServiceRequired(entry.getKey(), fixtureProject.getPath())
)
.forEach(entry -> {
String service = entry.getKey();
ServiceInfo infos = entry.getValue();
infos.getTcpPorts()
.forEach((container, host) -> {
String name = "test.fixtures." + service + ".tcp." + container;
Expand Down
2 changes: 2 additions & 0 deletions distribution/docker/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import org.elasticsearch.gradle.testfixtures.TestFixturesPlugin
apply plugin: 'elasticsearch.standalone-rest-test'
apply plugin: 'elasticsearch.test.fixtures'

testFixtures.useFixture()

configurations {
dockerPlugins
dockerSource
Expand Down
2 changes: 1 addition & 1 deletion plugins/repository-hdfs/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ versions << [
'hadoop2': '2.8.1'
]

testFixtures.useFixture ":test:fixtures:krb5kdc-fixture"
testFixtures.useFixture ":test:fixtures:krb5kdc-fixture", "hdfs"

configurations {
hdfsFixture
Expand Down
3 changes: 3 additions & 0 deletions plugins/repository-s3/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ task thirdPartyTest(type: Test) {

if (useFixture) {
apply plugin: 'elasticsearch.test.fixtures'

testFixtures.useFixture()

task writeDockerFile {
File minioDockerfile = new File("${project.buildDir}/minio-docker/Dockerfile")
outputs.file(minioDockerfile)
Expand Down
3 changes: 1 addition & 2 deletions x-pack/qa/kerberos-tests/build.gradle
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import java.nio.file.Path
import java.nio.file.Paths
import java.nio.file.Files

apply plugin: 'elasticsearch.testclusters'
apply plugin: 'elasticsearch.standalone-rest-test'
apply plugin: 'elasticsearch.rest-test'
apply plugin: 'elasticsearch.test.fixtures'

testFixtures.useFixture ":test:fixtures:krb5kdc-fixture"
testFixtures.useFixture ":test:fixtures:krb5kdc-fixture", "peppa"

dependencies {
testCompile project(':x-pack:plugin:core')
Expand Down
2 changes: 1 addition & 1 deletion x-pack/qa/oidc-op-tests/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ dependencies {
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')
testCompile project(path: xpackModule('security'), configuration: 'testArtifacts')
}
testFixtures.useFixture ":x-pack:test:idp-fixture"
testFixtures.useFixture ":x-pack:test:idp-fixture", "oidc-provider"

String ephemeralPort;
task setupPorts {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/qa/openldap-tests/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ dependencies {
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')
}

testFixtures.useFixture ":x-pack:test:idp-fixture"
testFixtures.useFixture ":x-pack:test:idp-fixture", "openldap"

Project idpFixtureProject = xpackProject("test:idp-fixture")
String outputDir = "${project.buildDir}/generated-resources/${project.name}"
Expand Down
2 changes: 1 addition & 1 deletion x-pack/qa/third-party/active-directory/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ processTestResources {

compileTestJava.options.compilerArgs << "-Xlint:-rawtypes,-unchecked"

// we have to repeat these patterns because the security test resources are effectively in the src of this project
// we have to repeat these patterns because the security test resources are effectively in the src of this p
forbiddenPatterns {
exclude '**/*.key'
exclude '**/*.p12'
Expand Down

0 comments on commit 5fd7505

Please sign in to comment.