Skip to content

Commit

Permalink
Update task names for rest compatiblity (#75267)
Browse files Browse the repository at this point in the history
This commit updates two task names:
```
yamlRestCompatTest -> yamlRestTestV7CompatTest
transformV7RestTests -> yamlRestTestV7CompatTransform 
```

`7` is the N-1 version and calculated, such that when `8` is 
N-1 version the task names will be  `yamlRestTestV8CompatTest` and 
`yamlRestTestV8CompatTransform`

The motivation for `yamlRestCompatTest -> yamlRestTestV7CompatTest`  is that 
many projects have configured `yamlRestCompatTest` 
but that configuration is specific to the N-1 version. For example, 
if we blacklist tests when running compatibility with v7, we don't also
want to blacklist those tests when running compatibility with v8.

By introducing a version-specific identifier in the name, the task will not
even exist when bumping the version creating the need to (correctly) remove
the version-specific condition.

The motivation for `transformV7RestTests -> yamlRestTestV7CompatTransform` 
is to provide more consistent naming. 

The idea behind the naming is the main task people
are likely familiar with is :

`yamlRestTest` so we will use that as a base.
`yamlRestTestV7CompatTest` to run the version-specific compat tests
`yamlRestTestV7CompatTransform` to run the version-specific transformations for the compat tests

CI should be un-effected since since we introduced a lifecycle task 
name `checkRestCompat` which is what CI should be configured to use.
  • Loading branch information
jakelandis authored Sep 3, 2021
1 parent 222d95a commit 26dfe02
Show file tree
Hide file tree
Showing 15 changed files with 54 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ class YamlRestCompatTestPluginFuncTest extends AbstractRestResourcesFuncTest {
def compatibleVersion = Version.fromString(VersionProperties.getVersions().get("elasticsearch")).getMajor() - 1
def specIntermediateDir = "restResources/v${compatibleVersion}/yamlSpecs"
def testIntermediateDir = "restResources/v${compatibleVersion}/yamlTests"
def transformTask = ":transformV${compatibleVersion}RestTests"
def transformTask = ":yamlRestTestV${compatibleVersion}CompatTransform"
def YAML_FACTORY = new YAMLFactory()
def MAPPER = new ObjectMapper(YAML_FACTORY)
def READER = MAPPER.readerFor(ObjectNode.class)
def WRITER = MAPPER.writerFor(ObjectNode.class)

def "yamlRestCompatTest does nothing when there are no tests"() {
def "yamlRestTestVxCompatTest does nothing when there are no tests"() {
given:
addSubProject(":distribution:bwc:minor") << """
configurations { checkout }
Expand All @@ -44,16 +44,16 @@ class YamlRestCompatTestPluginFuncTest extends AbstractRestResourcesFuncTest {
"""

when:
def result = gradleRunner("yamlRestCompatTest", '--stacktrace').build()
def result = gradleRunner("yamlRestTestV${compatibleVersion}CompatTest", '--stacktrace').build()

then:
result.task(':yamlRestCompatTest').outcome == TaskOutcome.NO_SOURCE
result.task(":yamlRestTestV${compatibleVersion}CompatTest").outcome == TaskOutcome.NO_SOURCE
result.task(':copyRestCompatApiTask').outcome == TaskOutcome.NO_SOURCE
result.task(':copyRestCompatTestTask').outcome == TaskOutcome.NO_SOURCE
result.task(transformTask).outcome == TaskOutcome.NO_SOURCE
}

def "yamlRestCompatTest executes and copies api and transforms tests from :bwc:minor"() {
def "yamlRestTestVxCompatTest executes and copies api and transforms tests from :bwc:minor"() {
given:
internalBuild()

Expand Down Expand Up @@ -93,10 +93,10 @@ class YamlRestCompatTestPluginFuncTest extends AbstractRestResourcesFuncTest {
file("distribution/bwc/minor/checkoutDir/src/yamlRestTest/resources/rest-api-spec/test/" + test) << ""

when:
def result = gradleRunner("yamlRestCompatTest").build()
def result = gradleRunner("yamlRestTestV${compatibleVersion}CompatTest").build()

then:
result.task(':yamlRestCompatTest').outcome == TaskOutcome.SKIPPED
result.task(":yamlRestTestV${compatibleVersion}CompatTest").outcome == TaskOutcome.SKIPPED
result.task(':copyRestCompatApiTask').outcome == TaskOutcome.SUCCESS
result.task(':copyRestCompatTestTask').outcome == TaskOutcome.SUCCESS
result.task(transformTask).outcome == TaskOutcome.SUCCESS
Expand All @@ -123,16 +123,16 @@ class YamlRestCompatTestPluginFuncTest extends AbstractRestResourcesFuncTest {
result.task(':copyYamlTestsTask').outcome == TaskOutcome.NO_SOURCE

when:
result = gradleRunner("yamlRestCompatTest").build()
result = gradleRunner("yamlRestTestV${compatibleVersion}CompatTest").build()

then:
result.task(':yamlRestCompatTest').outcome == TaskOutcome.SKIPPED
result.task(":yamlRestTestV${compatibleVersion}CompatTest").outcome == TaskOutcome.SKIPPED
result.task(':copyRestCompatApiTask').outcome == TaskOutcome.UP_TO_DATE
result.task(':copyRestCompatTestTask').outcome == TaskOutcome.UP_TO_DATE
result.task(transformTask).outcome == TaskOutcome.UP_TO_DATE
}

def "yamlRestCompatTest is wired into check and checkRestCompat"() {
def "yamlRestTestVxCompatTest is wired into check and checkRestCompat"() {
given:

addSubProject(":distribution:bwc:minor") << """
Expand All @@ -155,7 +155,7 @@ class YamlRestCompatTestPluginFuncTest extends AbstractRestResourcesFuncTest {
then:
result.task(':check').outcome == TaskOutcome.UP_TO_DATE
result.task(':checkRestCompat').outcome == TaskOutcome.UP_TO_DATE
result.task(':yamlRestCompatTest').outcome == TaskOutcome.NO_SOURCE
result.task(":yamlRestTestV${compatibleVersion}CompatTest").outcome == TaskOutcome.NO_SOURCE
result.task(':copyRestCompatApiTask').outcome == TaskOutcome.NO_SOURCE
result.task(':copyRestCompatTestTask').outcome == TaskOutcome.NO_SOURCE
result.task(transformTask).outcome == TaskOutcome.NO_SOURCE
Expand All @@ -169,7 +169,7 @@ class YamlRestCompatTestPluginFuncTest extends AbstractRestResourcesFuncTest {
then:
result.task(':check').outcome == TaskOutcome.UP_TO_DATE
result.task(':checkRestCompat').outcome == TaskOutcome.UP_TO_DATE
result.task(':yamlRestCompatTest').outcome == TaskOutcome.SKIPPED
result.task(":yamlRestTestV${compatibleVersion}CompatTest").outcome == TaskOutcome.SKIPPED
result.task(':copyRestCompatApiTask').outcome == TaskOutcome.SKIPPED
result.task(':copyRestCompatTestTask').outcome == TaskOutcome.SKIPPED
result.task(transformTask).outcome == TaskOutcome.SKIPPED
Expand All @@ -195,7 +195,7 @@ class YamlRestCompatTestPluginFuncTest extends AbstractRestResourcesFuncTest {
dependencies {
yamlRestTestImplementation "junit:junit:4.12"
}
tasks.named("transformV7RestTests").configure({ task ->
tasks.named("yamlRestTestV${compatibleVersion}CompatTransform").configure({ task ->
task.replaceValueInMatch("_type", "_doc")
task.replaceValueInMatch("_source.values", ["z", "x", "y"], "one")
task.removeMatch("_source.blah")
Expand Down Expand Up @@ -266,7 +266,7 @@ class YamlRestCompatTestPluginFuncTest extends AbstractRestResourcesFuncTest {
- match: {}
""".stripIndent()
when:
def result = gradleRunner("yamlRestCompatTest").build()
def result = gradleRunner("yamlRestTestV${compatibleVersion}CompatTest").build()

then:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
* Apply this plugin to run the YAML based REST tests from a prior major version against this version's cluster.
*/
public class YamlRestCompatTestPlugin implements Plugin<Project> {
public static final String REST_COMPAT_CHECK_TASK_NAME = "checkRestCompat";
public static final String SOURCE_SET_NAME = "yamlRestCompatTest";
private static final String REST_COMPAT_CHECK_TASK_NAME = "checkRestCompat";
private static final String SOURCE_SET_NAME = "yamlRestCompatTest";
private static final Path RELATIVE_API_PATH = Path.of("rest-api-spec/api");
private static final Path RELATIVE_TEST_PATH = Path.of("rest-api-spec/test");
private static final Path RELATIVE_REST_API_RESOURCES = Path.of("rest-api-spec/src/main/resources");
Expand Down Expand Up @@ -138,9 +138,10 @@ public void apply(Project project) {
task.onlyIf(t -> isEnabled(project));
});


// transform the copied tests task
TaskProvider<RestCompatTestTransformTask> transformCompatTestTask = project.getTasks()
.register("transformV" + compatibleVersion + "RestTests", RestCompatTestTransformTask.class, task -> {
.register("yamlRestTestV"+ compatibleVersion + "CompatTransform", RestCompatTestTransformTask.class, task -> {
task.getSourceDirectory().set(copyCompatYamlTestTask.flatMap(CopyRestTestsTask::getOutputResourceDir));
task.getOutputDirectory()
.set(project.getLayout().getBuildDirectory().dir(compatTestsDir.resolve("transformed").toString()));
Expand All @@ -161,9 +162,11 @@ public void apply(Project project) {
.named(RestResourcesPlugin.COPY_YAML_TESTS_TASK)
.flatMap(CopyRestTestsTask::getOutputResourceDir);

// setup the yamlRestTest task
Provider<RestIntegTestTask> yamlRestCompatTestTask = RestTestUtil.registerTestTask(project, yamlCompatTestSourceSet);
project.getTasks().withType(RestIntegTestTask.class).named(SOURCE_SET_NAME).configure(testTask -> {
String testTaskName = "yamlRestTestV"+ compatibleVersion + "CompatTest";

// setup the test task
Provider<RestIntegTestTask> yamlRestCompatTestTask = RestTestUtil.registerTestTask(project, yamlCompatTestSourceSet, testTaskName);
project.getTasks().withType(RestIntegTestTask.class).named(testTaskName).configure(testTask -> {
// Use test runner and classpath from "normal" yaml source set
testTask.setTestClassesDirs(
yamlTestSourceSet.getOutput().getClassesDirs().plus(yamlCompatTestSourceSet.getOutput().getClassesDirs())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,18 @@ private RestTestUtil() {
}

/**
* Creates a task with the source set name of type {@link RestIntegTestTask}
* Creates a {@link RestIntegTestTask} task with the source set of the same name
*/
public static Provider<RestIntegTestTask> registerTestTask(Project project, SourceSet sourceSet) {
return registerTestTask(project, sourceSet, sourceSet.getName());
}

/**
* Creates a {@link RestIntegTestTask} task with a custom name for the provided source set
*/
public static Provider<RestIntegTestTask> registerTestTask(Project project, SourceSet sourceSet, String taskName) {
// lazily create the test task
return project.getTasks().register(sourceSet.getName(), RestIntegTestTask.class, testTask -> {
return project.getTasks().register(taskName, RestIntegTestTask.class, testTask -> {
testTask.setGroup(JavaBasePlugin.VERIFICATION_GROUP);
testTask.setDescription("Runs the REST tests against an external cluster");
project.getPlugins().withType(JavaPlugin.class, t ->
Expand Down
2 changes: 1 addition & 1 deletion modules/analysis-common/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ dependencies {
compileOnly project(':modules:lang-painless')
}

tasks.named("yamlRestCompatTest").configure {
tasks.named("yamlRestTestV7CompatTest").configure {
systemProperty 'tests.rest.blacklist', [
//marked as not needing compatible api
'indices.analyze/10_analyze/htmlStrip_deprecated', // Cleanup versioned deprecations in analysis #41560
Expand Down
4 changes: 2 additions & 2 deletions modules/ingest-common/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,6 @@ tasks.named("thirdPartyAudit").configure {
)
}

tasks.named("transformV7RestTests").configure({ task ->
tasks.named("yamlRestTestV7CompatTransform").configure { task ->
task.addAllowedWarningRegex("\\[types removal\\].*")
})
}
4 changes: 2 additions & 2 deletions modules/ingest-user-agent/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ testClusters.all {
extraConfigFile 'ingest-user-agent/test-regexes.yml', file('src/test/test-regexes.yml')
}

tasks.named("transformV7RestTests").configure({ task ->
tasks.named("yamlRestTestV7CompatTransform").configure {task ->
task.addAllowedWarningRegex("setting \\[ecs\\] is deprecated as ECS format is the default and only option")
})
}
4 changes: 2 additions & 2 deletions modules/lang-mustache/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ restResources {
}
}

tasks.named("transformV7RestTests").configure({ task ->
tasks.named("yamlRestTestV7CompatTransform").configure {task ->
task.addAllowedWarningRegex("\\[types removal\\].*")
})
}
2 changes: 1 addition & 1 deletion modules/parent-join/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ restResources {
restApi {
include '_common', 'bulk', 'cluster', 'nodes', 'indices', 'index', 'search'
}
}
}
2 changes: 1 addition & 1 deletion modules/percolator/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ restResources {
}
}

tasks.named("transformV7RestTests").configure({ task ->
tasks.named("yamlRestTestV7CompatTransform").configure({ task ->
task.addAllowedWarningRegex("\\[types removal\\].*")
})
4 changes: 2 additions & 2 deletions modules/reindex/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,11 @@ if (Os.isFamily(Os.FAMILY_WINDOWS)) {
}
}
}
tasks.named("transformV7RestTests").configure({ task ->
tasks.named("yamlRestTestV7CompatTransform").configure({ task ->
task.addAllowedWarningRegex("\\[types removal\\].*")
})

tasks.named("yamlRestCompatTest").configure {
tasks.named("yamlRestTestV7CompatTest").configure {
systemProperty 'tests.rest.blacklist', [
'reindex/20_validation/reindex without source gives useful error message', // exception with a type. Not much benefit adding _doc there.
'update_by_query/20_validation/update_by_query without source gives useful error message' // exception with a type. Not much benefit adding _doc there.
Expand Down
2 changes: 1 addition & 1 deletion plugins/analysis-icu/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ restResources {
tasks.named("dependencyLicenses").configure {
mapping from: /lucene-.*/, to: 'lucene'
}
tasks.named("yamlRestCompatTest").configure {
tasks.named("yamlRestTestV7CompatTest").configure {
systemProperty 'tests.rest.blacklist', [
//marked as not needing compatible api
'analysis_icu/10_basic/Normalization with deprecated unicodeSetFilter' // Cleanup versioned deprecations in analysis #41560
Expand Down
2 changes: 1 addition & 1 deletion plugins/ingest-attachment/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,6 @@ if (BuildParams.inFipsJvm) {
tasks.named("jarHell").configure { enabled = false }
tasks.named("test").configure { enabled = false }
tasks.named("yamlRestTest").configure { enabled = false };
tasks.named("yamlRestCompatTest").configure { enabled = false };
tasks.named("yamlRestTestV7CompatTest").configure { enabled = false };
tasks.named("testingConventions").configure { enabled = false };
}
5 changes: 3 additions & 2 deletions rest-api-spec/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ testClusters.all {

tasks.named("test").configure { enabled = false }
tasks.named("jarHell").configure { enabled = false }
tasks.named("yamlRestCompatTest").configure {
tasks.named("yamlRestTestV7CompatTest").configure {
systemProperty 'tests.rest.blacklist', [
// Cat API are meant to be consumed by humans, so will not be supported by Compatible REST API
'cat*/*/*',
Expand Down Expand Up @@ -84,6 +84,7 @@ tasks.named("yamlRestCompatTest").configure {
// upgrade api will only get a dummy endpoint returning an exception suggesting to use _reindex
'indices.upgrade/*/*',


'search.aggregation/20_terms/*profiler*', // The profiler results aren't backwards compatible.
'search.aggregation/370_doc_count_field/Test filters agg with doc_count', // Uses profiler for assertions which is not backwards compatible

Expand All @@ -99,7 +100,7 @@ tasks.named("yamlRestCompatTest").configure {
].join(',')
}

tasks.named("transformV7RestTests").configure({ task ->
tasks.named("yamlRestTestV7CompatTransform").configure({ task ->
task.replaceValueInMatch("_type", "_doc")
task.addAllowedWarningRegex("\\[types removal\\].*")
task.replaceValueInMatch("nodes.\$node_id.roles.8", "ml", "node_info role test")
Expand Down
12 changes: 5 additions & 7 deletions x-pack/plugin/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ tasks.named("yamlRestTest").configure {
dependsOn "copyExtraResources"
}

tasks.named("transformV7RestTests").configure({ task ->
tasks.named("yamlRestTestV7CompatTransform").configure({ task ->
task.replaceKeyInDo("license.delete", "xpack-license.delete")
task.replaceKeyInDo("license.get", "xpack-license.get")
task.replaceKeyInDo("license.get_basic_status", "xpack-license.get_basic_status")
Expand All @@ -113,8 +113,11 @@ tasks.named("transformV7RestTests").configure({ task ->

task.replaceKeyInDo("ssl.certificates", "xpack-ssl.certificates", "Test get SSL certificates")
task.addAllowedWarningRegexForTest(".*_xpack/ssl.* is deprecated.*", "Test get SSL certificates")
task.replaceValueInMatch("_type", "_doc")
task.addAllowedWarningRegex("\\[types removal\\].*")
task.addAllowedWarningRegexForTest("Including \\[accept_enterprise\\] in get license.*", "Installing enterprise license")
})
tasks.named("yamlRestCompatTest").configure {
tasks.named("yamlRestTestV7CompatTest").configure {
systemProperty 'tests.rest.blacklist', [
// to support it, it would require to almost revert back the #48725 and complicate the code
'vectors/10_dense_vector_basic/Deprecated function signature',
Expand Down Expand Up @@ -207,8 +210,3 @@ tasks.named("precommit").configure {
dependsOn 'enforceYamlTestConvention', 'enforceApiSpecsConvention'
}

tasks.named("transformV7RestTests").configure({ task ->
task.replaceValueInMatch("_type", "_doc")
task.addAllowedWarningRegex("\\[types removal\\].*")
task.addAllowedWarningRegexForTest("Including \\[accept_enterprise\\] in get license.*", "Installing enterprise license")
})
2 changes: 1 addition & 1 deletion x-pack/plugin/watcher/qa/rest/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ if (BuildParams.inFipsJvm){
tasks.named("yamlRestTest").configure{enabled = false }
}

tasks.named("transformV7RestTests").configure({ task ->
tasks.named("yamlRestTestV7CompatTransform").configure({ task ->
task.replaceKeyInDo("watcher.ack_watch", "xpack-watcher.ack_watch")
task.replaceKeyInDo("watcher.activate_watch", "xpack-watcher.activate_watch")
task.replaceKeyInDo("watcher.deactivate_watch", "xpack-watcher.deactivate_watch")
Expand Down

0 comments on commit 26dfe02

Please sign in to comment.