dongjinleekr commented on a change in pull request #7898:
URL: https://github.com/apache/kafka/pull/7898#discussion_r829746139
##########
File path: streams/src/test/java/org/apache/kafka/streams/StreamsConfigTest.java
##########
@@ -1024,55 +1033,61 @@ public void
shouldThrowConfigExceptionWhenOptimizationConfigNotValueInRange() {
@SuppressWarnings("deprecation")
@Test
public void shouldLogWarningWhenEosAlphaIsUsed() {
- props.put(StreamsConfig.PROCESSING_GUARANTEE_CONFIG,
StreamsConfig.EXACTLY_ONCE);
+ try (final LogCaptureContext logCaptureContext =
LogCaptureContext.create(
+ Collections.singletonMap(StreamsConfig.class.getName(), "DEBUG")
+ )) {
+ logCaptureContext.setLatch(3);
- LogCaptureAppender.setClassLoggerToDebug(StreamsConfig.class);
- try (final LogCaptureAppender appender =
LogCaptureAppender.createAndRegister(StreamsConfig.class)) {
+ props.put(StreamsConfig.PROCESSING_GUARANTEE_CONFIG,
StreamsConfig.EXACTLY_ONCE);
new StreamsConfig(props);
assertThat(
- appender.getMessages(),
- hasItem("Configuration parameter `" +
StreamsConfig.EXACTLY_ONCE +
- "` is deprecated and will be removed in the 4.0.0
release. " +
- "Please use `" + StreamsConfig.EXACTLY_ONCE_V2 +
"` instead. " +
- "Note that this requires broker version 2.5+ so
you should prepare " +
- "to upgrade your brokers if necessary.")
+ logCaptureContext.getMessages(),
+ hasItem("WARN Configuration parameter `" +
StreamsConfig.EXACTLY_ONCE +
+ "` is deprecated and will be removed in the 4.0.0 release.
" +
+ "Please use `" + StreamsConfig.EXACTLY_ONCE_V2 + "`
instead. " +
+ "Note that this requires broker version 2.5+ so you should
prepare " +
+ "to upgrade your brokers if necessary. ")
Review comment:
Because of `streams/src/test/resources/log4j2.properties`:
```
appender.console.layout.pattern=[%d] %p %m (%c:%L)%n
```
##########
File path: streams/src/test/java/org/apache/kafka/streams/StreamsConfigTest.java
##########
@@ -1024,55 +1033,61 @@ public void
shouldThrowConfigExceptionWhenOptimizationConfigNotValueInRange() {
@SuppressWarnings("deprecation")
@Test
public void shouldLogWarningWhenEosAlphaIsUsed() {
- props.put(StreamsConfig.PROCESSING_GUARANTEE_CONFIG,
StreamsConfig.EXACTLY_ONCE);
+ try (final LogCaptureContext logCaptureContext =
LogCaptureContext.create(
+ Collections.singletonMap(StreamsConfig.class.getName(), "DEBUG")
+ )) {
+ logCaptureContext.setLatch(3);
- LogCaptureAppender.setClassLoggerToDebug(StreamsConfig.class);
- try (final LogCaptureAppender appender =
LogCaptureAppender.createAndRegister(StreamsConfig.class)) {
+ props.put(StreamsConfig.PROCESSING_GUARANTEE_CONFIG,
StreamsConfig.EXACTLY_ONCE);
new StreamsConfig(props);
assertThat(
- appender.getMessages(),
- hasItem("Configuration parameter `" +
StreamsConfig.EXACTLY_ONCE +
- "` is deprecated and will be removed in the 4.0.0
release. " +
- "Please use `" + StreamsConfig.EXACTLY_ONCE_V2 +
"` instead. " +
- "Note that this requires broker version 2.5+ so
you should prepare " +
- "to upgrade your brokers if necessary.")
+ logCaptureContext.getMessages(),
+ hasItem("WARN Configuration parameter `" +
StreamsConfig.EXACTLY_ONCE +
+ "` is deprecated and will be removed in the 4.0.0 release.
" +
+ "Please use `" + StreamsConfig.EXACTLY_ONCE_V2 + "`
instead. " +
+ "Note that this requires broker version 2.5+ so you should
prepare " +
+ "to upgrade your brokers if necessary. ")
);
}
}
@SuppressWarnings("deprecation")
@Test
public void shouldLogWarningWhenEosBetaIsUsed() {
- props.put(StreamsConfig.PROCESSING_GUARANTEE_CONFIG,
StreamsConfig.EXACTLY_ONCE_BETA);
+ try (final LogCaptureContext logCaptureContext =
LogCaptureContext.create(
+ Collections.singletonMap(StreamsConfig.class.getName(), "DEBUG")
+ )) {
+ logCaptureContext.setLatch(3);
- LogCaptureAppender.setClassLoggerToDebug(StreamsConfig.class);
- try (final LogCaptureAppender appender =
LogCaptureAppender.createAndRegister(StreamsConfig.class)) {
+ props.put(StreamsConfig.PROCESSING_GUARANTEE_CONFIG,
StreamsConfig.EXACTLY_ONCE_BETA);
new StreamsConfig(props);
assertThat(
- appender.getMessages(),
- hasItem("Configuration parameter `" +
StreamsConfig.EXACTLY_ONCE_BETA +
- "` is deprecated and will be removed in the 4.0.0
release. " +
- "Please use `" + StreamsConfig.EXACTLY_ONCE_V2 +
"` instead.")
+ logCaptureContext.getMessages(),
+ hasItem("WARN Configuration parameter `" +
StreamsConfig.EXACTLY_ONCE_BETA +
+ "` is deprecated and will be removed in the 4.0.0 release.
" +
+ "Please use `" + StreamsConfig.EXACTLY_ONCE_V2 + "`
instead. ")
);
}
}
@SuppressWarnings("deprecation")
@Test
public void shouldLogWarningWhenRetriesIsUsed() {
- props.put(StreamsConfig.RETRIES_CONFIG, 0);
-
- LogCaptureAppender.setClassLoggerToDebug(StreamsConfig.class);
- try (final LogCaptureAppender appender =
LogCaptureAppender.createAndRegister(StreamsConfig.class)) {
+ try (final LogCaptureContext logCaptureContext =
LogCaptureContext.create()) {
+ logCaptureContext.setLatch(2);
+ props.put(StreamsConfig.RETRIES_CONFIG, 0);
new StreamsConfig(props);
+ logCaptureContext.await(30L, TimeUnit.SECONDS);
assertThat(
- appender.getMessages(),
- hasItem("Configuration parameter `" +
StreamsConfig.RETRIES_CONFIG +
- "` is deprecated and will be removed in the 4.0.0
release.")
+ logCaptureContext.getMessages(),
+ hasItem("WARN Configuration parameter `" +
StreamsConfig.RETRIES_CONFIG
+ + "` is deprecated and will be removed in the 4.0.0
release. ")
);
+ } catch (final InterruptedException e) {
+ e.printStackTrace();
Review comment:
No. This is a debris from the old code. Removed.
##########
File path:
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamSlidingWindowAggregateTest.java
##########
@@ -715,19 +713,13 @@ public void shouldLogAndMeterWhenSkippingNullKey() {
.aggregate(MockInitializer.STRING_INIT,
MockAggregator.toStringInstance("+"), Materialized.<String, String,
WindowStore<Bytes,
byte[]>>as("topic1-Canonicalized").withValueSerde(Serdes.String()));
props.setProperty(StreamsConfig.BUILT_IN_METRICS_VERSION_CONFIG,
builtInMetricsVersion);
-
- try (final LogCaptureAppender appender =
LogCaptureAppender.createAndRegister(KStreamSlidingWindowAggregate.class);
+ try (final LogCaptureContext logCaptureContext =
LogCaptureContext.create();
final TopologyTestDriver driver = new
TopologyTestDriver(builder.build(), props)) {
+ logCaptureContext.setLatch(1);
Review comment:
Oh, `await` is just a debris of old approach; it is not called anymore.
So removed, and the documentation on `setLatch` was added instead.
##########
File path:
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamSlidingWindowAggregateTest.java
##########
@@ -715,19 +713,13 @@ public void shouldLogAndMeterWhenSkippingNullKey() {
.aggregate(MockInitializer.STRING_INIT,
MockAggregator.toStringInstance("+"), Materialized.<String, String,
WindowStore<Bytes,
byte[]>>as("topic1-Canonicalized").withValueSerde(Serdes.String()));
props.setProperty(StreamsConfig.BUILT_IN_METRICS_VERSION_CONFIG,
builtInMetricsVersion);
-
- try (final LogCaptureAppender appender =
LogCaptureAppender.createAndRegister(KStreamSlidingWindowAggregate.class);
+ try (final LogCaptureContext logCaptureContext =
LogCaptureContext.create();
final TopologyTestDriver driver = new
TopologyTestDriver(builder.build(), props)) {
+ logCaptureContext.setLatch(1);
Review comment:
Documentaton on `LogCaptureContext` added.
##########
File path:
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStoreTest.java
##########
@@ -53,161 +53,166 @@ RocksDBStore getRocksDBStore() {
@Test
public void shouldOpenNewStoreInRegularMode() {
- try (final LogCaptureAppender appender =
LogCaptureAppender.createAndRegister(RocksDBTimestampedStore.class)) {
+ try (final LogCaptureContext logCaptureContext =
LogCaptureContext.create()) {
+ logCaptureContext.setLatch(2);
rocksDBStore.init((StateStoreContext) context, rocksDBStore);
- assertThat(appender.getMessages(), hasItem("Opening store " +
DB_NAME + " in regular mode"));
- }
+ assertThat(logCaptureContext.getMessages(), hasItem("INFO Opening
store " + DB_NAME + " in regular mode "));
- try (final KeyValueIterator<Bytes, byte[]> iterator =
rocksDBStore.all()) {
- assertThat(iterator.hasNext(), is(false));
+ try (final KeyValueIterator<Bytes, byte[]> iterator =
rocksDBStore.all()) {
+ assertThat(iterator.hasNext(), is(false));
+ }
}
}
@Test
public void shouldOpenExistingStoreInRegularMode() throws Exception {
- // prepare store
- rocksDBStore.init((StateStoreContext) context, rocksDBStore);
- rocksDBStore.put(new Bytes("key".getBytes()),
"timestamped".getBytes());
- rocksDBStore.close();
+ try (final LogCaptureContext logCaptureContext =
LogCaptureContext.create()) {
+ logCaptureContext.setLatch(3);
- // re-open store
- try (final LogCaptureAppender appender =
LogCaptureAppender.createAndRegister(RocksDBTimestampedStore.class)) {
+ // prepare store
rocksDBStore.init((StateStoreContext) context, rocksDBStore);
-
- assertThat(appender.getMessages(), hasItem("Opening store " +
DB_NAME + " in regular mode"));
- } finally {
+ rocksDBStore.put(new Bytes("key".getBytes()),
"timestamped".getBytes());
rocksDBStore.close();
Review comment:
The try block with `LogCaptureAppender` resource was removed; That's the
reason.
--
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.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
