codeconsole commented on PR #16399: URL: https://github.com/apache/grails-core/pull/16399#issuecomment-5848002977
@matrei thanks for the second pass, and for the probes. All five are addressed in `47835cb838`. **1. `proxyHandler`.** You are right: `core`'s registrar registers it before the test's configuration is read, so the plugin's definition wins over a `beans` block's, as over an application's `@Bean`. `unitTesting.adoc` now excludes that case by name: the block does not replace a bean an included plugin registers, "even under the name of one of those stand-ins: the `core` plugin, which is always included, registers `proxyHandler`". It also still points to `beanRegistrar` for replacing it. Pinned in `ReplacingBeansSpec`: - `ReplacingHarnessDefaultsSpec` now declares `proxyHandler` next to `messageSource` and asserts it stays a `DefaultProxyHandler`. That feature fails if the early registration is skipped. - `BeanRegistrarOverPluginStandInSpec` asserts that `beanRegistrar()` does replace it. **2. `doWithConfig` and the environment.** Taken the in-scope route. The harness takes `config.toProperties()` before and after `doWithConfig`, and adds the changed keys to the context's `Environment` as a first-priority `MapPropertySource` (`doWithConfig`). This happens in `customizeGrailsApplication`, which the early post-processor triggers ahead of `ConfigurationClassPostProcessor`. So the test's and included plugins' `.conditionalOnProperty(...)`, and the framework's `@ConditionalOnProperty`, see it. - Your probe is now `DoWithConfigEnvironmentSpec`. It asserts `environment.getProperty(...) == 'true'`, that `probeFeature` is present, and that `probeValue == 'true'`. Both features fail without the property source. - The guide's *Manipulating Configuration* section says so, and the upgrade note lists it with the other harness changes, since the framework's property conditions now react to `doWithConfig` too. **3. Keeping a plugin bean over a harness default.** The fixture plugin's registrar now also registers `messageSource` as `IncludedMessageSource`, a `StaticMessageSource` subclass. `IncludedPluginBeanOverHarnessDefaultSpec` asserts it. With the keep condition replaced by `if (false)`, that spec fails and the other 34 pass. **4. "Using more than one".** Now reads "the `doWithSpring` and `beanRegistrar` beans of included plugins, which are registered first", as suggested. **5. Nits.** - `IncludedPluginBeansPostProcessor` returns early unless `grailsApplicationPostProcessor` is defined and `isTypeMatch(..., TestRuntimeGrailsApplicationPostProcessor)`. The subclass's post-processor then registers plugin beans in its own `postProcessBeanDefinitionRegistry`, as before this PR. `OtherPostProcessorSpec` pins it: a builder whose `registerGrailsAppPostProcessorBean` registers a plain `GrailsApplicationPostProcessor` subclass builds, and gets `core`'s beans. With the hard-typed lookup restored, that spec fails with exactly the `BeanNotOfRequiredTypeException` you described. - `UNIT_TEST_CONFIGURATION_NAME` is now a constant on the public `GrailsBeans` annotation. The transform and `GrailsUnitTest` both read it there, and `GrailsUnitTest` no longer imports `GrailsBeansASTTransformation`. The transform's copy is gone, so nothing on the compiler class was made public for the trait. Verified with targeted runs on `47835cb838`: - `grails-testing-support-core`: 44 tests, 0 failures. - `GrailsBeansASTTransformationSpec`: 402 tests, 0 failures. - `codeStyle`: clean for both modules. The PR description's Compatibility and Validation sections are updated to match. -- 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]
