Apply review feedback

breskeby · Nov 21, 2023 · f12f5c8 · f12f5c8
1 parent a82bce2
commit f12f5c8
Show file tree

Hide file tree

Showing 8 changed files with 40 additions and 66 deletions.
diff --git a/build-tools-internal/version.properties b/build-tools-internal/version.properties
@@ -44,7 +44,7 @@ mocksocket        = 1.2
 
 # test container dependencies
 testcontainer     = 1.19.2
-docker            = 3.3.4
+dockerJava        = 3.3.4
 ductTape          = 1.0.8
 commonsCompress   = 1.24.0
 

diff --git a/...amlRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ClientYamlTestSuiteIT.java b/...amlRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ClientYamlTestSuiteIT.java
@@ -13,38 +13,27 @@
 import fixture.s3.S3HttpFixtureWithSessionToken;
 
 import com.carrotsearch.randomizedtesting.annotations.Name;
+import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
 
+import org.elasticsearch.core.Booleans;
 import org.elasticsearch.test.cluster.ElasticsearchCluster;
 import org.elasticsearch.test.fixtures.testcontainers.TestContainersThreadFilter;
 import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate;
 import org.junit.ClassRule;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 
-import java.util.List;
-
 @ThreadLeakFilters(filters = { TestContainersThreadFilter.class })
 public class RepositoryS3ClientYamlTestSuiteIT extends AbstractRepositoryS3ClientYamlTestSuiteIT {
+    static final boolean USE_FIXTURE = Booleans.parseBoolean(System.getProperty("tests.use.fixture", "true"));
 
     public static final S3HttpFixture s3Fixture = new S3HttpFixture(USE_FIXTURE);
     public static final S3HttpFixtureWithSessionToken s3HttpFixtureWithSessionToken = new S3HttpFixtureWithSessionToken(USE_FIXTURE);
     public static final S3HttpFixtureWithEC2 s3Ec2 = new S3HttpFixtureWithEC2(USE_FIXTURE);
 
     private static final String s3TemporarySessionToken = "session_token";
 
-    @Override
-    protected List<String> blackListed() {
-        return USE_FIXTURE
-            ? List.of("repository_s3/50_repository_ecs_credentials/*", "repository_s3/60_repository_sts_credentials/*")
-            : List.of(
-                "repository_s3/30_repository_temporary_credentials/*",
-                "repository_s3/40_repository_ec2_credentials/*",
-                "repository_s3/50_repository_ecs_credentials/*",
-                "repository_s3/60_repository_sts_credentials/*"
-            );
-    }
-
     public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
         .module("repository-s3")
         .keystore("s3.client.integration_test_permanent.access_key", System.getProperty("s3PermanentAccessKey"))
@@ -61,6 +50,19 @@ protected List<String> blackListed() {
     @ClassRule
     public static TestRule ruleChain = RuleChain.outerRule(s3Fixture).around(s3Ec2).around(s3HttpFixtureWithSessionToken).around(cluster);
 
+    @ParametersFactory
+    public static Iterable<Object[]> parameters() throws Exception {
+        return createParameters(
+            USE_FIXTURE
+                ? new String[] {
+                    "repository_s3/10_basic",
+                    "repository_s3/20_repository_permanent_credentials",
+                    "repository_s3/30_repository_temporary_credentials",
+                    "repository_s3/40_repository_ec2_credentials" }
+                : new String[] { "repository_s3/10_basic", "repository_s3/20_repository_permanent_credentials" }
+        );
+    }
+
     public RepositoryS3ClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate) {
         super(testCandidate);
     }

diff --git a/...RestTest/java/org/elasticsearch/repositories/s3/RepositoryS3EcsClientYamlTestSuiteIT.java b/...RestTest/java/org/elasticsearch/repositories/s3/RepositoryS3EcsClientYamlTestSuiteIT.java
@@ -11,6 +11,7 @@
 import fixture.s3.S3HttpFixtureWithECS;
 
 import com.carrotsearch.randomizedtesting.annotations.Name;
+import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 
 import org.elasticsearch.test.cluster.ElasticsearchCluster;
 import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate;
@@ -19,8 +20,6 @@
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 
-import java.util.List;
-
 public class RepositoryS3EcsClientYamlTestSuiteIT extends AbstractRepositoryS3ClientYamlTestSuiteIT {
     private static final S3HttpFixtureWithECS s3Ecs = new S3HttpFixtureWithECS(USE_FIXTURE);
 
@@ -38,14 +37,9 @@ public static void onlyWhenRunWithTestFixture() {
         assumeTrue("Only run with fixture enabled", USE_FIXTURE);
     }
 
-    protected List<String> blackListed() {
-        return List.of(
-            "repository_s3/10_basic/*",
-            "repository_s3/20_repository_permanent_credentials/*",
-            "repository_s3/30_repository_temporary_credentials/*",
-            "repository_s3/40_repository_ec2_credentials/*",
-            "repository_s3/60_repository_sts_credentials/*"
-        );
+    @ParametersFactory
+    public static Iterable<Object[]> parameters() throws Exception {
+        return createParameters(new String[] { "repository_s3/50_repository_ecs_credentials" });
     }
 
     public RepositoryS3EcsClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate) {

diff --git a/...stTest/java/org/elasticsearch/repositories/s3/RepositoryS3MinioClientYamlTestSuiteIT.java b/...stTest/java/org/elasticsearch/repositories/s3/RepositoryS3MinioClientYamlTestSuiteIT.java
@@ -9,6 +9,7 @@
 package org.elasticsearch.repositories.s3;
 
 import com.carrotsearch.randomizedtesting.annotations.Name;
+import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
 
 import org.elasticsearch.test.cluster.ElasticsearchCluster;
@@ -20,8 +21,6 @@
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 
-import java.util.List;
-
 @ThreadLeakFilters(filters = { TestContainersThreadFilter.class })
 public class RepositoryS3MinioClientYamlTestSuiteIT extends AbstractRepositoryS3ClientYamlTestSuiteIT {
 
@@ -42,13 +41,9 @@ public static void onlyWhenRunWithTestFixture() {
         assumeTrue("Only run with fixture enabled", USE_FIXTURE);
     }
 
-    protected List<String> blackListed() {
-        return List.of(
-            "repository_s3/30_repository_temporary_credentials/*",
-            "repository_s3/40_repository_ec2_credentials/*",
-            "repository_s3/50_repository_ecs_credentials/*",
-            "repository_s3/60_repository_sts_credentials/*"
-        );
+    @ParametersFactory
+    public static Iterable<Object[]> parameters() throws Exception {
+        return createParameters(new String[] { "repository_s3/10_basic", "repository_s3/20_repository_permanent_credentials" });
     }
 
     public RepositoryS3MinioClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate) {

diff --git a/.../java/org/elasticsearch/repositories/s3/RepositoryS3RegionalStsClientYamlTestSuiteIT.java b/.../java/org/elasticsearch/repositories/s3/RepositoryS3RegionalStsClientYamlTestSuiteIT.java
@@ -9,15 +9,14 @@
 package org.elasticsearch.repositories.s3;
 
 import com.carrotsearch.randomizedtesting.annotations.Name;
+import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 
 import org.elasticsearch.test.cluster.ElasticsearchCluster;
 import org.elasticsearch.test.cluster.util.resource.Resource;
 import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 
-import java.util.List;
-
 public class RepositoryS3RegionalStsClientYamlTestSuiteIT extends AbstractRepositoryS3ClientYamlTestSuiteIT {
     @ClassRule
     public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
@@ -37,14 +36,9 @@ public static void onlyWhenRunWithTestFixture() {
         assumeTrue("Only run with fixture enabled", USE_FIXTURE);
     }
 
-    protected List<String> blackListed() {
-        return List.of(
-            "repository_s3/20_repository_permanent_credentials/*",
-            "repository_s3/30_repository_temporary_credentials/*",
-            "repository_s3/40_repository_ec2_credentials/*",
-            "repository_s3/50_repository_ecs_credentials/*",
-            "repository_s3/60_repository_sts_credentials/*"
-        );
+    @ParametersFactory
+    public static Iterable<Object[]> parameters() throws Exception {
+        return createParameters(new String[] { "repository_s3/10_basic" });
     }
 
     public RepositoryS3RegionalStsClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate) {

diff --git a/...RestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsClientYamlTestSuiteIT.java b/...RestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsClientYamlTestSuiteIT.java
@@ -12,7 +12,9 @@
 import fixture.s3.S3HttpFixtureWithSTS;
 
 import com.carrotsearch.randomizedtesting.annotations.Name;
+import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 
+import org.elasticsearch.core.Booleans;
 import org.elasticsearch.test.cluster.ElasticsearchCluster;
 import org.elasticsearch.test.cluster.util.resource.Resource;
 import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate;
@@ -21,9 +23,9 @@
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 
-import java.util.List;
-
 public class RepositoryS3StsClientYamlTestSuiteIT extends AbstractRepositoryS3ClientYamlTestSuiteIT {
+    static final boolean USE_FIXTURE = Booleans.parseBoolean(System.getProperty("tests.use.fixture", "true"));
+
     public static final S3HttpFixture s3Fixture = new S3HttpFixture(USE_FIXTURE);
     private static final S3HttpFixtureWithSTS s3Sts = new S3HttpFixtureWithSTS(USE_FIXTURE);
 
@@ -47,14 +49,9 @@ public static void onlyWhenRunWithTestFixture() {
         assumeTrue("Only run with fixture enabled", USE_FIXTURE);
     }
 
-    protected List<String> blackListed() {
-        return List.of(
-            "repository_s3/10_basic/*",
-            "repository_s3/20_repository_permanent_credentials/*",
-            "repository_s3/30_repository_temporary_credentials/*",
-            "repository_s3/40_repository_ec2_credentials/*",
-            "repository_s3/50_repository_ecs_credentials/*"
-        );
+    @ParametersFactory
+    public static Iterable<Object[]> parameters() throws Exception {
+        return createParameters(new String[] { "repository_s3/60_repository_sts_credentials" });
     }
 
     public RepositoryS3StsClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate) {

diff --git a/test/fixtures/minio-fixture/build.gradle b/test/fixtures/minio-fixture/build.gradle
@@ -22,10 +22,10 @@ dependencies {
   api "org.testcontainers:testcontainers:${versions.testcontainer}"
   implementation "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
   implementation "org.slf4j:slf4j-api:${versions.slf4j}"
-  implementation "com.github.docker-java:docker-java-api:${versions.docker}"
-  runtimeOnly "com.github.docker-java:docker-java-transport-zerodep:${versions.docker}"
-  runtimeOnly "com.github.docker-java:docker-java-transport:${versions.docker}"
-  runtimeOnly "com.github.docker-java:docker-java-core:${versions.docker}"
+  implementation "com.github.docker-java:docker-java-api:${versions.dockerJava}"
+  runtimeOnly "com.github.docker-java:docker-java-transport-zerodep:${versions.dockerJava}"
+  runtimeOnly "com.github.docker-java:docker-java-transport:${versions.dockerJava}"
+  runtimeOnly "com.github.docker-java:docker-java-core:${versions.dockerJava}"
   runtimeOnly "org.apache.commons:commons-compress:${versions.commonsCompress}"
   runtimeOnly "org.rnorth.duct-tape:duct-tape:${versions.ductTape}"
   runtimeOnly "org.rnorth.duct-tape:duct-tape:${versions.ductTape}"

diff --git a/...rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java b/...rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java
@@ -162,10 +162,6 @@ public void initAndResetContext() throws Exception {
             for (final String entry : blacklistAdditions) {
                 blacklistPathMatchers.add(new BlacklistedPathPatternMatcher(entry));
             }
-            for (final String entry : blackListed()) {
-                blacklistPathMatchers.add(new BlacklistedPathPatternMatcher(entry));
-            }
-
         }
         assert restTestExecutionContext != null;
         assert adminExecutionContext != null;
@@ -177,10 +173,6 @@ public void initAndResetContext() throws Exception {
         restTestExecutionContext.clear();
     }
 
-    protected List<String> blackListed() {
-        return List.of();
-    }
-
     /**
      * Create the test execution context. Can be overwritten in sub-implementations of the test if the context needs to be modified.
      */