showuon commented on a change in pull request #9121:
URL: https://github.com/apache/kafka/pull/9121#discussion_r475362316
##########
File path:
streams/src/test/java/org/apache/kafka/streams/processor/internals/GlobalStateManagerImplTest.java
##########
@@ -180,6 +183,27 @@ public void shouldReadCheckpointOffsets() throws
IOException {
assertEquals(expected, offsets);
}
+ @Test
+ public void shouldLogWarningMessageWhenIOExceptionInCheckPoint() throws
IOException {
+ final Map<TopicPartition, Long> offsets = Collections.singletonMap(t1,
25L);
+ stateManager.initialize();
+ stateManager.updateChangelogOffsets(offsets);
+
+ final File file = new File(stateDirectory.globalStateDir(),
StateManagerUtil.CHECKPOINT_FILE_NAME + ".tmp");
+ file.createNewFile();
+ // set the checkpoint tmp file to read-only to simulate the
IOException situation
+ file.setWritable(false);
+
+ try (final LogCaptureAppender appender =
+
LogCaptureAppender.createAndRegister(GlobalStateManagerImpl.class)) {
+
+ // checkpoint should fail due to the file is readonly
+ stateManager.checkpoint();
+ assertThat(appender.getMessages(), hasItem(containsString(
+ "Failed to write offset checkpoint file to " +
checkpointFile.getPath() + " for global stores")));
Review comment:
I agree it'll be better to do the assertion after the try-block. But no,
we can't move the assert out of the try-block because the `appender` is
declared within try block. We can move the assert out of try-block if we don't
use the try resource auto-close pattern, but I don't think it would be better.
Also, we assert within try-block for the `LogCaptureAppender` in other
places. I think they are all for the same reason as I mentioned above.
Thanks.
----------------------------------------------------------------
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
