Checkstyle: added additional checks to, primarily, safeguard against bugs. (#1154)

This commit is contained in:
stair 2023-06-27 15:24:03 -04:00 committed by GitHub
parent 4eff398147
commit a9b0d00852
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 19 additions and 25 deletions

View file

@ -49,7 +49,7 @@ public class MessageTest {
new ProcessRecordsMessage(), new ProcessRecordsMessage(),
new ShutdownRequestedMessage(), new ShutdownRequestedMessage(),
new LeaseLostMessage(), new LeaseLostMessage(),
new ShardEndedMessage() new ShardEndedMessage(),
}; };
// TODO: fix this // TODO: fix this

View file

@ -152,7 +152,7 @@ public class RetrievalConfig {
if (streamTracker().isMultiStream()) { if (streamTracker().isMultiStream()) {
throw new IllegalArgumentException( throw new IllegalArgumentException(
"Cannot set initialPositionInStreamExtended when multiStreamTracker is set"); "Cannot set initialPositionInStreamExtended when multiStreamTracker is set");
}; }
final StreamIdentifier streamIdentifier = getSingleStreamIdentifier(); final StreamIdentifier streamIdentifier = getSingleStreamIdentifier();
final StreamConfig updatedConfig = new StreamConfig(streamIdentifier, initialPositionInStreamExtended); final StreamConfig updatedConfig = new StreamConfig(streamIdentifier, initialPositionInStreamExtended);

View file

@ -397,7 +397,7 @@ public class ShardShardRecordProcessorCheckpointerTest {
assertThat(checkpointer.largestPermittedCheckpointValue(), equalTo(sequenceNumber)); assertThat(checkpointer.largestPermittedCheckpointValue(), equalTo(sequenceNumber));
} }
/* /**
* This test is a mixed test of checking some basic functionality of checkpointing at a sequence number and making * This test is a mixed test of checking some basic functionality of checkpointing at a sequence number and making
* sure certain bounds checks and validations are being performed inside the checkpointer to prevent clients from * sure certain bounds checks and validations are being performed inside the checkpointer to prevent clients from
* checkpointing out of order/too big/non-numeric values that aren't valid strings for them to be checkpointing * checkpointing out of order/too big/non-numeric values that aren't valid strings for them to be checkpointing
@ -444,7 +444,7 @@ public class ShardShardRecordProcessorCheckpointerTest {
new ExtendedSequenceNumber("bogus-checkpoint-value"), // Can't checkpoint at non-numeric string new ExtendedSequenceNumber("bogus-checkpoint-value"), // Can't checkpoint at non-numeric string
ExtendedSequenceNumber.SHARD_END, // Can't go to the end unless it is set as the max ExtendedSequenceNumber.SHARD_END, // Can't go to the end unless it is set as the max
ExtendedSequenceNumber.TRIM_HORIZON, // Can't go back to an initial sentinel value ExtendedSequenceNumber.TRIM_HORIZON, // Can't go back to an initial sentinel value
ExtendedSequenceNumber.LATEST // Can't go back to an initial sentinel value ExtendedSequenceNumber.LATEST, // Can't go back to an initial sentinel value
}; };
for (ExtendedSequenceNumber badCheckpointValue : valuesWeShouldNotBeAbleToCheckpointAt) { for (ExtendedSequenceNumber badCheckpointValue : valuesWeShouldNotBeAbleToCheckpointAt) {
try { try {
@ -477,7 +477,7 @@ public class ShardShardRecordProcessorCheckpointerTest {
processingCheckpointer.lastCheckpointValue(), equalTo(ExtendedSequenceNumber.SHARD_END)); processingCheckpointer.lastCheckpointValue(), equalTo(ExtendedSequenceNumber.SHARD_END));
} }
/* /**
* This test is a mixed test of checking some basic functionality of two phase checkpointing at a sequence number * This test is a mixed test of checking some basic functionality of two phase checkpointing at a sequence number
* and making sure certain bounds checks and validations are being performed inside the checkpointer to prevent * and making sure certain bounds checks and validations are being performed inside the checkpointer to prevent
* clients from checkpointing out of order/too big/non-numeric values that aren't valid strings for them to be * clients from checkpointing out of order/too big/non-numeric values that aren't valid strings for them to be
@ -548,7 +548,7 @@ public class ShardShardRecordProcessorCheckpointerTest {
new ExtendedSequenceNumber("bogus-checkpoint-value"), // Can't checkpoint at non-numeric string new ExtendedSequenceNumber("bogus-checkpoint-value"), // Can't checkpoint at non-numeric string
ExtendedSequenceNumber.SHARD_END, // Can't go to the end unless it is set as the max ExtendedSequenceNumber.SHARD_END, // Can't go to the end unless it is set as the max
ExtendedSequenceNumber.TRIM_HORIZON, // Can't go back to an initial sentinel value ExtendedSequenceNumber.TRIM_HORIZON, // Can't go back to an initial sentinel value
ExtendedSequenceNumber.LATEST // Can't go back to an initial sentinel value ExtendedSequenceNumber.LATEST, // Can't go back to an initial sentinel value
}; };
for (ExtendedSequenceNumber badCheckpointValue : valuesWeShouldNotBeAbleToCheckpointAt) { for (ExtendedSequenceNumber badCheckpointValue : valuesWeShouldNotBeAbleToCheckpointAt) {
try { try {
@ -566,7 +566,6 @@ public class ShardShardRecordProcessorCheckpointerTest {
assertThat("Largest sequence number should not have changed", assertThat("Largest sequence number should not have changed",
processingCheckpointer.largestPermittedCheckpointValue(), equalTo(thirdSequenceNumber)); processingCheckpointer.largestPermittedCheckpointValue(), equalTo(thirdSequenceNumber));
assertThat(checkpoint.getCheckpointObject(shardId).pendingCheckpoint(), nullValue()); assertThat(checkpoint.getCheckpointObject(shardId).pendingCheckpoint(), nullValue());
} }
// advance to third number // advance to third number
@ -601,7 +600,6 @@ public class ShardShardRecordProcessorCheckpointerTest {
* *
* @throws Exception * @throws Exception
*/ */
@SuppressWarnings("serial")
@Test @Test
public final void testMixedCheckpointCalls() throws Exception { public final void testMixedCheckpointCalls() throws Exception {
for (LinkedHashMap<String, CheckpointAction> testPlan : getMixedCallsTestPlan()) { for (LinkedHashMap<String, CheckpointAction> testPlan : getMixedCallsTestPlan()) {
@ -617,7 +615,6 @@ public class ShardShardRecordProcessorCheckpointerTest {
* *
* @throws Exception * @throws Exception
*/ */
@SuppressWarnings("serial")
@Test @Test
public final void testMixedTwoPhaseCheckpointCalls() throws Exception { public final void testMixedTwoPhaseCheckpointCalls() throws Exception {
for (LinkedHashMap<String, CheckpointAction> testPlan : getMixedCallsTestPlan()) { for (LinkedHashMap<String, CheckpointAction> testPlan : getMixedCallsTestPlan()) {

View file

@ -17,7 +17,6 @@ package software.amazon.kinesis.metrics;
import org.junit.Test; import org.junit.Test;
import software.amazon.awssdk.services.cloudwatch.model.StandardUnit; import software.amazon.awssdk.services.cloudwatch.model.StandardUnit;
import software.amazon.kinesis.metrics.EndingMetricsScope;
public class EndingMetricsScopeTest { public class EndingMetricsScopeTest {

View file

@ -27,31 +27,28 @@ import static org.junit.Assert.assertThat;
@Slf4j @Slf4j
public class AWSExceptionManagerTest { public class AWSExceptionManagerTest {
private static final String EXPECTED_HANDLING_MARKER = AWSExceptionManagerTest.class.getSimpleName();
private final AWSExceptionManager manager = new AWSExceptionManager();
@Test @Test
public void testSpecificException() { public void testSpecificException() {
AWSExceptionManager manager = new AWSExceptionManager();
final String EXPECTED_HANDLING_MARKER = "Handled-TestException";
manager.add(TestException.class, t -> { manager.add(TestException.class, t -> {
log.info("Handling test exception: {} -> {}", t.getMessage(), t.getAdditionalMessage()); log.info("Handling test exception: {} -> {}", t.getMessage(), t.getAdditionalMessage());
return new RuntimeException(EXPECTED_HANDLING_MARKER, t); return new RuntimeException(EXPECTED_HANDLING_MARKER, t);
}); });
TestException te = new TestException("Main Mesage", "Sub Message"); TestException te = new TestException("Main Message", "Sub Message");
RuntimeException converted = manager.apply(te); RuntimeException converted = manager.apply(te);
assertThat(converted, isA(RuntimeException.class)); assertThat(converted, isA(RuntimeException.class));
assertThat(converted.getMessage(), equalTo(EXPECTED_HANDLING_MARKER)); assertThat(converted.getMessage(), equalTo(EXPECTED_HANDLING_MARKER));
assertThat(converted.getCause(), equalTo(te)); assertThat(converted.getCause(), equalTo(te));
} }
@Test @Test
public void testParentException() { public void testParentException() {
AWSExceptionManager manager = new AWSExceptionManager();
final String EXPECTED_HANDLING_MARKER = "Handled-IllegalStateException";
manager.add(IllegalArgumentException.class, i -> new RuntimeException("IllegalArgument", i)); manager.add(IllegalArgumentException.class, i -> new RuntimeException("IllegalArgument", i));
manager.add(Exception.class, i -> new RuntimeException("RawException", i)); manager.add(Exception.class, i -> new RuntimeException("RawException", i));
manager.add(IllegalStateException.class, i -> new RuntimeException(EXPECTED_HANDLING_MARKER, i)); manager.add(IllegalStateException.class, i -> new RuntimeException(EXPECTED_HANDLING_MARKER, i));
@ -66,8 +63,7 @@ public class AWSExceptionManagerTest {
@Test @Test
public void testDefaultHandler() { public void testDefaultHandler() {
final String EXPECTED_HANDLING_MARKER = "Handled-Default"; manager.defaultFunction(i -> new RuntimeException(EXPECTED_HANDLING_MARKER, i));
AWSExceptionManager manager = new AWSExceptionManager().defaultFunction(i -> new RuntimeException(EXPECTED_HANDLING_MARKER, i));
manager.add(IllegalArgumentException.class, i -> new RuntimeException("IllegalArgument", i)); manager.add(IllegalArgumentException.class, i -> new RuntimeException("IllegalArgument", i));
manager.add(Exception.class, i -> new RuntimeException("RawException", i)); manager.add(Exception.class, i -> new RuntimeException("RawException", i));
@ -83,8 +79,6 @@ public class AWSExceptionManagerTest {
@Test @Test
public void testIdHandler() { public void testIdHandler() {
AWSExceptionManager manager = new AWSExceptionManager();
manager.add(IllegalArgumentException.class, i -> new RuntimeException("IllegalArgument", i)); manager.add(IllegalArgumentException.class, i -> new RuntimeException("IllegalArgument", i));
manager.add(Exception.class, i -> new RuntimeException("RawException", i)); manager.add(Exception.class, i -> new RuntimeException("RawException", i));
manager.add(IllegalStateException.class, i -> i); manager.add(IllegalStateException.class, i -> i);

View file

@ -24,7 +24,6 @@ import org.junit.runner.RunWith;
import org.mockito.Mock; import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner; import org.mockito.runners.MockitoJUnitRunner;
import org.slf4j.Logger; import org.slf4j.Logger;
import software.amazon.kinesis.retrieval.ThrottlingReporter;
@RunWith(MockitoJUnitRunner.class) @RunWith(MockitoJUnitRunner.class)
public class ThrottlingReporterTest { public class ThrottlingReporterTest {
@ -40,7 +39,6 @@ public class ThrottlingReporterTest {
reporter.throttled(); reporter.throttled();
verify(throttleLog).warn(anyString()); verify(throttleLog).warn(anyString());
verify(throttleLog, never()).error(anyString()); verify(throttleLog, never()).error(anyString());
} }
@Test @Test
@ -63,7 +61,6 @@ public class ThrottlingReporterTest {
reporter.throttled(); reporter.throttled();
verify(throttleLog, times(2)).warn(anyString()); verify(throttleLog, times(2)).warn(anyString());
verify(throttleLog, times(3)).error(anyString()); verify(throttleLog, times(3)).error(anyString());
} }
private class LogTestingThrottingReporter extends ThrottlingReporter { private class LogTestingThrottingReporter extends ThrottlingReporter {

View file

@ -23,8 +23,13 @@
<module name="TreeWalker"> <module name="TreeWalker">
<module name="AvoidStarImport"/> <module name="AvoidStarImport"/>
<module name="ArrayTrailingComma"/>
<module name="ConstantName"/> <module name="ConstantName"/>
<module name="CovariantEquals"/>
<module name="EmptyStatement"/>
<module name="EqualsHashCode"/>
<module name="InvalidJavadocPosition"/> <module name="InvalidJavadocPosition"/>
<module name="LocalFinalVariableName"/>
<module name="LocalVariableName"/> <module name="LocalVariableName"/>
<module name="MemberName"/> <module name="MemberName"/>
<module name="MethodName"> <module name="MethodName">
@ -36,7 +41,9 @@
<module name="OneTopLevelClass"/> <module name="OneTopLevelClass"/>
<module name="OuterTypeFilename"/> <module name="OuterTypeFilename"/>
<module name="ParameterName"/> <module name="ParameterName"/>
<module name="RedundantImport"/>
<module name="UnusedImports"/> <module name="UnusedImports"/>
<module name="UpperEll"/>
<module name="WhitespaceAfter"/> <module name="WhitespaceAfter"/>
</module> </module>