ijuma commented on a change in pull request #9520:
URL: https://github.com/apache/kafka/pull/9520#discussion_r549417155
##########
File path:
clients/src/test/java/org/apache/kafka/clients/producer/KafkaProducerTest.java
##########
@@ -192,11 +192,12 @@ public void testConstructorWithSerializers() {
new KafkaProducer<>(producerProps, new ByteArraySerializer(), new
ByteArraySerializer()).close();
}
- @Test(expected = ConfigException.class)
+ @Test
public void testNoSerializerProvided() {
Properties producerProps = new Properties();
producerProps.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG,
"localhost:9000");
- new KafkaProducer(producerProps);
+
Review comment:
Was this new line intentional?
##########
File path:
clients/src/test/java/org/apache/kafka/clients/producer/internals/TransactionManagerTest.java
##########
@@ -444,34 +444,30 @@ public void
testAddPartitionToTransactionRetainsRetryBackoffWhenPartitionsAlread
assertEquals(DEFAULT_RETRY_BACKOFF_MS, handler.retryBackoffMs());
}
- @Test(expected = IllegalStateException.class)
+ @Test
public void testMaybeAddPartitionToTransactionBeforeInitTransactions() {
- transactionManager.failIfNotReadyForSend();
- transactionManager.maybeAddPartitionToTransaction(new
TopicPartition("foo", 0));
+ assertThrows(IllegalStateException.class, () ->
transactionManager.failIfNotReadyForSend());
}
- @Test(expected = IllegalStateException.class)
+ @Test
public void testMaybeAddPartitionToTransactionBeforeBeginTransaction() {
doInitTransactions();
- transactionManager.failIfNotReadyForSend();
- transactionManager.maybeAddPartitionToTransaction(new
TopicPartition("foo", 0));
+ assertThrows(IllegalStateException.class, () ->
transactionManager.failIfNotReadyForSend());
Review comment:
Same as above.
##########
File path:
clients/src/test/java/org/apache/kafka/clients/producer/internals/TransactionManagerTest.java
##########
@@ -444,34 +444,30 @@ public void
testAddPartitionToTransactionRetainsRetryBackoffWhenPartitionsAlread
assertEquals(DEFAULT_RETRY_BACKOFF_MS, handler.retryBackoffMs());
}
- @Test(expected = IllegalStateException.class)
+ @Test
public void testMaybeAddPartitionToTransactionBeforeInitTransactions() {
- transactionManager.failIfNotReadyForSend();
- transactionManager.maybeAddPartitionToTransaction(new
TopicPartition("foo", 0));
+ assertThrows(IllegalStateException.class, () ->
transactionManager.failIfNotReadyForSend());
}
- @Test(expected = IllegalStateException.class)
+ @Test
public void testMaybeAddPartitionToTransactionBeforeBeginTransaction() {
doInitTransactions();
- transactionManager.failIfNotReadyForSend();
- transactionManager.maybeAddPartitionToTransaction(new
TopicPartition("foo", 0));
+ assertThrows(IllegalStateException.class, () ->
transactionManager.failIfNotReadyForSend());
}
- @Test(expected = KafkaException.class)
+ @Test
public void testMaybeAddPartitionToTransactionAfterAbortableError() {
doInitTransactions();
transactionManager.beginTransaction();
transactionManager.transitionToAbortableError(new KafkaException());
- transactionManager.failIfNotReadyForSend();
- transactionManager.maybeAddPartitionToTransaction(new
TopicPartition("foo", 0));
+ assertThrows(KafkaException.class, () ->
transactionManager.failIfNotReadyForSend());
Review comment:
Same as above.
##########
File path:
clients/src/test/java/org/apache/kafka/clients/producer/MockProducerTest.java
##########
@@ -663,12 +623,6 @@ public void
shouldPreserveOffsetsFromCommitByGroupMetadataOnAbortIfTransactionsA
producer.beginTransaction();
String group2 = "g2";
- Map<TopicPartition, OffsetAndMetadata> groupCommit2 = new
HashMap<TopicPartition, OffsetAndMetadata>() {
- {
- put(new TopicPartition(topic, 2), new OffsetAndMetadata(53L,
null));
- put(new TopicPartition(topic, 3), new OffsetAndMetadata(84L,
null));
- }
- };
producer.sendOffsetsToTransaction(groupCommit, new
ConsumerGroupMetadata(group2));
Review comment:
I think this was intended to be `groupCommit2`, right?
##########
File path:
clients/src/test/java/org/apache/kafka/clients/producer/internals/TransactionManagerTest.java
##########
@@ -444,34 +444,30 @@ public void
testAddPartitionToTransactionRetainsRetryBackoffWhenPartitionsAlread
assertEquals(DEFAULT_RETRY_BACKOFF_MS, handler.retryBackoffMs());
}
- @Test(expected = IllegalStateException.class)
+ @Test
public void testMaybeAddPartitionToTransactionBeforeInitTransactions() {
- transactionManager.failIfNotReadyForSend();
- transactionManager.maybeAddPartitionToTransaction(new
TopicPartition("foo", 0));
+ assertThrows(IllegalStateException.class, () ->
transactionManager.failIfNotReadyForSend());
}
- @Test(expected = IllegalStateException.class)
+ @Test
public void testMaybeAddPartitionToTransactionBeforeBeginTransaction() {
doInitTransactions();
- transactionManager.failIfNotReadyForSend();
- transactionManager.maybeAddPartitionToTransaction(new
TopicPartition("foo", 0));
+ assertThrows(IllegalStateException.class, () ->
transactionManager.failIfNotReadyForSend());
}
- @Test(expected = KafkaException.class)
+ @Test
public void testMaybeAddPartitionToTransactionAfterAbortableError() {
doInitTransactions();
transactionManager.beginTransaction();
transactionManager.transitionToAbortableError(new KafkaException());
- transactionManager.failIfNotReadyForSend();
- transactionManager.maybeAddPartitionToTransaction(new
TopicPartition("foo", 0));
+ assertThrows(KafkaException.class, () ->
transactionManager.failIfNotReadyForSend());
}
- @Test(expected = KafkaException.class)
+ @Test
public void testMaybeAddPartitionToTransactionAfterFatalError() {
doInitTransactions();
transactionManager.transitionToFatalError(new KafkaException());
- transactionManager.failIfNotReadyForSend();
- transactionManager.maybeAddPartitionToTransaction(new
TopicPartition("foo", 0));
+ assertThrows(KafkaException.class, () ->
transactionManager.failIfNotReadyForSend());
Review comment:
Same as above.
##########
File path:
clients/src/test/java/org/apache/kafka/clients/producer/internals/TransactionManagerTest.java
##########
@@ -444,34 +444,30 @@ public void
testAddPartitionToTransactionRetainsRetryBackoffWhenPartitionsAlread
assertEquals(DEFAULT_RETRY_BACKOFF_MS, handler.retryBackoffMs());
}
- @Test(expected = IllegalStateException.class)
+ @Test
public void testMaybeAddPartitionToTransactionBeforeInitTransactions() {
- transactionManager.failIfNotReadyForSend();
- transactionManager.maybeAddPartitionToTransaction(new
TopicPartition("foo", 0));
+ assertThrows(IllegalStateException.class, () ->
transactionManager.failIfNotReadyForSend());
Review comment:
This change doesn't seem right. The test is intended to test
`maybeAddPartitionToTransaction`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
