codeconsole commented on code in PR #16399:
URL: https://github.com/apache/grails-core/pull/16399#discussion_r4102139151
##########
grails-testing-support-core/src/main/groovy/org/grails/testing/GrailsApplicationBuilder.groovy:
##########
@@ -170,9 +178,18 @@ class GrailsApplicationBuilder {
beanFactory.allowCircularReferences =
environment.getProperty(Settings.SPRING_MAIN_ALLOW_CIRCULAR_REFERENCES,
Boolean, Boolean.TRUE)
def classLoader = this.class.classLoader
+ // The test's own configuration first, as an application's comes
before auto-configuration:
+ // parsed ahead of them, its beans are what an auto-configuration's
@ConditionalOnMissingBean sees.
+ configurationClasses?.each { Class<?> configurationClass ->
Review Comment:
Fixed for the harness defaults in 34aaea15b6. After it registers
`messageSource`, `conversionService` and the rest, `registerBeans` now puts
back whatever the test's configuration declared under one of those names. So a
`beans` block replaces them, as an application's configuration replaces the
framework's auto-configured beans, which back off.
`ReplacingHarnessDefaultsSpec` pins this, and it fails with the restore
disabled.
I've left included plugins' beans as they are, because that is what boot
does. An application `@Bean` does not replace a plugin bean of the same name:
the early phase registers the plugin's definition before the application's
configuration is parsed, and `ConfigurationClassBeanDefinitionReader` then
skips the `@Bean` ("a definition for bean ... already exists. This top-level
bean definition is considered as an override"). I checked this with
`EarlyPluginRegistrationOrderingSpec`'s setup and an application declaring
`overrideProbe` as a `@Bean`: the plugin's `EarlyOrderingPluginResolver` wins.
Only the application's `doWithSpring` or `beanRegistrar` replaces it, as the
existing override spec there shows.
So in a test, an included plugin's bean still wins over a `beans` block
bean. `IncludedPluginBeanOverBeansBlockSpec` pins that,
`BeanRegistrarOverIncludedPluginBeanSpec` pins that the test's
`beanRegistrar()` replaces one, and the unit testing guide now says to use
`beanRegistrar()` for that case.
##########
grails-core/src/main/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformation.groovy:
##########
@@ -387,6 +391,8 @@ class GlobalGrailsClassInjectorTransformation implements
ASTTransformation, Comp
if (beansProperty == null ||
!classNode.getAnnotations(GRAILS_BEANS_ANNOTATION).isEmpty()) {
Review Comment:
It's reported now, on both paths (34aaea15b6): "A unit test's 'beans' block
cannot be @Shared - Spock moves a shared field where the beans DSL cannot
follow it. Remove @Shared: the beans are created once for the test class
whether or not it is there."
I went with reporting it rather than compiling it. Compiling it would mean
undoing more of Spock's rewrite than the moved initializer (the rename and the
accessors it generates), and `@Shared` gains nothing here, since the context is
built once per spec class. `GrailsBeansASTTransformationSpec` covers the
explicit `@GrailsBeans` path, and it no longer reports the misleading "requires
a 'beans' property". `GlobalGrailsClassInjectorTransformationSpec` covers the
convention.
##########
grails-testing-support-core/src/test/resources/META-INF/grails-plugin.xml:
##########
@@ -0,0 +1,3 @@
+<plugin name='includedBeans' version='1.0'>
Review Comment:
Removed in 34aaea15b6. The included-plugin specs pass on the descriptor the
global transform writes into the test output.
--
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]