codeconsole commented on PR #15940:
URL: https://github.com/apache/grails-core/pull/15940#issuecomment-4930875135

   Thanks for the thorough audit — all five findings were accurate. Addressed 
in 170ae02d0f:
   
   **1. Plugin lifecycle** — Added `I18nGrailsPluginSpec` covering 
`doWithApplicationContext()` publishing the resolved locales to the servlet 
context, the non-`WebApplicationContext` and missing-bean early returns, and 
`onChange` clearing the resolver cache and republishing a freshly computed 
list. This also surfaced an NPE in `onChange` when 
`BuildSettings.RESOURCES_DIR` is unset (as in unit tests), now null-guarded.
   
   **2. `availableLocaleResolver` bean wiring** — `I18nAutoConfigurationSpec` 
now asserts the bean registers by default, honours `grails.i18n.default.locale` 
(including the underscore form `pt_BR`) and 
`grails.i18n.availableLocales.includePlugins=false`, and backs off to a 
user-defined `AvailableLocaleResolver` bean.
   
   **3. Web-profile skeleton `main.gsp`** — Acknowledged, but leaving as-is: 
`grails-profiles` modules are resource-only packaging projects with no test 
source sets anywhere in the repo, so there is no existing place to test profile 
templates. The markup is byte-identical to the forge template, which 
`GrailsGspSpec` covers.
   
   **4. `AvailableLocaleResolver` branches** — Covered everything reachable 
through the public API: null and `Locale.ROOT` default locales, the 
`IOException` catch (via a `ClassLoader` whose `getResources` throws, through 
the public constructor), and deliberate malformed-bundle fixtures 
(`standalone.properties`, `messages_.properties`, `bogus-plugin_zz.properties`) 
replacing the incidental coverage. The remaining two branches (`filename == 
null`, non-`.properties` filename) are defensive dead code on the scan path — 
the classpath pattern guarantees `.properties` filenames — and testing them 
would require invoking the private method directly, which this repo's testing 
rules disallow.
   
   **5. `available="false"`** — Added an explicit test: with locales published 
to the servlet context, `available="false"` still renders the full JVM locale 
list.
   


-- 
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]

Reply via email to