borinquenkid commented on PR #15666:
URL: https://github.com/apache/grails-core/pull/15666#issuecomment-5099250136
*Posted with assistance from Claude Code (Anthropic's CLI agent), verifying
the fixes in cbfe2fc8.*
@codeconsole I checked cbfe2fc8 against each point from @jdaugherty's
2026-07-26 review by reading the diff and running the affected test suites (new
specs, checkstyle, full grails-core + grails-web-mvc). All seven are genuinely
resolved, not just asserted in the reply comments:
- Class-literal config now resolves correctly (verified: a raw `Class` value
survives `environment.getProperty(key, Object.class)` unconverted)
- ClassLoader fixed to `applicationContext.getClassLoader()` (test proves
grails-core's own loader can't see the class, the context's loader can)
- Both property reads are now independently try/caught — a bad value
degrades instead of failing `bootstrapContext.close()`
- `STACK_TRACE_FILTERER_BEAN_NAME` moved out of `grails-bootstrap` (zero net
diff on `StackTraceFilterer.java` vs `8.0.x`)
- `GrailsExceptionResolver`'s promoted-bean lookup now catches
`BeansException` broadly — a same-name bean of the wrong type degrades cleanly
- Doc note moved to `upgrading80x.adoc` §38, sequential numbering, accurate
boundary wording
- Reflection is gone from both unit specs
Two things I don't think were caught yet:
1. **`GrailsBootstrapRegistryInitializer.java:155-178`
(`resolveFiltererClass`) — silent fallback on a wrong-typed config value.** If
`grails.logging.stackTraceFiltererClass` is set to something that's neither a
`Class` nor a `CharSequence` (a YAML list, boolean, int), it falls through both
`instanceof` checks and returns `DefaultStackTraceFilterer.class` at line 178
with no `LOG.warn`. Every other failure path here logs; this one silently
downgrades, contradicting the method's own javadoc ("every config read...
degrades... with a logged warning").
2. **Class-literal test coverage doesn't exercise the real
`application.groovy` path.** The new `GrailsBootstrapRegistryInitializerSpec`
test uses a synthetic `MapPropertySource` holding a raw `Class`, not the actual
`GroovyConfigPropertySourceLoader` → `NavigableMapPropertySource` pipeline that
parses `application.groovy` — the exact shape from the original bug report.
Both classes are available in `grails-core/src/main` and could back a more
faithful test.
Everything else I traced (bean-promotion ordering vs. `refresh()`,
`volatile` field visibility, the `registerSingleton` → later
`registerBeanDefinition` override semantics, test isolation via
`cleanup()`/`cleanupSpec()`) checked out under test.
--
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]
