AWS, Core: Replace .withFailMessage() usage with .as() (apache#10000)

We almost never want to use `.withFailMessage()` as that will override
any enriched contextual information that AssertJ generally provides
about actual/expected. It's better to use `.as()` to add some
description to the check being done, which allows to still show
contextual information about actual/expected if the assertion ever
fails.

aws/src/test/java/org/apache/iceberg/aws/AwsClientPropertiesTest.java

@@ -46,7 +46,7 @@ public void testApplyClientRegion() {
     Mockito.verify(mockS3ClientBuilder).region(regionArgumentCaptor.capture());
     Region region = regionArgumentCaptor.getValue();
     Assertions.assertThat(region.id())
-        .withFailMessage("region parameter should match what is set in CLIENT_REGION")
+        .as("region parameter should match what is set in CLIENT_REGION")
         .isEqualTo("us-east-1");
   }
 
@@ -56,9 +56,9 @@ public void testDefaultCredentialsConfiguration() {
     AwsCredentialsProvider credentialsProvider =
         awsClientProperties.credentialsProvider(null, null, null);
 
-    Assertions.assertThat(credentialsProvider instanceof DefaultCredentialsProvider)
-        .withFailMessage("Should use default credentials if nothing is set")
-        .isTrue();
+    Assertions.assertThat(credentialsProvider)
+        .as("Should use default credentials if nothing is set")
+        .isInstanceOf(DefaultCredentialsProvider.class);
   }
 
   @Test
@@ -70,7 +70,7 @@ public void testCreatesNewInstanceOfDefaultCredentialsConfiguration() {
         awsClientProperties.credentialsProvider(null, null, null);
 
     Assertions.assertThat(credentialsProvider)
-        .withFailMessage("Should create a new instance in each call")
+        .as("Should create a new instance in each call")
         .isNotSameAs(credentialsProvider2);
   }
 
@@ -81,17 +81,15 @@ public void testBasicCredentialsConfiguration() {
     AwsCredentialsProvider credentialsProvider =
         awsClientProperties.credentialsProvider("key", "secret", null);
 
-    Assertions.assertThat(credentialsProvider.resolveCredentials() instanceof AwsBasicCredentials)
-        .withFailMessage(
-            "Should use basic credentials if access key ID and secret access key are set")
-        .isTrue();
+    Assertions.assertThat(credentialsProvider.resolveCredentials())
+        .as("Should use basic credentials if access key ID and secret access key are set")
+        .isInstanceOf(AwsBasicCredentials.class);
     Assertions.assertThat(credentialsProvider.resolveCredentials().accessKeyId())
-        .withFailMessage("The access key id should be the same as the one set by tag ACCESS_KEY_ID")
+        .as("The access key id should be the same as the one set by tag ACCESS_KEY_ID")
         .isEqualTo("key");
 
     Assertions.assertThat(credentialsProvider.resolveCredentials().secretAccessKey())
-        .withFailMessage(
-            "The secret access key should be the same as the one set by tag SECRET_ACCESS_KEY")
+        .as("The secret access key should be the same as the one set by tag SECRET_ACCESS_KEY")
         .isEqualTo("secret");
   }
 
@@ -102,15 +100,14 @@ public void testSessionCredentialsConfiguration() {
     AwsCredentialsProvider credentialsProvider =
         awsClientProperties.credentialsProvider("key", "secret", "token");
 
-    Assertions.assertThat(credentialsProvider.resolveCredentials() instanceof AwsSessionCredentials)
-        .withFailMessage("Should use session credentials if session token is set")
-        .isTrue();
+    Assertions.assertThat(credentialsProvider.resolveCredentials())
+        .as("Should use session credentials if session token is set")
+        .isInstanceOf(AwsSessionCredentials.class);
     Assertions.assertThat(credentialsProvider.resolveCredentials().accessKeyId())
-        .withFailMessage("The access key id should be the same as the one set by tag ACCESS_KEY_ID")
+        .as("The access key id should be the same as the one set by tag ACCESS_KEY_ID")
         .isEqualTo("key");
     Assertions.assertThat(credentialsProvider.resolveCredentials().secretAccessKey())
-        .withFailMessage(
-            "The secret access key should be the same as the one set by tag SECRET_ACCESS_KEY")
+        .as("The secret access key should be the same as the one set by tag SECRET_ACCESS_KEY")
         .isEqualTo("secret");
   }
 }

aws/src/test/java/org/apache/iceberg/aws/HttpClientPropertiesTest.java

@@ -45,9 +45,9 @@ public void testUrlHttpClientConfiguration() {
     Mockito.verify(mockS3ClientBuilder).httpClientBuilder(httpClientBuilderCaptor.capture());
     SdkHttpClient.Builder capturedHttpClientBuilder = httpClientBuilderCaptor.getValue();
 
-    Assertions.assertThat(capturedHttpClientBuilder instanceof UrlConnectionHttpClient.Builder)
-        .withFailMessage("Should use url connection http client")
-        .isTrue();
+    Assertions.assertThat(capturedHttpClientBuilder)
+        .as("Should use url connection http client")
+        .isInstanceOf(UrlConnectionHttpClient.Builder.class);
   }
 
   @Test
