Skip to content

Commit

Permalink
chore(test): Replace try/fail/catch/assert with assertThat (#199)
Browse files Browse the repository at this point in the history
  • Loading branch information
@guicamest
guicamest committed Oct 3, 2023
1 parent 2eb44e4 commit 3f2db3e
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 160 deletions.
12 changes: 2 additions & 10 deletions src/test/java/software/amazon/nio/spi/s3/S3ClientProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,7 @@ public void testGenerateClientWith403Then301ResponsesNoHeader(){
);

// then you should get a NoSuchElement exception when you try to get the header
try {
provider.generateClient("test-bucket", mockClient);
} catch (Exception e) {
assertEquals(NoSuchElementException.class, e.getClass());
}
assertThrows(NoSuchElementException.class, () -> provider.generateClient("test-bucket", mockClient));

final InOrder inOrder = inOrder(mockClient);
inOrder.verify(mockClient).getBucketLocation(any(Consumer.class));
Expand All @@ -190,11 +186,7 @@ public void testGenerateAsyncClientWith403Then301ResponsesNoHeader(){
);

// then you should get a NoSuchElement exception when you try to get the header
try {
provider.generateAsyncClient("test-bucket", mockClient);
} catch (Exception e) {
assertEquals(NoSuchElementException.class, e.getClass());
}
assertThrows(NoSuchElementException.class, () -> provider.generateAsyncClient("test-bucket", mockClient));

final InOrder inOrder = inOrder(mockClient);
inOrder.verify(mockClient).getBucketLocation(any(Consumer.class));
Expand Down
65 changes: 26 additions & 39 deletions src/test/java/software/amazon/nio/spi/s3/S3FileSystemProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package software.amazon.nio.spi.s3;

import java.io.IOException;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
Expand Down Expand Up @@ -57,6 +58,7 @@
import java.util.stream.Collectors;
import org.junit.jupiter.api.AfterEach;

import static org.assertj.core.api.Assertions.assertThatCode;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.lenient;
Expand Down Expand Up @@ -99,13 +101,10 @@ public void newFileSystem() {
//
// A filesystem for pathUri has been created already ;)
//
try {
provider.newFileSystem(URI.create(pathUri));
fail("filesystem created twice!");
} catch (FileSystemAlreadyExistsException x) {
assertTrue(x.getMessage().contains("'foo'"));
}

assertThatCode(() -> provider.newFileSystem(URI.create(pathUri)))
.as("filesystem created twice!")
.isInstanceOf(FileSystemAlreadyExistsException.class)
.hasMessageContaining("'foo'");
//
// New AWS S3 file system
//
Expand All @@ -115,12 +114,10 @@ public void newFileSystem() {
//
// New AWS S3 file system with same bucket but different path
//
try {
provider.newFileSystem(URI.create("s3://foo2/baa2"));
fail("filesystem created twice!");
} catch (FileSystemAlreadyExistsException x) {
assertTrue(x.getMessage().contains("'foo2'"));
}
assertThatCode(() -> provider.newFileSystem(URI.create("s3://foo2/baa2")))
.as("filesystem created twice!")
.isInstanceOf(FileSystemAlreadyExistsException.class)
.hasMessageContaining("'foo2'");
provider.closeFileSystem(fs);
}

Expand All @@ -129,32 +126,22 @@ public void newFileSystemWrongArguments() {
//
// IllegalArgumentException if URI is not good
//
try {
provider.newFileSystem((URI)null);
fail("mising argument check!");
} catch (IllegalArgumentException x) {
assertEquals("uri can not be null", x.getMessage());
}

try {
provider.newFileSystem(URI.create("noscheme"));
fail("mising argument check!");
} catch (IllegalArgumentException x) {
assertEquals(
"invalid uri 'noscheme', please provide an uri as s3://bucket",
x.getMessage()
);
}

try {
provider.newFileSystem(URI.create("s3:///"));
fail("mising argument check!");
} catch (IllegalArgumentException x) {
assertEquals(
"invalid uri 's3:///', please provide an uri as s3://bucket",
x.getMessage()
);
}

assertThatCode(() -> provider.newFileSystem(null))
.as("missing argument check!")
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("uri can not be null");

assertThatCode(() -> provider.newFileSystem(URI.create("noscheme")))
.as("missing argument check!")
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("invalid uri 'noscheme', please provide an uri as s3://bucket");

assertThatCode(() -> provider.newFileSystem(URI.create("s3:///")))
.as("missing argument check!")
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("invalid uri 's3:///', please provide an uri as s3://bucket");

}

@Test
Expand Down
32 changes: 8 additions & 24 deletions src/test/java/software/amazon/nio/spi/s3/S3FileSystemTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import java.util.Iterator;
import java.util.concurrent.CompletableFuture;
import java.util.function.Consumer;

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.BDDAssertions.then;
import org.junit.jupiter.api.AfterEach;
import static org.junit.jupiter.api.Assertions.*;
Expand All @@ -29,7 +31,6 @@
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.services.s3.S3AsyncClient;
import software.amazon.awssdk.services.s3.model.HeadObjectResponse;
import software.amazon.awssdk.services.s3.model.NoSuchBucketException;

@ExtendWith(MockitoExtension.class)
public class S3FileSystemTest {
Expand Down Expand Up @@ -65,12 +66,8 @@ public void close() throws IOException {
s3FileSystem.close();
assertFalse(s3FileSystem.isOpen(), "File system should return false from isOpen when closed has been called");

//
// close() also removes the instance from the provider
//
try {
provider.getFileSystem(s3Uri);
} catch (FileSystemNotFoundException x) {} // OK
// close() should also remove the instance from the provider
assertThrows(FileSystemNotFoundException.class, () -> provider.getFileSystem(s3Uri));
}

@Test
Expand Down Expand Up @@ -142,25 +139,12 @@ public void testGetOpenChannelsIsNotModifiable() {
public void plainInitializationWithError() {
//
// Was want to try a plain initialization (i.e. without any mocks).
// We expect standard cluent to throw an exception because the bucket
// is not found
//
// We expect standard client to throw an exception due to missing credentials
final Path path = Paths.get(URI.create("s3://does-not-exists-" + System.currentTimeMillis() + "/dir"));
then(path).isInstanceOf(S3Path.class);
try {
then(Files.exists(path)).isFalse();
fail("client should fail...");
} catch (NoSuchBucketException x) {
//
// we get here is we have the network
//
then(x).hasMessageStartingWith("The specified bucket does not exist");
} catch (SdkClientException x) {
//
// or here if we don't
//

}
assertThatThrownBy(() -> Files.exists(path))
.isInstanceOf(SdkClientException.class)
.hasMessageStartingWith("Unable to load credentials");
}

@Test
Expand Down
64 changes: 27 additions & 37 deletions src/test/java/software/amazon/nio/spi/s3/config/S3NioSpiConfigurationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@

import static com.github.stefanbirkner.systemlambda.SystemLambda.restoreSystemProperties;
import static com.github.stefanbirkner.systemlambda.SystemLambda.withEnvironmentVariable;

import java.util.HashMap;
import java.util.Map;
import java.util.Properties;

import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.BDDAssertions.entry;

import static org.assertj.core.api.BDDAssertions.then;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.fail;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
Expand Down Expand Up @@ -105,12 +107,10 @@ public void withAndGetEndpoint() {
then(config.withEndpoint(null).getEndpoint()).isEqualTo("");
then(config.withEndpoint("noport.somewhere.com").getEndpoint()).isEqualTo("noport.somewhere.com");

try {
config.withEndpoint("wrongport.somewhere.com:aabbcc");
fail("missing sanity check");
} catch (IllegalArgumentException x) {
then(x).hasMessage("endpoint 'wrongport.somewhere.com:aabbcc' does not match format host:port where port is a number");
}
assertThatCode(() -> config.withEndpoint("wrongport.somewhere.com:aabbcc"))
.as("missing sanity check")
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("endpoint 'wrongport.somewhere.com:aabbcc' does not match format host:port where port is a number");
}

@Test
Expand All @@ -121,38 +121,32 @@ public void withAndGetEndpointProtocol() {
then(overriddenConfig.getEndpointProtocol()).isEqualTo("https");
then(badOverriddenConfig.getEndpointProtocol()).isEqualTo(S3_SPI_ENDPOINT_PROTOCOL_DEFAULT);

try {
config.withEndpointProtocol("ftp");
fail("missing sanity check");
} catch (IllegalArgumentException x) {
then(x).hasMessage("endpoint prococol must be one of ('http', 'https')");
}
assertThatCode(() -> config.withEndpointProtocol("ftp"))
.as("missing sanity check")
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("endpoint prococol must be one of ('http', 'https')");
}

@Test
public void withAndGetMaxFragmentNumber() {
then(config.withMaxFragmentNumber(1000)).isSameAs(config);
then(config.getMaxFragmentNumber()).isEqualTo(1000);

try {
config.withMaxFragmentNumber(-1);
fail("missing sanity check");
} catch (IllegalArgumentException x) {
then(x).hasMessage("maxFragmentNumber must be positive");
}
assertThatCode(() -> config.withMaxFragmentNumber(-1))
.as("missing sanity check")
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("maxFragmentNumber must be positive");
}

@Test
public void withAndGetMaxFragmentSize() {
then(config.withMaxFragmentSize(4000)).isSameAs(config);
then(config.getMaxFragmentSize()).isEqualTo(4000);

try {
config.withMaxFragmentSize(-1);
fail("missing sanity check");
} catch (IllegalArgumentException x) {
then(x).hasMessage("maxFragmentSize must be positive");
}
assertThatCode(() -> config.withMaxFragmentSize(-1))
.as("missing sanity check")
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("maxFragmentSize must be positive");
}

@Test
Expand All @@ -169,12 +163,10 @@ public void withAndGetPlainCredentials() {

then(config.withCredentials(null, "something").getCredentials()).isNull();

try {
config.withCredentials("akey", null);
fail("missing sanity check");
} catch (IllegalArgumentException x) {
then(x).hasMessage("secretAccessKey can not be null");
}
assertThatCode(() -> config.withCredentials("akey", null))
.as("missing sanity check")
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("secretAccessKey can not be null");
}

@Test
Expand Down Expand Up @@ -229,12 +221,10 @@ public void withAndGetBucketName() {
then(config.withBucketName("anothername").getBucketName()).isEqualTo("anothername");
then(config.withBucketName(null).getBucketName()).isNull();

try {
config.withBucketName("Wrong/bucket;name");
fail("missing sanity check");
} catch (IllegalArgumentException x) {
then(x).hasMessage("Bucket name should not contain uppercase characters");
}
assertThatCode(() -> config.withBucketName("Wrong/bucket;name"))
.as("missing sanity check")
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Bucket name should not contain uppercase characters");
}

@Test
Expand Down
Loading

0 comments on commit 3f2db3e

Please sign in to comment.