Skip to content

Fix transport version validation task name (#133186) #133260

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.

Sign up for GitHub

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

Closed
rjernst wants to merge 1 commit into from
Closed

Fix transport version validation task name (#133186) #133260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 8 additions & 11 deletions ...roovy/org/elasticsearch/gradle/internal/transport/AbstractTransportVersionFuncTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest {
javaResource("myserver", "transport/definitions/unreferenced/" + name + ".csv", id)
}

def definedAndUsedTransportVersion(String name, String ids) {
return definedAndUsedTransportVersion(name, ids, "Test${name.capitalize()}")
def namedAndReferencedTransportVersion(String name, String ids) {
return namedAndReferencedTransportVersion(name, ids, "Test${name.capitalize()}")
}

def definedAndUsedTransportVersion(String name, String ids, String classname) {
def namedAndReferencedTransportVersion(String name, String ids, String classname) {
javaSource("myserver", "org.elasticsearch", classname, "", """
static final TransportVersion usage = TransportVersion.fromName("${name}");
""")
Expand All @@ -60,17 +60,17 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest {
return gradleRunner(":${project}:validateTransportVersionReferences").buildAndFail()
}

def validateDefinitionsFails() {
return gradleRunner(":myserver:validateTransportVersionDefinitions").buildAndFail()
def validateResourcesFails() {
return gradleRunner(":myserver:validateTransportVersionResources").buildAndFail()
}

def assertReferencesFailure(BuildResult result, String project, String expectedOutput) {
def assertValidateReferencesFailure(BuildResult result, String project, String expectedOutput) {
result.task(":${project}:validateTransportVersionReferences").outcome == TaskOutcome.FAILED
assertOutputContains(result.output, expectedOutput)
}

def assertDefinitionsFailure(BuildResult result, String expectedOutput) {
result.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.FAILED
def assertValidateResourcesFailure(BuildResult result, String expectedOutput) {
result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.FAILED
assertOutputContains(result.output, expectedOutput)
}

Expand All @@ -81,9 +81,6 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest {
include ':myserver'
include ':myplugin'
"""
file("gradle.properties") << """
org.elasticsearch.transport.definitionsProject=:myserver
"""

file("myserver/build.gradle") << """
apply plugin: 'java-library'
Expand Down
92 changes: 46 additions & 46 deletions ...ovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes

def "test setup works"() {
when:
def result = gradleRunner("validateTransportVersionDefinitions", "validateTransportVersionReferences").build()
def result = gradleRunner("validateTransportVersionResources", "validateTransportVersionReferences").build()
then:
result.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.SUCCESS
result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS
result.task(":myserver:validateTransportVersionReferences").outcome == TaskOutcome.SUCCESS
result.task(":myplugin:validateTransportVersionReferences").outcome == TaskOutcome.SUCCESS
}
Expand All @@ -32,27 +32,27 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes
when:
def result = validateReferencesFails("myplugin")
then:
assertReferencesFailure(result, "myplugin", "TransportVersion.fromName(\"dne\") was used at " +
assertValidateReferencesFailure(result, "myplugin", "TransportVersion.fromName(\"dne\") was used at " +
"org.elasticsearch.plugin.MyPlugin line 6, but lacks a transport version definition.")
}

def "references must be defined"() {
given:
namedTransportVersion("not_used", "1000000")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Transport version definition file " +
assertValidateResourcesFailure(result, "Transport version definition file " +
"[myserver/src/main/resources/transport/definitions/named/not_used.csv] is not referenced")
}

def "names must be lowercase alphanum or underscore"() {
given:
definedAndUsedTransportVersion("${name}", "8100000", "TestNames")
namedAndReferencedTransportVersion("${name}", "8100000", "TestNames")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Transport version definition file " +
assertValidateResourcesFailure(result, "Transport version definition file " +
"[myserver/src/main/resources/transport/definitions/named/${name}.csv] does not have a valid name, " +
"must be lowercase alphanumeric and underscore")

Expand All @@ -62,130 +62,130 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes

def "definitions contain at least one id"() {
given:
definedAndUsedTransportVersion("empty", "")
namedAndReferencedTransportVersion("empty", "")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Transport version definition file " +
assertValidateResourcesFailure(result, "Transport version definition file " +
"[myserver/src/main/resources/transport/definitions/named/empty.csv] does not contain any ids")
}

def "definitions have ids in descending order"() {
given:
definedAndUsedTransportVersion("out_of_order", "8100000,8200000")
namedAndReferencedTransportVersion("out_of_order", "8100000,8200000")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Transport version definition file " +
assertValidateResourcesFailure(result, "Transport version definition file " +
"[myserver/src/main/resources/transport/definitions/named/out_of_order.csv] does not have ordered ids")
}

def "definition ids are unique"() {
given:
definedAndUsedTransportVersion("duplicate", "8123000")
namedAndReferencedTransportVersion("duplicate", "8123000")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Transport version definition file " +
assertValidateResourcesFailure(result, "Transport version definition file " +
"[myserver/src/main/resources/transport/definitions/named/existing_92.csv] contains id 8123000 already defined in " +
"[myserver/src/main/resources/transport/definitions/named/duplicate.csv]")
}

def "definitions have bwc ids with non-zero patch part"() {
given:
definedAndUsedTransportVersion("patched", "8200000,8100000")
namedAndReferencedTransportVersion("patched", "8200000,8100000")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Transport version definition file " +
assertValidateResourcesFailure(result, "Transport version definition file " +
"[myserver/src/main/resources/transport/definitions/named/patched.csv] contains bwc id [8100000] with a patch part of 0")
}

def "definitions have primary ids which cannot change"() {
given:
namedTransportVersion("existing_92", "8500000")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Transport version definition file " +
assertValidateResourcesFailure(result, "Transport version definition file " +
"[myserver/src/main/resources/transport/definitions/named/existing_92.csv] has modified primary id from 8123000 to 8500000")
}

def "cannot change committed ids to a branch"() {
given:
namedTransportVersion("existing_92", "8123000,8012002")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Transport version definition file " +
assertValidateResourcesFailure(result, "Transport version definition file " +
"[myserver/src/main/resources/transport/definitions/named/existing_92.csv] modifies existing patch id from 8012001 to 8012002")
}

def "latest files must reference defined name"() {
given:
latestTransportVersion("9.2", "dne", "8123000")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Latest transport version file " +
assertValidateResourcesFailure(result, "Latest transport version file " +
"[myserver/src/main/resources/transport/latest/9.2.csv] contains transport version name [dne] which is not defined")
}

def "latest files id must exist in definition"() {
given:
latestTransportVersion("9.2", "existing_92", "8124000")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Latest transport version file " +
assertValidateResourcesFailure(result, "Latest transport version file " +
"[myserver/src/main/resources/transport/latest/9.2.csv] has id 8124000 which is not in definition " +
"[myserver/src/main/resources/transport/definitions/named/existing_92.csv]")
}

def "latest files have latest id within base"() {
given:
latestTransportVersion("9.0", "seemingly_latest", "8110001")
definedAndUsedTransportVersion("original", "8110000")
definedAndUsedTransportVersion("seemingly_latest", "8111000,8110001")
definedAndUsedTransportVersion("actual_latest", "8112000,8110002")
namedAndReferencedTransportVersion("original", "8110000")
namedAndReferencedTransportVersion("seemingly_latest", "8111000,8110001")
namedAndReferencedTransportVersion("actual_latest", "8112000,8110002")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Latest transport version file " +
assertValidateResourcesFailure(result, "Latest transport version file " +
"[myserver/src/main/resources/transport/latest/9.0.csv] has id 8110001 from [seemingly_latest] with base 8110000 " +
"but another id 8110002 from [actual_latest] is later for that base")
}

def "latest files cannot change base id"() {
given:
definedAndUsedTransportVersion("original", "8013000")
definedAndUsedTransportVersion("patch", "8015000,8013001")
namedAndReferencedTransportVersion("original", "8013000")
namedAndReferencedTransportVersion("patch", "8015000,8013001")
latestTransportVersion("9.1", "patch", "8013001")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Latest transport version file " +
assertValidateResourcesFailure(result, "Latest transport version file " +
"[myserver/src/main/resources/transport/latest/9.1.csv] modifies base id from 8012000 to 8013000")
}

def "ids must be dense"() {
given:
definedAndUsedTransportVersion("original", "8013000")
definedAndUsedTransportVersion("patch1", "8015000,8013002")
namedAndReferencedTransportVersion("original", "8013000")
namedAndReferencedTransportVersion("patch1", "8015000,8013002")
latestTransportVersion("9.0", "patch1", "8013002")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Transport version base id 8013000 is missing patch ids between 8013000 and 8013002")
assertValidateResourcesFailure(result, "Transport version base id 8013000 is missing patch ids between 8013000 and 8013002")
}

def "primary id must not be patch version"() {
given:
definedAndUsedTransportVersion("patch", "8015001")
namedAndReferencedTransportVersion("patch", "8015001")
when:
def result = validateDefinitionsFails()
def result = validateResourcesFails()
then:
assertDefinitionsFailure(result, "Transport version definition file " +
assertValidateResourcesFailure(result, "Transport version definition file " +
"[myserver/src/main/resources/transport/definitions/named/patch.csv] has patch version 8015001 as primary id")
}

Expand All @@ -194,8 +194,8 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes
file("myserver/src/main/resources/transport/unreferenced/initial_9_0_0.csv").delete()
file("myserver/src/main/resources/transport/unreferenced").deleteDir()
when:
def result = gradleRunner(":myserver:validateTransportVersionDefinitions").build()
def result = gradleRunner(":myserver:validateTransportVersionResources").build()
then:
result.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.SUCCESS
result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS
}
}
15 changes: 3 additions & 12 deletions ...ain/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,12 @@
import org.gradle.api.Project;
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.artifacts.dsl.DependencyHandler;
import org.gradle.api.file.Directory;
import org.gradle.api.plugins.JavaPlugin;
import org.gradle.api.tasks.Copy;
import org.gradle.language.base.plugins.LifecycleBasePlugin;

import java.util.Map;

import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getDefinitionsDirectory;
import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getResourcesDirectory;

public class TransportVersionResourcesPlugin implements Plugin<Project> {

@Override
Expand All @@ -43,22 +39,17 @@ public void apply(Project project) {
}

var validateTask = project.getTasks()
.register("validateTransportVersionDefinitions", ValidateTransportVersionResourcesTask.class, t -> {
.register("validateTransportVersionResources", ValidateTransportVersionResourcesTask.class, t -> {
t.setGroup("Transport Versions");
t.setDescription("Validates that all defined TransportVersion constants are used in at least one project");
Directory resourcesDir = getResourcesDirectory(project);
if (resourcesDir.getAsFile().exists()) {
t.getResourcesDirectory().set(resourcesDir);
}
t.setDescription("Validates that all transport version resources are internally consistent with each other");
t.getReferencesFiles().setFrom(tvReferencesConfig);
});
project.getTasks().named(LifecycleBasePlugin.CHECK_TASK_NAME).configure(t -> t.dependsOn(validateTask));

var generateManifestTask = project.getTasks()
.register("generateTransportVersionManifest", GenerateTransportVersionManifestTask.class, t -> {
t.setGroup("Transport Versions");
t.setDescription("Generate a manifest resource for all the known transport version definitions");
t.getDefinitionsDirectory().set(getDefinitionsDirectory(getResourcesDirectory(project)));
t.setDescription("Generate a manifest resource for all transport version definitions");
t.getManifestFile().set(project.getLayout().getBuildDirectory().file("generated-resources/manifest.txt"));
});
project.getTasks().named(JavaPlugin.PROCESS_RESOURCES_TASK_NAME, Copy.class).configure(t -> {
Expand Down