@@ -62,9 +62,9 @@ public void testApacheHttpClientConfiguration() {
     httpClientProperties.applyHttpClientConfigurations(mockS3ClientBuilder);
     Mockito.verify(mockS3ClientBuilder).httpClientBuilder(httpClientBuilderCaptor.capture());
     SdkHttpClient.Builder capturedHttpClientBuilder = httpClientBuilderCaptor.getValue();
-    Assertions.assertThat(capturedHttpClientBuilder instanceof ApacheHttpClient.Builder)
-        .withFailMessage("Should use apache http client")
-        .isTrue();
+    Assertions.assertThat(capturedHttpClientBuilder)
+        .as("Should use apache http client")
+        .isInstanceOf(ApacheHttpClient.Builder.class);
   }
 
   @Test

aws/src/test/java/org/apache/iceberg/aws/TestS3FileIOAwsClientFactories.java

@@ -35,7 +35,7 @@ public void testS3FileIOImplCatalogPropertyDefined() {
         "org.apache.iceberg.aws.s3.DefaultS3FileIOAwsClientFactory");
     Object factoryImpl = S3FileIOAwsClientFactories.initialize(properties);
     Assertions.assertThat(factoryImpl)
-        .withFailMessage(
+        .as(
             "should instantiate an object of type S3FileIOAwsClientFactory when s3.client-factory-impl is set")
         .isInstanceOf(S3FileIOAwsClientFactory.class);
   }
@@ -46,7 +46,7 @@ public void testS3FileIOImplCatalogPropertyNotDefined() {
     Map<String, String> properties = Maps.newHashMap();
     Object factoryImpl = S3FileIOAwsClientFactories.initialize(properties);
     Assertions.assertThat(factoryImpl)
-        .withFailMessage(
+        .as(
             "should instantiate an object of type AwsClientFactory when s3.client-factory-impl is not set")
         .isInstanceOf(AwsClientFactory.class);
   }

aws/src/test/java/org/apache/iceberg/aws/s3/TestS3FileIOProperties.java

@@ -447,13 +447,13 @@ public void testApplyS3ServiceConfigurations() {
 
     S3Configuration s3Configuration = s3ConfigurationCaptor.getValue();
     Assertions.assertThat(s3Configuration.pathStyleAccessEnabled())
-        .withFailMessage("s3 path style access enabled parameter should be set to true")
+        .as("s3 path style access enabled parameter should be set to true")
         .isTrue();
     Assertions.assertThat(s3Configuration.useArnRegionEnabled())
-        .withFailMessage("s3 use arn region enabled parameter should be set to true")
+        .as("s3 use arn region enabled parameter should be set to true")
         .isTrue();
     Assertions.assertThat(s3Configuration.accelerateModeEnabled())
-        .withFailMessage("s3 acceleration mode enabled parameter should be set to true")
+        .as("s3 acceleration mode enabled parameter should be set to true")
         .isFalse();
   }
 

core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java

@@ -380,17 +380,17 @@ public void testListNamespaces() {
 
     catalog.createNamespace(ns1);
     Assertions.assertThat(catalog.listNamespaces())
-        .withFailMessage("Should include newdb_1")
+        .as("Should include newdb_1")
         .hasSameElementsAs(concat(starting, ns1));
 
     catalog.createNamespace(ns2);
     Assertions.assertThat(catalog.listNamespaces())
-        .withFailMessage("Should include newdb_1 and newdb_2")
+        .as("Should include newdb_1 and newdb_2")
         .hasSameElementsAs(concat(starting, ns1, ns2));
 
     catalog.dropNamespace(ns1);
     Assertions.assertThat(catalog.listNamespaces())
-        .withFailMessage("Should include newdb_2, not newdb_1")
+        .as("Should include newdb_2, not newdb_1")
         .hasSameElementsAs(concat(starting, ns2));
 
     catalog.dropNamespace(ns2);
@@ -415,36 +415,34 @@ public void testListNestedNamespaces() {
 
     catalog.createNamespace(parent);
     Assertions.assertThat(catalog.listNamespaces())
-        .withFailMessage("Should include parent")
+        .as("Should include parent")
         .hasSameElementsAs(concat(starting, parent));
 
     Assertions.assertThat(catalog.listNamespaces(parent))
-        .withFailMessage("Should have no children in newly created parent namespace")
+        .as("Should have no children in newly created parent namespace")
         .isEmpty();
 
     catalog.createNamespace(child1);
     Assertions.assertThat(catalog.listNamespaces(parent))
-        .withFailMessage("Should include child1")
+        .as("Should include child1")
         .hasSameElementsAs(ImmutableList.of(child1));
 
     catalog.createNamespace(child2);
     Assertions.assertThat(catalog.listNamespaces(parent))
-        .withFailMessage("Should include child1 and child2")
+        .as("Should include child1 and child2")
         .hasSameElementsAs(ImmutableList.of(child1, child2));
 
     Assertions.assertThat(catalog.listNamespaces())
-        .withFailMessage("Should not change listing the root")
+        .as("Should not change listing the root")
         .hasSameElementsAs(concat(starting, parent));
 
     catalog.dropNamespace(child1);
     Assertions.assertThat(catalog.listNamespaces(parent))
-        .withFailMessage("Should include only child2")
+        .as("Should include only child2")
         .hasSameElementsAs(ImmutableList.of(child2));
 
     catalog.dropNamespace(child2);
-    Assertions.assertThat(catalog.listNamespaces(parent))
-        .withFailMessage("Should be empty")
-        .isEmpty();
+    Assertions.assertThat(catalog.listNamespaces(parent)).as("Should be empty").isEmpty();
   }
 
   @Test