From dd859b66438deb5175cd8b93933c2c761e5d2e38 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 12 Oct 2022 14:30:34 -0400 Subject: [PATCH 1/3] Run aggs tests in aggs module Run the aggregations tests v7 compat tests against the aggregations module and *not* the `rest-api-spec` module. This allows us to drop `rest-api-spec`'s dependency on the aggregations module and keep it "just the server" which is nice. There are a few side effects here that are ok: 1. We run all aggregations REST tests in the aggregations module. Even the ones in `rest-api-spec`. This means we run them twice. We plan to move all of the aggregations REST tests into the aggregations module anyway. 2. We now bundle the REST tests in the aggregations module into the tests that the clients run for their verification step. This should keep our clients from losing coverage. --- .../rest/compat/YamlRestCompatTestPlugin.java | 7 ++++++- modules/aggregations/build.gradle | 19 +++++++++++++++++++ rest-api-spec/build.gradle | 8 +------- x-pack/rest-resources-zip/build.gradle | 1 + 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/rest/compat/YamlRestCompatTestPlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/rest/compat/YamlRestCompatTestPlugin.java index 534ac6f458e8a..b352ee0bcd260 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/rest/compat/YamlRestCompatTestPlugin.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/rest/compat/YamlRestCompatTestPlugin.java @@ -58,6 +58,7 @@ public class YamlRestCompatTestPlugin implements Plugin { 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"); + private static final Path RELATIVE_REST_CORE = Path.of("rest-api-spec"); private static final Path RELATIVE_REST_XPACK_RESOURCES = Path.of("x-pack/plugin/src/test/resources"); private static final Path RELATIVE_REST_PROJECT_RESOURCES = Path.of("src/yamlRestTest/resources"); private static final int COMPATIBLE_VERSION = Version.fromString(VersionProperties.getVersions().get("elasticsearch")).getMajor() - 1; @@ -142,7 +143,11 @@ public void apply(Project project) { task.getOutputResourceDir().set(projectLayout.getBuildDirectory().dir(compatTestsDir.resolve("original").toString())); task.setCoreConfigToFileTree( config -> fileOperations.fileTree( - config.getSingleFile().toPath().resolve(RELATIVE_REST_API_RESOURCES).resolve(RELATIVE_TEST_PATH) + config.getSingleFile() + .toPath() + .resolve(RELATIVE_REST_CORE) + .resolve(RELATIVE_REST_PROJECT_RESOURCES) + .resolve(RELATIVE_TEST_PATH) ) ); task.setXpackConfigToFileTree( diff --git a/modules/aggregations/build.gradle b/modules/aggregations/build.gradle index 8fe127b28ea5e..0e8287dfbbbed 100644 --- a/modules/aggregations/build.gradle +++ b/modules/aggregations/build.gradle @@ -18,4 +18,23 @@ restResources { restApi { include '_common', 'indices', 'cluster', 'index', 'search', 'nodes', 'bulk' } + restTests { + // Pulls in all aggregation tests from core AND the forwards v7's core for forwards compatibility + includeCore 'search.aggregation' + } +} + +artifacts { + restTests(new File(projectDir, "src/yamlRestTest/resources/rest-api-spec/test")) +} + +tasks.named("yamlRestTestV7CompatTransform").configure { task -> + task.skipTest("search.aggregation/20_terms/string profiler via global ordinals filters implementation", "The profiler results aren't backwards compatible.") + task.skipTest("search.aggregation/20_terms/string profiler via global ordinals native implementation", "The profiler results aren't backwards compatible.") + task.skipTest("search.aggregation/20_terms/string profiler via map", "The profiler results aren't backwards compatible.") + task.skipTest("search.aggregation/20_terms/numeric profiler", "The profiler results aren't backwards compatible.") + task.skipTest("search.aggregation/210_top_hits_nested_metric/top_hits aggregation with sequence numbers", "#42809 the use nested path and filter sort throws an exception") + task.skipTest("search.aggregation/370_doc_count_field/Test filters agg with doc_count", "Uses profiler for assertions which is not backwards compatible") + + task.addAllowedWarningRegex("\\[types removal\\].*") } diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index 537b915bad326..831599b8287a2 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -38,7 +38,6 @@ testClusters.configureEach { // This dependency can be removed when aggregations and // respective yaml test have been moved to aggregations module. // See for the progress: https://github.com/elastic/elasticsearch/issues/90283 - module ':modules:aggregations' requiresFeature 'es.index_mode_feature_flag_registered', Version.fromString("8.0.0") } @@ -47,6 +46,7 @@ tasks.named("yamlRestTestV7CompatTransform").configure { task -> task.skipTestsByFilePattern("**/cat*/*.yml", "Cat API are meant to be consumed by humans, so will not be supported by Compatible REST API") task.skipTestsByFilePattern("**/indices.upgrade/*.yml", "upgrade api will only get a dummy endpoint returning an exception suggesting to use _reindex") task.skipTestsByFilePattern("**/indices.stats/60_field_usage/*/*.yml", "field usage results will be different between lucene versions") + task.skipTestsByFilePattern("**/search.aggregation/*.yml", "run by the aggregation module") task.skipTest("bulk/11_dynamic_templates/Dynamic templates", "Error message has changed") task.skipTest("indices.create/20_mix_typeless_typeful/Implicitly create a typed index while there is a typeless template", "Type information about the type is removed and not passed down. The logic to check for this is also removed.") @@ -70,18 +70,12 @@ tasks.named("yamlRestTestV7CompatTransform").configure { task -> task.skipTest("indices.stats/20_translog/Translog retention settings are deprecated", "translog settings removal is not supported under compatible api") task.skipTest("indices.stats/20_translog/Translog retention without soft_deletes", "translog settings removal is not supported under compatible api") task.skipTest("indices.stats/20_translog/Translog stats on closed indices without soft-deletes", "translog settings removal is not supported under compatible api") - task.skipTest("search.aggregation/370_doc_count_field/Test filters agg with doc_count", "Uses profiler for assertions which is not backwards compatible") task.skipTest("indices.create/10_basic/Create index without soft deletes", "Make soft-deletes mandatory in 8.0 #51122 - settings changes are note supported in Rest Api compatibility") task.skipTest("field_caps/30_filter/Field caps with index filter", "behaviour change after #63692 4digits dates are parsed as epoch and in quotes as year") task.skipTest("indices.forcemerge/10_basic/Check deprecation warning when incompatible only_expunge_deletes and max_num_segments values are both set", "#44761 bug fix") task.skipTest("search/340_type_query/type query", "#47207 type query throws exception in compatible mode") - task.skipTest("search.aggregation/210_top_hits_nested_metric/top_hits aggregation with sequence numbers", "#42809 the use nested path and filter sort throws an exception") task.skipTest("search/310_match_bool_prefix/multi_match multiple fields with cutoff_frequency throws exception", "#42654 cutoff_frequency, common terms are not supported. Throwing an exception") task.skipTest("search_shards/10_basic/Search shards aliases with and without filters", "Filter representation no longer outputs default boosts") - task.skipTest("search.aggregation/20_terms/string profiler via global ordinals filters implementation", "The profiler results aren't backwards compatible.") - task.skipTest("search.aggregation/20_terms/string profiler via global ordinals native implementation", "The profiler results aren't backwards compatible.") - task.skipTest("search.aggregation/20_terms/string profiler via map", "The profiler results aren't backwards compatible.") - task.skipTest("search.aggregation/20_terms/numeric profiler", "The profiler results aren't backwards compatible.") task.skipTest("migration/10_get_feature_upgrade_status/Get feature upgrade status", "Awaits backport") task.skipTest("search/330_fetch_fields/Test disable source", "Error no longer thrown") task.skipTest("search/240_date_nanos/doc value fields are working as expected across date and date_nanos fields", "Fetching docvalues field multiple times is no longer allowed") diff --git a/x-pack/rest-resources-zip/build.gradle b/x-pack/rest-resources-zip/build.gradle index 75477db89827e..2ac8bd65ddc36 100644 --- a/x-pack/rest-resources-zip/build.gradle +++ b/x-pack/rest-resources-zip/build.gradle @@ -19,6 +19,7 @@ configurations { dependencies { apis project(path: ':rest-api-spec', configuration: 'restSpecs') freeTests project(path: ':rest-api-spec', configuration: 'restTests') + freeTests project(path: ':modules:aggregations', configuration: 'restTests') compatApis project(path: ':rest-api-spec', configuration: 'restCompatSpecs') compatApis project(path: ':x-pack:plugin', configuration: 'restCompatSpecs') freeCompatTests project(path: ':rest-api-spec', configuration: 'restCompatTests') From e59d77ba5cf3df8bb683148ec0e700b863174be5 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 12 Oct 2022 14:46:57 -0400 Subject: [PATCH 2/3] Fix doc --- rest-api-spec/build.gradle | 3 --- 1 file changed, 3 deletions(-) diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index 831599b8287a2..ddf78b4301ae8 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -35,9 +35,6 @@ artifacts { testClusters.configureEach { module ':modules:mapper-extras' - // This dependency can be removed when aggregations and - // respective yaml test have been moved to aggregations module. - // See for the progress: https://github.com/elastic/elasticsearch/issues/90283 requiresFeature 'es.index_mode_feature_flag_registered', Version.fromString("8.0.0") } From 51dabd4feac5a7c1522407b7f8100f0b92922ff7 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 12 Oct 2022 14:47:53 -0400 Subject: [PATCH 3/3] Better grouping --- modules/aggregations/build.gradle | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/aggregations/build.gradle b/modules/aggregations/build.gradle index 0e8287dfbbbed..ae639be3db4af 100644 --- a/modules/aggregations/build.gradle +++ b/modules/aggregations/build.gradle @@ -24,10 +24,6 @@ restResources { } } -artifacts { - restTests(new File(projectDir, "src/yamlRestTest/resources/rest-api-spec/test")) -} - tasks.named("yamlRestTestV7CompatTransform").configure { task -> task.skipTest("search.aggregation/20_terms/string profiler via global ordinals filters implementation", "The profiler results aren't backwards compatible.") task.skipTest("search.aggregation/20_terms/string profiler via global ordinals native implementation", "The profiler results aren't backwards compatible.") @@ -38,3 +34,7 @@ tasks.named("yamlRestTestV7CompatTransform").configure { task -> task.addAllowedWarningRegex("\\[types removal\\].*") } + +artifacts { + restTests(new File(projectDir, "src/yamlRestTest/resources/rest-api-spec/test")) +}