[Rest Api Compatibility] Allow transforming warnings per test (elasti…

…c#75187) Warning related transformations missed the possibility to apply per single test only. Also a warning changed in elastic#67158 for indices.close so this PR also applies the transformation for 7.x test relates elastic#51816
masseyke · Jul 16, 2021 · a9f50d1 · a9f50d1
1 parent 2275cd8
commit a9f50d1
Show file tree

Hide file tree

Showing 11 changed files with 132 additions and 21 deletions.
diff --git a/.../main/java/org/elasticsearch/gradle/internal/rest/compat/RestCompatTestTransformTask.java b/.../main/java/org/elasticsearch/gradle/internal/rest/compat/RestCompatTestTransformTask.java
@@ -282,6 +282,15 @@ public void removeWarning(String... warnings) {
         transformations.add(new RemoveWarnings(Set.copyOf(Arrays.asList(warnings))));
     }
 
+    /**
+     * Removes one or more warnings
+     * @param warnings the warning(s) to remove
+     * @param testName the test name to remove the warning
+     */
+    public void removeWarningForTest(String warnings, String testName) {
+        transformations.add(new RemoveWarnings(Set.copyOf(Arrays.asList(warnings)), testName));
+    }
+
     /**
      * Adds one or more allowed warnings
      * @param allowedWarnings the warning(s) to add
@@ -298,6 +307,15 @@ public void addAllowedWarningRegex(String... allowedWarningsRegex) {
         transformations.add(new InjectAllowedWarnings(true, Arrays.asList(allowedWarningsRegex)));
     }
 
+    /**
+     * Adds one or more allowed regular expression warnings
+     * @param allowedWarningsRegex the regex warning(s) to add
+     * @testName the test name to add a allowedWarningRegex
+     */
+    public void addAllowedWarningRegexForTest(String allowedWarningsRegex, String testName) {
+        transformations.add(new InjectAllowedWarnings(true, Arrays.asList(allowedWarningsRegex), testName));
+    }
+
     @OutputDirectory
     public DirectoryProperty getOutputDirectory() {
         return outputDirectory;

diff --git a/...org/elasticsearch/gradle/internal/test/rest/transform/warnings/InjectAllowedWarnings.java b/...org/elasticsearch/gradle/internal/test/rest/transform/warnings/InjectAllowedWarnings.java
@@ -11,10 +11,13 @@
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import com.fasterxml.jackson.databind.node.ObjectNode;
+
+import org.elasticsearch.gradle.internal.test.rest.transform.RestTestContext;
 import org.elasticsearch.gradle.internal.test.rest.transform.RestTestTransformByParentObject;
 import org.elasticsearch.gradle.internal.test.rest.transform.feature.FeatureInjector;
 import org.gradle.api.tasks.Input;
 import org.gradle.api.tasks.Internal;
+import org.gradle.api.tasks.Optional;
 
 import java.util.List;
 
@@ -26,6 +29,7 @@ public class InjectAllowedWarnings extends FeatureInjector implements RestTestTr
     private static JsonNodeFactory jsonNodeFactory = JsonNodeFactory.withExactBigDecimals(false);
 
     private final List<String> allowedWarnings;
+    private String testName;
     private final boolean isRegex;
 
     /**
@@ -40,8 +44,18 @@ public InjectAllowedWarnings(List<String> allowedWarnings) {
      * @param allowedWarnings The allowed warnings to inject
      */
     public InjectAllowedWarnings(boolean isRegex, List<String> allowedWarnings) {
+        this(isRegex, allowedWarnings, null);
+    }
+
+    /**
+     * @param isRegex true if should inject the regex variant of allowed warnings
+     * @param allowedWarnings The allowed warnings to inject
+     * @param testName The testName to inject
+     */
+    public InjectAllowedWarnings(boolean isRegex, List<String> allowedWarnings, String testName) {
         this.isRegex = isRegex;
         this.allowedWarnings = allowedWarnings;
+        this.testName = testName;
     }
 
     @Override
@@ -52,7 +66,7 @@ public void transformTest(ObjectNode doNodeParent) {
             arrayWarnings = new ArrayNode(jsonNodeFactory);
             doNodeValue.set(getSkipFeatureName(), arrayWarnings);
         }
-        allowedWarnings.forEach(arrayWarnings::add);
+        this.allowedWarnings.forEach(arrayWarnings::add);
     }
 
     @Override
@@ -71,4 +85,15 @@ public String getSkipFeatureName() {
     public List<String> getAllowedWarnings() {
         return allowedWarnings;
     }
+
+    @Override
+    public boolean shouldApply(RestTestContext testContext) {
+        return testName == null || testContext.getTestName().equals(testName);
+    }
+
+    @Input
+    @Optional
+    public String getTestName() {
+        return testName;
+    }
 }
diff --git a/...n/java/org/elasticsearch/gradle/internal/test/rest/transform/warnings/RemoveWarnings.java b/...n/java/org/elasticsearch/gradle/internal/test/rest/transform/warnings/RemoveWarnings.java
@@ -10,9 +10,12 @@
 
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
+
+import org.elasticsearch.gradle.internal.test.rest.transform.RestTestContext;
 import org.elasticsearch.gradle.internal.test.rest.transform.RestTestTransformByParentObject;
 import org.gradle.api.tasks.Input;
 import org.gradle.api.tasks.Internal;
+import org.gradle.api.tasks.Optional;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -28,13 +31,22 @@
 public class RemoveWarnings implements RestTestTransformByParentObject {
 
     private final Set<String> warnings;
+    private String testName;
 
     /**
      * @param warnings The allowed warnings to inject
      */
     public RemoveWarnings(Set<String> warnings) {
         this.warnings = warnings;
     }
+    /**
+     * @param warnings The allowed warnings to inject
+     * @param testName The testName to inject
+     */
+    public RemoveWarnings(Set<String> warnings, String testName) {
+        this.warnings = warnings;
+        this.testName = testName;
+    }
 
     @Override
     public void transformTest(ObjectNode doNodeParent) {
@@ -66,4 +78,15 @@ public String getKeyToFind() {
     public Set<String> getWarnings() {
         return warnings;
     }
+
+    @Override
+    public boolean shouldApply(RestTestContext testContext) {
+        return testName == null || testContext.getTestName().equals(testName);
+    }
+
+    @Input
+    @Optional
+    public String getTestName() {
+        return testName;
+    }
 }
diff --git a/...l/src/test/java/org/elasticsearch/gradle/internal/test/rest/transform/TransformTests.java b/...l/src/test/java/org/elasticsearch/gradle/internal/test/rest/transform/TransformTests.java
@@ -17,24 +17,25 @@
 import com.fasterxml.jackson.databind.node.TextNode;
 import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
 import com.fasterxml.jackson.dataformat.yaml.YAMLParser;
+
 import org.elasticsearch.gradle.internal.test.GradleUnitTestCase;
 import org.elasticsearch.gradle.internal.test.rest.transform.headers.InjectHeaders;
 import org.hamcrest.CoreMatchers;
+import org.hamcrest.Matchers;
 import org.hamcrest.core.IsCollectionContaining;
 import org.junit.Before;
 
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.LongAdder;
-import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
 public abstract class TransformTests extends GradleUnitTestCase {
@@ -181,11 +182,11 @@ protected ObjectNode getSkipNode(ArrayNode setupNodeValue) {
         return null;
     }
 
-    protected void validateBodyHasWarnings(String featureName, List<ObjectNode> tests, Set<String> expectedWarnings) {
+    protected void validateBodyHasWarnings(String featureName, List<ObjectNode> tests, Collection<String> expectedWarnings) {
         validateBodyHasWarnings(featureName, null, tests, expectedWarnings);
     }
 
-    protected void validateBodyHasWarnings(String featureName, String testName, List<ObjectNode> tests, Set<String> expectedWarnings) {
+    protected void validateBodyHasWarnings(String featureName, String testName, List<ObjectNode> tests, Collection<String> expectedWarnings) {
         AtomicBoolean actuallyDidSomething = new AtomicBoolean(false);
         tests.forEach(test -> {
             Iterator<Map.Entry<String, JsonNode>> testsIterator = test.fields();
@@ -201,13 +202,10 @@ protected void validateBodyHasWarnings(String featureName, String testName, List
                             ObjectNode doSection = (ObjectNode) testSection.get("do");
                             assertThat(doSection.get(featureName), CoreMatchers.notNullValue());
                             ArrayNode warningsNode = (ArrayNode) doSection.get(featureName);
-                            LongAdder assertions = new LongAdder();
-                            warningsNode.forEach(warning -> {
-                                if (expectedWarnings.contains(warning.asText())) {
-                                    assertions.increment();
-                                }
-                            });
-                            assertThat(assertions.intValue(), CoreMatchers.equalTo(expectedWarnings.size()));
+                            List<String> actual  = new ArrayList<>();
+                            warningsNode.forEach(node -> actual.add(node.asText()));
+                            String[] expected = expectedWarnings.toArray(new String[]{});
+                            assertThat(actual, Matchers.containsInAnyOrder(expected));
                             actuallyDidSomething.set(true);
                         }
                     });

diff --git a/...csearch/gradle/internal/test/rest/transform/warnings/InjectAllowedWarningsRegexTests.java b/...csearch/gradle/internal/test/rest/transform/warnings/InjectAllowedWarningsRegexTests.java
@@ -49,8 +49,7 @@ public void testInjectAllowedWarningsWithPreExisting() throws Exception {
         List<ObjectNode> transformedTests = transformTests(tests);
         printTest(testName, transformedTests);
         validateSetupAndTearDown(transformedTests);
-        validateBodyHasWarnings(ALLOWED_WARNINGS_REGEX, tests, Set.of("c", "d"));
-        validateBodyHasWarnings(ALLOWED_WARNINGS_REGEX, tests, addWarnings);
+        validateBodyHasWarnings(ALLOWED_WARNINGS_REGEX, tests, Set.of("c", "d", "added warning"));
     }
 
     @Override

diff --git a/...lasticsearch/gradle/internal/test/rest/transform/warnings/InjectAllowedWarningsTests.java b/...lasticsearch/gradle/internal/test/rest/transform/warnings/InjectAllowedWarningsTests.java
@@ -49,8 +49,24 @@ public void testInjectAllowedWarningsWithPreExisting() throws Exception {
         List<ObjectNode> transformedTests = transformTests(tests);
         printTest(testName, transformedTests);
         validateSetupAndTearDown(transformedTests);
+        validateBodyHasWarnings(ALLOWED_WARNINGS, transformedTests, List.of("a", "b", "added warning"));
+    }
+
+    @Test
+    public void testInjectAllowedWarningsWithPreExistingForSingleTest() throws Exception {
+        String testName = "/rest/transform/warnings/with_existing_allowed_warnings.yml";
+        List<ObjectNode> tests = getTests(testName);
+        validateSetupExist(tests);
         validateBodyHasWarnings(ALLOWED_WARNINGS, tests, Set.of("a", "b"));
-        validateBodyHasWarnings(ALLOWED_WARNINGS, tests, addWarnings);
+        List<ObjectNode> transformedTests = transformTests(tests, getTransformationsForTest("Test with existing allowed warnings"));
+        printTest(testName, transformedTests);
+        validateSetupAndTearDown(transformedTests);
+        validateBodyHasWarnings(ALLOWED_WARNINGS, "Test with existing allowed warnings", transformedTests, Set.of("a", "b", "added warning"));
+        validateBodyHasWarnings(ALLOWED_WARNINGS, "Test with existing allowed warnings not to change", transformedTests, Set.of("a", "b"));
+    }
+
+    private List<RestTestTransform<?>> getTransformationsForTest(String testName) {
+        return Collections.singletonList(new InjectAllowedWarnings(false, new ArrayList<>(addWarnings), testName));
     }
 
     @Override

diff --git a/.../elasticsearch/gradle/internal/test/rest/transform/warnings/InjectWarningsRegexTests.java b/.../elasticsearch/gradle/internal/test/rest/transform/warnings/InjectWarningsRegexTests.java
@@ -67,8 +67,8 @@ public void testInjectWarningsWithPreExisting() throws Exception {
         List<ObjectNode> transformedTests = transformTests(tests);
         printTest(testName, transformedTests);
         validateSetupAndTearDown(transformedTests);
-        validateBodyHasWarnings(WARNINGS_REGEX, tests, Set.of("c", "d"));
-        validateBodyHasWarnings(WARNINGS_REGEX, "Test warnings", tests, addWarnings);
+        validateBodyHasWarnings(WARNINGS_REGEX, "Not the test to change", tests, Set.of("c", "d"));
+        validateBodyHasWarnings(WARNINGS_REGEX, "Test warnings", tests, Set.of("c", "d", "added warning"));
     }
 
     @Override

diff --git a/...a/org/elasticsearch/gradle/internal/test/rest/transform/warnings/InjectWarningsTests.java b/...a/org/elasticsearch/gradle/internal/test/rest/transform/warnings/InjectWarningsTests.java
@@ -66,8 +66,8 @@ public void testInjectWarningsWithPreExisting() throws Exception {
         List<ObjectNode> transformedTests = transformTests(tests);
         printTest(testName, transformedTests);
         validateSetupAndTearDown(transformedTests);
-        validateBodyHasWarnings(WARNINGS, tests, Set.of("a", "b"));
-        validateBodyHasWarnings(WARNINGS, "Test warnings", tests, addWarnings);
+        validateBodyHasWarnings(WARNINGS, "Not the test to change", tests, Set.of("a", "b"));
+        validateBodyHasWarnings(WARNINGS, "Test warnings", tests, Set.of("a", "b", "added warning"));
     }
 
     @Override

diff --git a/...a/org/elasticsearch/gradle/internal/test/rest/transform/warnings/RemoveWarningsTests.java b/...a/org/elasticsearch/gradle/internal/test/rest/transform/warnings/RemoveWarningsTests.java
@@ -50,6 +50,23 @@ public void testRemoveWarningWithPreExisting() throws Exception {
         validateBodyHasWarnings(WARNINGS, tests, Set.of("b"));
     }
 
+    @Test
+    public void testRemoveWarningWithPreExistingFromSingleTest() throws Exception {
+        String testName = "/rest/transform/warnings/with_existing_warnings.yml";
+        List<ObjectNode> tests = getTests(testName);
+        validateSetupExist(tests);
+        validateBodyHasWarnings(WARNINGS, tests, Set.of("a", "b"));
+        List<ObjectNode> transformedTests = transformTests(tests, getTransformationsForTest("Test warnings"));
+        printTest(testName, transformedTests);
+        validateSetupAndTearDown(transformedTests);
+        validateBodyHasWarnings(WARNINGS, "Test warnings", tests, Set.of("b"));
+        validateBodyHasWarnings(WARNINGS, "Not the test to change", tests, Set.of("a", "b"));
+    }
+
+    private List<RestTestTransform<?>> getTransformationsForTest(String testName) {
+        return Collections.singletonList(new RemoveWarnings(Set.of("a"), testName));
+    }
+
     /**
      * test file has preexisting single warning
      */

diff --git a/...ls-internal/src/test/resources/rest/transform/warnings/with_existing_allowed_warnings.yml b/...ls-internal/src/test/resources/rest/transform/warnings/with_existing_allowed_warnings.yml
@@ -17,3 +17,15 @@ setup:
         id: "something"
   - match: { acknowledged: true }
 
+---
+"Test with existing allowed warnings not to change":
+  - do:
+      allowed_warnings:
+        - "a"
+        - "b"
+      allowed_warnings_regex:
+        - "c"
+        - "d"
+      something:
+        id: "something_else"
+  - match: { acknowledged: true }
diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle
@@ -92,8 +92,6 @@ tasks.named("yamlRestCompatTest").configure {
     'cluster.voting_config_exclusions/10_basic/Throw exception when adding voting config exclusion without specifying nodes',
     'indices.flush/10_basic/Index synced flush rest test',
     'indices.forcemerge/10_basic/Check deprecation warning when incompatible only_expunge_deletes and max_num_segments values are both set',
-    'indices.open/10_basic/?wait_for_active_shards default is deprecated',
-    'indices.open/10_basic/?wait_for_active_shards=index-setting',
     // not fixing this in #70966
     'indices.put_template/11_basic_with_types/Put template with empty mappings',
     'search.aggregation/200_top_hits_metric/top_hits aggregation with sequence numbers',
@@ -224,6 +222,11 @@ tasks.named("transformV7RestTests").configure({ task ->
   )
   task.replaceValueInMatch("_all.primaries.indexing.types._doc.index_total", 2)
 
+  //override for "indices.open/10_basic/?wait_for_active_shards default is deprecated" and "indices.open/10_basic/?wait_for_active_shards=index-setting"
+  task.addAllowedWarningRegexForTest("\\?wait_for_active_shards=index-setting is now the default behaviour.*", "?wait_for_active_shards=index-setting")
+  task.removeWarningForTest("the default value for the ?wait_for_active_shards parameter will change from '0' to 'index-setting' in version 8; " +
+    "specify '?wait_for_active_shards=index-setting' to adopt the future default behaviour, or '?wait_for_active_shards=0' to preserve today's behaviour"
+    , "?wait_for_active_shards default is deprecated")
 })
 
 tasks.register('enforceYamlTestConvention').configure {