vy commented on code in PR #2227:
URL: https://github.com/apache/logging-log4j2/pull/2227#discussion_r1463035845
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/QueueFullAbstractTest.java:
##########
@@ -302,9 +302,11 @@ protected static void assertAsyncLoggerConfig(final
LoggerContext ctx, final int
final Configuration config = ctx.getConfiguration();
assertThat(config).isNotNull();
assertThat(config.getRootLogger()).isInstanceOf(AsyncLoggerConfig.class);
- final AsyncLoggerConfigDisruptor disruptor =
(AsyncLoggerConfigDisruptor) config.getAsyncLoggerConfigDelegate();
- final Field sizeField = field(AsyncLoggerConfigDisruptor.class,
"ringBufferSize");
- assertThat(sizeField.get(disruptor)).isEqualTo(expectedBufferSize);
+ final AsyncLoggerConfigDisruptor disruptorWrapper =
+ (AsyncLoggerConfigDisruptor)
config.getAsyncLoggerConfigDelegate();
+ final Disruptor<?> disruptor = (Disruptor<?>)
+ field(AsyncLoggerConfigDisruptor.class,
"disruptor").get(disruptorWrapper);
Review Comment:
Just a reminder that `AsyncLoggerConfigDisruptor` could have had a
package-local `getDisruptor` to ease testing, just like `getWaitStrategy()`. I
am fine with the currently implemented reflection approach too.
We have a mixture of both all over the code base. [Excluding the reflection
usage when package-local has accessibility problems due to tests being located
in a different package,] I wish we would have had a convention for this.
--
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: notifications-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
