Copilot commented on code in PR #16067:
URL: https://github.com/apache/grails-core/pull/16067#discussion_r3678006622
##########
grails-core/src/test/groovy/org/apache/grails/core/GrailsBootstrapRegistryInitializerSpec.groovy:
##########
@@ -225,39 +229,54 @@ class GrailsBootstrapRegistryInitializerSpec extends
Specification {
def context = contextWithProperties([
(Settings.SETTING_LOG_FULL_STACKTRACE_ON_FILTER): 'false'
])
- def originalErr = System.err
- def baos = new ByteArrayOutputStream()
- System.setErr(new PrintStream(baos, true))
+ def appender = attachRecordingAppender()
when:
closeBootstrapContext(context)
GrailsUtil.deepSanitize(exceptionWithApplicationFrame())
then: "no 'Full Stack Trace:' entry is emitted"
- System.err.flush()
- !baos.toString().contains(StackTraceFilterer.FULL_STACK_TRACE_MESSAGE)
+ appender.list.every {
!it.formattedMessage.contains(StackTraceFilterer.FULL_STACK_TRACE_MESSAGE) }
cleanup:
- System.setErr(originalErr)
+ detachAppender(appender)
}
def 'defaults logFullStackTraceOnFilter to true on the promoted
DefaultStackTraceFilterer'() {
given:
def context = contextWithProperties([:])
- def originalErr = System.err
- def baos = new ByteArrayOutputStream()
- System.setErr(new PrintStream(baos, true))
+ def appender = attachRecordingAppender()
when:
closeBootstrapContext(context)
GrailsUtil.deepSanitize(exceptionWithApplicationFrame())
then: 'the positive control proving the negative case above is
meaningful'
- System.err.flush()
- baos.toString().contains(StackTraceFilterer.FULL_STACK_TRACE_MESSAGE)
+ appender.list.any {
it.formattedMessage.contains(StackTraceFilterer.FULL_STACK_TRACE_MESSAGE) }
cleanup:
- System.setErr(originalErr)
+ detachAppender(appender)
+ }
+
+ /**
+ * {@link DefaultStackTraceFilterer#STACK_LOG} routes through
commons-logging, which on this
+ * classpath resolves to a jcl-over-slf4j binding rather than a bare
console writer -- so the
+ * emitted message never touches {@code System.err} for capture there. The
logger name is a
+ * public constant precisely so tests and logging config can attach to it
directly instead.
+ */
+ private static ListAppender<ILoggingEvent> attachRecordingAppender() {
+ def logger =
LoggerFactory.getLogger(DefaultStackTraceFilterer.STACK_LOG_NAME) as Logger
+ def appender = new ListAppender<ILoggingEvent>()
Review Comment:
This spec introduces custom Logback ListAppender attach/detach helpers.
There is already an established test fixture for capturing Logback output in
this module: org.apache.grails.core.testing.support.LogCapture (see
grails-core/src/test/groovy/org/grails/exception/reporting/StackTraceFiltererSpec.groovy).
Using LogCapture here would avoid duplicating helper logic and ensures logger
level/additivity are restored after the test, keeping the suite more consistent
and resilient to logging config changes.
##########
grails-core/src/test/groovy/grails/util/GrailsUtilStackFiltererSpec.groovy:
##########
@@ -139,11 +138,31 @@ class GrailsUtilStackFiltererSpec extends Specification {
GrailsUtil.deepSanitize(exceptionWithApplicationFrame())
then: "a 'Full Stack Trace:' entry is emitted -- the positive control
proving the negative case above is meaningful"
- System.err.flush()
- baos.toString().contains(StackTraceFilterer.FULL_STACK_TRACE_MESSAGE)
+ appender.list.any {
it.formattedMessage.contains(StackTraceFilterer.FULL_STACK_TRACE_MESSAGE) }
cleanup:
- System.setErr(originalErr)
+ detachAppender(appender)
+ }
+
+ /**
+ * {@link DefaultStackTraceFilterer#STACK_LOG} routes through
commons-logging, which on this
+ * classpath resolves to a jcl-over-slf4j binding rather than a bare
console writer -- so the
+ * emitted message never touches {@code System.err} for capture there. The
logger name is a
+ * public constant precisely so tests and logging config can attach to it
directly instead.
+ */
+ private static ListAppender<ILoggingEvent> attachRecordingAppender() {
+ def logger =
LoggerFactory.getLogger(DefaultStackTraceFilterer.STACK_LOG_NAME) as Logger
+ def appender = new ListAppender<ILoggingEvent>()
Review Comment:
This spec adds custom Logback ListAppender attach/detach helpers, but
grails-core already provides a reusable test fixture for this:
org.apache.grails.core.testing.support.LogCapture (see
grails-core/src/test/groovy/org/grails/exception/reporting/StackTraceFiltererSpec.groovy).
Reusing LogCapture would remove duplicated helper code, and it also normalizes
logger level/additivity and restores prior logger state on close, keeping tests
better isolated from logging configuration changes.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
