Repository navigation
Give unit tests beanRegistrar() and configuration classes; deprecate doWithSpring() - #16399
Conversation
bean(Outer.Helper) derived its name from ClassNode#getNameWithoutPackage(), the binary Outer$Helper, and so registered a bean called outer$Helper where the documented default - the type's decapitalized simple name - is helper. Nested types are the usual shape of a test's fixtures, which is how this surfaced. The simple name now comes from the outer class when the type is compiled alongside it, and from Class#getSimpleName() for a type already compiled. No in-tree beans block derives a name from a nested type, so none of their bean names change.
…doWithSpring() Plugins and applications moved off the bean builder DSL to beanRegistrar() (apache#15934, apache#15994) and the beans DSL (apache#16019, apache#16292), but GrailsUnitTest still offered only doWithSpring(), and a test had no way to use the beans DSL at all. - beanRegistrar(): a BeanRegistrar for the test's context, drained where an application's is - after doWithSpring(), so it wins a name conflict with the DSL. - getConfigurationClasses(): configuration classes for the test's context, registered ahead of the framework's auto-configurations as an application's own configuration is, so an auto-configuration's @ConditionalOnMissingBean backs off from them. By default the static nested @configuration classes of the test and of the tests it extends, the convention Spring's own test support follows; a nested @GrailsBeans @configuration class is how a test uses the beans DSL. - getIncludePlugins() now brings in an included plugin's beans block too. The harness registered only org.grails auto-configurations, so a third-party plugin's generated FooAutoConfiguration never reached a unit test, and defineBeans(plugin) cannot apply it after the context has refreshed. - doWithSpring() is deprecated, matching Plugin and GrailsApplicationLifeCycle. The unit testing guide leads with the new hooks; the upgrade guide and What's New note the deprecation.
A beans property on an Application class or a plugin descriptor is compiled by convention, but a test had to wrap its block in a nested @GrailsBeans @configuration class. Annotating the test itself cannot work: Spring would construct an instance of its own to call the @bean methods, outside Spock, where field initializers such as a Mock() cannot run. A class implementing org.grails.testing.GrailsUnitTest - as every Grails testing trait does - now gets the convention too. Its beans compile onto a generated static nested @configuration(proxyBeanMethods = false) class named BeansConfiguration, the shape a group(...) already compiles to, and the testing support registers it like any other nested configuration class, so a base spec's block applies to the specs that extend it. The trait is matched by name, so the beans DSL still does not depend on the testing support. A test that already declares a BeansConfiguration class is reported. Spock moves a specification's field initializers into a generated method before this runs. The closure is taken back from the field = value statement that assigns the beans field - found by the field it assigns, not by Spock's method name - where the block would otherwise have been found empty and skipped without a word.
| 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 -> |
There was a problem hiding this comment.
Registering the test's configuration here means it is parsed before anything TestRuntimeGrailsApplicationPostProcessor writes. The harness defaults from registerBeans (messageSource, proxyHandler, conversionService, ...) and the included plugins' doWithSpring and beanRegistrar beans all land afterwards and win a shared name, so a beans block cannot replace any of them:
class MessageSourceSpec extends Specification implements GrailsUnitTest {
def beans = {
bean('messageSource', MyMessageSource)
}
void "the test's messageSource is used"() {
expect:
applicationContext.getBean('messageSource') instanceof MyMessageSource // fails: StaticMessageSource
}
}The same override through the deprecated doWithSpring() works, because it runs after registerBeans in the same closure. A nested @Configuration class behaves like the beans block, and a bean from an included plugin's doWithSpring or beanRegistrar (core's customEditors, say) replaces the test's in the same way. Only the test's beanRegistrar() gets the last word.
That is the reverse of an application: GrailsEarlyPluginRegistrationPostProcessor drains plugin beans before the application's configuration is parsed, so an application's beans block replaces a plugin bean, as the upgrade guide says. The unit testing guide leads with the beans block for exactly this case ("To provide or replace beans in the context..."), so a test migrating off doWithSpring silently ends up with the default instead.
Could the harness register the default and plugin beans ahead of the test's configuration, the way the early registration phase does for an application? If that is out of scope here, the guide should say that replacing a framework or plugin bean needs beanRegistrar(). Either way, a spec that replaces messageSource and an included plugin's bean from a beans block would pin whichever behavior is chosen.
There was a problem hiding this comment.
Fixed for the harness defaults in 34aaea1. 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.
There was a problem hiding this comment.
Thanks, the harness defaults now behave as an application's would. On names you're right: I ran the early phase the way EarlyPluginRegistrationOrderingSpec does, with IncludedBeansGrailsPlugin discovered and its generated auto-configuration registered after it, and an application @Bean named registeredGreeting loses to the plugin's beanRegistrar() bean there too. The test and the application agree on name precedence.
Conditions still differ, and not only the test's own. The harness registers included plugins' doWithSpring and beanRegistrar beans after the configuration classes are parsed, so an included plugin's conditionalOnMissingBean() beans cannot back off from them either. The fixture shows this now. IncludedBeansGrailsPlugin's registrar registers registeredGreeting, an IncludedGreeting, and its beans block declares bean(IncludedGreeting).conditionalOnMissingBean():
| Context | getBeansOfType(IncludedGreeting) |
|---|---|
| Application (early phase, then the plugin's auto-configuration) | [registeredGreeting] |
Unit test including includedBeans |
[includedGreeting, registeredGreeting] |
So IncludedPluginBeansSpec passes on a bean the plugin would not register in an application. A test that includes such a plugin and asks for the type (getBean(IncludedGreeting), say) gets NoUniqueBeanDefinitionException, where the application has a single bean. This is the back-off the upgrade guide describes under "Plugin beans win @ConditionalOnMissingBean races". The limitation the guide now states for a test's own conditions is the same gap, since an application's configuration does see plugin beans.
Could the harness register included plugins' doWithSpring and beanRegistrar beans before the configuration classes are parsed, as the early phase does? That would close both gaps. If that is out of scope, "Spring configuration from plugins" should say that an included plugin's conditional beans do not back off from plugin beans in a unit test. IncludedPluginBeansSpec should then stop depending on that: give registeredGreeting a type of its own, and pin the difference in a spec named for it.
There was a problem hiding this comment.
I took the second option in 8e96d24 and documented the difference. Registering included plugins' beans before configuration parsing would close both gaps, but it moves the plugin drain for every unit test, not just ones using the new hooks. It would interact with how doWithConfig is applied, for one: the early phase builds its config before a test's doWithConfig runs. I'd rather make that change in its own PR, where it can be judged on its own.
- The test plugin's registrar bean now has its own type,
RegisteredGreeting, soIncludedPluginBeansSpecno longer passes on a bean the plugin wouldn't create in an application. - The plugin also has a conditional fallback of that type.
IncludedPluginConditionalBeansSpecpins the difference: the test gets[registeredGreeting, fallbackGreeting]where an application getsregisteredGreetingalone. TestConditionalBeanOverIncludedPluginSpecpins the same gap for the test's own conditional bean. It also shows that the plugin's fallback does back off from the test's bean, as it would in an application, because the test's configuration is read first.- "Spring configuration from plugins" now has a warning that a
@ConditionalOnMissingBeanbean doesn't back off from an included plugin'sdoWithSpring/beanRegistrarbeans in a unit test, whether it's the plugin's or the test's, and that such beans should be looked up by name.
There was a problem hiding this comment.
I've reconsidered and closed the gap here instead, in 174b768. The harness now registers included plugins' doWithSpring and beanRegistrar beans before the configuration classes are read, as the early phase does.
-
How: it adds a post-processor to the context by hand, which Spring runs ahead of configuration class parsing. That post-processor has the test's
TestRuntimeGrailsApplicationPostProcessorregister the plugin beans, using the plugins it loaded and with the test'sdoWithConfigalready applied. -
grails-core hook:
GrailsApplicationPostProcessorgainsisPluginBeanRegistrationDone(), which is true once the early phase has run. The harness overrides it so its later pass doesn't register the plugin beans twice.applyPluginBeanRegistrarsis now protected for it. -
Harness defaults: they still give way to plugin beans, as before.
-
Specs, now pinning parity:
IncludedPluginConditionalBeansSpec: the plugin's conditionalfallbackGreetingbacks off from its ownregisteredGreeting, and a plugindoWithSpringbean reading config sees the test'sdoWithConfig.TestConditionalBeanOverIncludedPluginSpec: the test's conditional bean backs off.DoWithSpringOverIncludedPluginBeanSpec: a test'sdoWithSpringreplaces a plugin bean of the same name, as an application's does. It used to be the other way round.
The three parity features fail with the early post-processor removed. The warning I'd added to "Spring configuration from plugins" is gone, and the upgrade guide notes the two behaviour changes for existing tests.
Besides this module's specs, I ran EarlyPluginRegistrationOrderingSpec, GSP's ApplicationTagLibTests and several harness users in test-suite-web and -uber. CI has the rest.
| @@ -387,6 +391,8 @@ class GlobalGrailsClassInjectorTransformation implements ASTTransformation, Comp | |||
| if (beansProperty == null || !classNode.getAnnotations(GRAILS_BEANS_ANNOTATION).isEmpty()) { | |||
There was a problem hiding this comment.
A @Shared block is dropped here without a word. Spock renames a shared field to $spock_sharedField_beans and removes the beans property, so getProperty('beans') is null by the time this runs:
class SharedBeansSpec extends Specification implements GrailsUnitTest {
@Shared
def beans = {
bean('greeting', String) { 'hello' }
}
void "the bean is registered"() {
expect:
applicationContext.containsBean('greeting') // false, and no BeansConfiguration is generated
}
}With @GrailsBeans written out it fails instead, with @GrailsBeans requires a 'beans' property initialised to a closure, which points away from the cause. @Shared is a natural thing to reach for, since the context is built once per spec class. Could a unit test's shared beans field either be compiled like an instance one, or be reported with a message that names @Shared? A spec for whichever it is would cover it.
There was a problem hiding this comment.
It's reported now, on both paths (34aaea1): "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.
There was a problem hiding this comment.
Verified at 58b9974: the convention and @GrailsBeans paths both report a @Shared block with the new message, and an unrelated @Shared beans map compiles as before.
| @@ -0,0 +1,3 @@ | |||
| <plugin name='includedBeans' version='1.0'> | |||
There was a problem hiding this comment.
This file looks unnecessary: the global transform already writes META-INF/grails-plugin.xml for IncludedBeansGrailsPlugin into build/classes/groovy/test, since the descriptor is compiled from test sources. With this resource deleted, all of the module's tests still pass, the three included-plugin specs among them. Keeping it puts two descriptors for includedBeans on the test classpath (versions 1.0 and 8.0.0-SNAPSHOT). Can it be removed?
There was a problem hiding this comment.
Removed in 34aaea1. The included-plugin specs pass on the descriptor the global transform writes into the test output.
There was a problem hiding this comment.
Confirmed: the resource is gone, and all 31 of the module's tests pass on the descriptor the transform generates.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #16399 +/- ##
==================================================
+ Coverage 57.5998% 57.8565% +0.2566%
- Complexity 22697 22859 +162
==================================================
Files 2133 2136 +3
Lines 104226 104659 +433
Branches 18692 18807 +115
==================================================
+ Hits 60034 60552 +518
+ Misses 35906 35746 -160
- Partials 8286 8361 +75
🚀 New features to boost your workflow:
|
…ns block The test's configuration is parsed before the harness registers its own stand-ins for framework beans (messageSource, conversionService, proxyHandler, ...), and those were registered over it, so a beans block could not replace them. In an application the framework's beans are auto-configurations that back off from the application's own. The harness now puts back what the test's configuration declared under one of those names. An included plugin's doWithSpring and beanRegistrar beans still win over a beans block bean of the same name. That matches boot: the early phase registers plugin beans before the application's configuration is parsed, and Spring then skips an application @bean whose name is already taken, treating the earlier definition as an override. The unit testing guide says to use beanRegistrar(), applied last, to replace one. Specs pin both. Spock renames a @shared field and removes its property before the beans transform runs, so a @shared beans block was dropped without a word, or, with @GrailsBeans written out, reported as a missing property. It is now reported as @shared, on both paths. The test resources' META-INF/grails-plugin.xml is removed: the global transform already writes one for IncludedBeansGrailsPlugin into the test output, and the two put two descriptors for one plugin on the classpath.
matrei
left a comment
There was a problem hiding this comment.
Review of #16399: Give unit tests beanRegistrar() and configuration classes; deprecate doWithSpring()
Head: 34aaea15b6 (4 commits) · Base: 8.0.x @ 4f1a5c84fd (the merge base, so no rebase is needed) · merges cleanly.
What I ran (in an export of the head, with --no-build-cache and cleanTest; the result XML was freshly written):
:grails-beans-dsl:test: 425 tests, 0 failures:grails-core:test: 685 tests, 0 failures:grails-testing-support-core:test: all 28 of the PR's tests pass, including the newReplacingBeansSpec- Two specs of mine, added to that module and included below:
ReplaceFrameworkBeanSpec(finding 1: 4 tests, 2 failures) andMixedBeanHooksSpec(finding 2: 4 tests, 1 failure)
The design reads well overall. Compiling the block onto a static nested BeansConfiguration sidesteps the "Spring constructs the spec outside Spock" problem. Reusing the group/anonymous-class reach checks for it is the right call. Finding the Spock-moved initializer by the field it assigns, rather than by Spock's method name, is robust.
The fourth commit already covers several things I would have raised:
- a
beansblock now replaces the harness's own defaults, such asmessageSource; - a
@Sharedblock is reported instead of being dropped; - an included plugin's precedence over a
beansblock is documented and pinned; - the duplicate test
grails-plugin.xmlis gone.
What remains:
1. A beans block or nested @Configuration still cannot replace a bean from an org.grails configuration
The new restore step in GrailsApplicationBuilder.groovy:289-296 puts back the test's definitions that the harness's own registerBeans defaults took over. A framework configuration the harness registers, such as CodecsConfiguration, is parsed in the same ConfigurationClassPostProcessor pass as the test's configuration classes. If it declares the same name unconditionally, it overwrites the test's definition right there. So by the time beanDefinitionsDeclaredBy looks, there is no test definition left to restore. CodecsConfiguration.codecLookup is one such bean:
| Hook | Test's codecLookup wins? |
|---|---|
doWithSpring() |
yes |
beanRegistrar() |
yes |
beans block |
no, the framework's DefaultCodecLookup wins |
nested @Configuration |
no, same |
ReplaceFrameworkBeanSpec.groovy: drop into grails-testing-support-core/src/test/groovy/org/grails/testing/. On this head it gives 4 tests, 2 failures: ReplaceFrameworkBeanSpec (beans block) and the configuration-class spec get the framework's DefaultCodecLookup.
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.grails.testing
import org.springframework.beans.factory.BeanRegistrar
import org.springframework.beans.factory.BeanRegistry
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.core.env.Environment
import org.grails.encoder.CodecLookup
import org.grails.plugins.codecs.DefaultCodecLookup
import spock.lang.Specification
/**
* Replaces {@code codecLookup}, which {@code CodecsConfiguration} registers unconditionally (no
* {@code @ConditionalOnMissingBean}), from a {@code beans} block.
*/
class ReplaceFrameworkBeanSpec extends Specification implements GrailsUnitTest {
static class ReplacementCodecLookup extends DefaultCodecLookup {
}
def beans = {
bean('codecLookup', CodecLookup) {
new ReplacementCodecLookup()
}
}
void "a beans block replaces an unconditional framework bean"() {
expect: 'fails: CodecsConfiguration is parsed after BeansConfiguration and overrides codecLookup'
applicationContext.getBean('codecLookup') instanceof ReplacementCodecLookup
}
}
class ReplaceFrameworkBeanWithConfigurationClassSpec extends Specification implements GrailsUnitTest {
@Configuration
static class ReplacementConfiguration {
@Bean('codecLookup')
CodecLookup codecLookup() {
new ReplaceFrameworkBeanSpec.ReplacementCodecLookup()
}
}
void "a nested configuration class replaces an unconditional framework bean"() {
expect: 'fails: CodecsConfiguration is parsed after ReplacementConfiguration and overrides codecLookup'
applicationContext.getBean('codecLookup') instanceof ReplaceFrameworkBeanSpec.ReplacementCodecLookup
}
}
class ReplaceFrameworkBeanWithDoWithSpringSpec extends Specification implements GrailsUnitTest {
Closure doWithSpring() {
{ ->
codecLookup(ReplaceFrameworkBeanSpec.ReplacementCodecLookup)
}
}
void "doWithSpring() replaces an unconditional framework bean"() {
expect:
applicationContext.getBean('codecLookup') instanceof ReplaceFrameworkBeanSpec.ReplacementCodecLookup
}
}
class ReplaceFrameworkBeanWithBeanRegistrarSpec extends Specification implements GrailsUnitTest {
BeanRegistrar beanRegistrar() {
return { BeanRegistry registry, Environment environment ->
registry.registerBean('codecLookup', ReplaceFrameworkBeanSpec.ReplacementCodecLookup)
} as BeanRegistrar
}
void "beanRegistrar() replaces an unconditional framework bean"() {
expect:
applicationContext.getBean('codecLookup') instanceof ReplaceFrameworkBeanSpec.ReplacementCodecLookup
}
}This is how an application's own @Bean relates to an unconditional auto-configuration bean, so the behaviour is defensible. The docs, though, now say more than it does. unitTesting.adoc:54 reads "A bean the block declares replaces one the test's context provides under the same name, such as messageSource, as an application's configuration replaces a framework default". codecLookup is provided by the test's context and is a framework default. unitTesting.adoc:30 ("To provide or replace beans") and the upgrade note steer people away from doWithSpring, and a test migrated that way still silently runs against the framework bean.
Could you either:
- narrow line 54 to the harness's own defaults plus framework beans that back off (
@ConditionalOnMissingBean), and point tobeanRegistrar()for anything else, as it already does for included plugins; - or make the collision visible, for example by failing, or at least logging, when a framework configuration's bean replaces one the test's configuration declared.
Either way, a spec pinning the chosen behaviour for an unconditional framework bean would help. The spec above can be adapted.
2. Mixing hooks in one test is untested, and a beans-block condition does not see the other hooks
Nothing combines a test's beans block with a nested @Configuration, doWithSpring or beanRegistrar() in one test. BeanRegistrarHookSpec covers only doWithSpring + beanRegistrar(), and ReplacingBeansSpec covers a block against an included plugin. The docs present the hooks side by side, and a migrating test will pass through that state. With all four in one test:
- Every hook's unique beans are registered.
- A shared name resolves
beanRegistrar()>doWithSpring>beansblock > nested@Configuration. The last step depends onClass#getDeclaredClasses()order.BeansConfigurationcomes last even when the block is declared before the nested class, but that order is not specified. - A
beans-block.conditionalOnMissingBean()does not back off from a same-type bean registered bybeanRegistrar()(ordoWithSpring). Those are registered after configuration-class parsing, so the condition cannot see them, and the test ends up with two beans of the type. Injection orgetBean(Type)then fails withNoUniqueBeanDefinitionException.
MixedBeanHooksSpec.groovy: drop into grails-testing-support-core/src/test/groovy/org/grails/testing/. On this head it gives 4 tests, 1 failure: the three precedence features pass and pin the current order; the conditional back-off feature fails with both blockWidget and registrarWidget registered.
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.grails.testing
import org.springframework.beans.factory.BeanRegistrar
import org.springframework.beans.factory.BeanRegistry
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.core.env.Environment
import spock.lang.Specification
/**
* One test using every bean registration hook at once. Each hook registers a bean of its own, and
* the shared names are declared by progressively fewer hooks, so each one shows the next step of the
* precedence: {@code beanRegistrar()} over {@code doWithSpring()} over the {@code beans} block over a
* nested {@code @Configuration} class.
*/
class MixedBeanHooksSpec extends Specification implements GrailsUnitTest {
static class Widget {
String source
}
def beans = {
bean('fromBeansBlock', StringBuilder) { new StringBuilder('beans block') }
bean('declaredByAllFour', StringBuilder) { new StringBuilder('beans block') }
bean('declaredByAllButRegistrar', StringBuilder) { new StringBuilder('beans block') }
bean('declaredByBlockAndConfiguration', StringBuilder) { new StringBuilder('beans block') }
bean('blockWidget', Widget).conditionalOnMissingBean() { new Widget(source: 'beans block') }
}
// Declared after the beans block, so source order would put it last
@Configuration
static class NestedConfiguration {
@Bean
StringBuilder fromConfiguration() { new StringBuilder('configuration') }
@Bean
StringBuilder declaredByAllFour() { new StringBuilder('configuration') }
@Bean
StringBuilder declaredByAllButRegistrar() { new StringBuilder('configuration') }
@Bean
StringBuilder declaredByBlockAndConfiguration() { new StringBuilder('configuration') }
}
Closure doWithSpring() {
{ ->
fromDoWithSpring(StringBuilder, 'doWithSpring')
declaredByAllFour(StringBuilder, 'doWithSpring')
declaredByAllButRegistrar(StringBuilder, 'doWithSpring')
}
}
BeanRegistrar beanRegistrar() {
return { BeanRegistry registry, Environment environment ->
registry.registerBean('fromRegistrar', StringBuilder) { BeanRegistry.Spec<StringBuilder> spec ->
spec.supplier { new StringBuilder('registrar') }
}
registry.registerBean('declaredByAllFour', StringBuilder) { BeanRegistry.Spec<StringBuilder> spec ->
spec.supplier { new StringBuilder('registrar') }
}
registry.registerBean('registrarWidget', Widget) { BeanRegistry.Spec<Widget> spec ->
spec.supplier { new Widget(source: 'registrar') }
}
} as BeanRegistrar
}
void "every hook contributes its beans"() {
expect:
applicationContext.getBean('fromBeansBlock').toString() == 'beans block'
applicationContext.getBean('fromConfiguration').toString() == 'configuration'
applicationContext.getBean('fromDoWithSpring').toString() == 'doWithSpring'
applicationContext.getBean('fromRegistrar').toString() == 'registrar'
}
void "a shared name resolves beanRegistrar() over doWithSpring() over the beans block over a nested configuration class"() {
expect:
applicationContext.getBean('declaredByAllFour').toString() == 'registrar'
applicationContext.getBean('declaredByAllButRegistrar').toString() == 'doWithSpring'
applicationContext.getBean('declaredByBlockAndConfiguration').toString() == 'beans block'
}
void "the beans block's class is registered after the nested configuration class, whatever the source order"() {
expect: 'this order comes from Class#getDeclaredClasses(), which does not specify one'
configurationClasses*.simpleName == ['NestedConfiguration', 'BeansConfiguration']
}
void "a conditional bean in the beans block backs off from a bean of its type registered by beanRegistrar()"() {
expect: 'fails: the condition is evaluated before beanRegistrar() runs, so both widgets are registered'
applicationContext.getBeansOfType(Widget).keySet() == ['registrarWidget'] as Set
}
}Please add a spec along these lines. Also say in the unit testing guide which hook wins a shared name, and that conditions in a beans block or configuration class only see configuration-class beans. If the order between BeansConfiguration and hand-written nested classes is meant to be relied on, sort it explicitly (for example, BeansConfiguration last) instead of inheriting getDeclaredClasses() order.
3. Minor: the @Shared report also fires for an unrelated @Shared beans field
GlobalGrailsClassInjectorTransformation.groovy:395 reports any $spock_sharedField_beans on a unit test, without looking at what it holds. A test with an unrelated @Shared def beans = [someKey: 'someValue'] now fails to compile with "A unit test's 'beans' block cannot be @shared" (I checked), where the Compatibility section promises that an unrelated beans property is left alone. Checking that the moved initializer is a closure, or better one of DSL declarations, before reporting would keep that promise.
4. Minor: the initializer is reclaimed before the convention decides whether to claim the property
GlobalGrailsClassInjectorTransformation.groovy:399 calls reclaimMovedInitializer before beansDslStatements/isBeansDslStatement decide whether the closure is DSL-shaped. For a spec with an unrelated def beans = { … } that the convention leaves alone, the closure is still moved out of Spock's $spock_initializeFields and back onto the field, so it runs in the constructor. That is harmless for a closure literal, but the "left alone" promise is not quite literal. Consider inspecting the moved statement first and only moving it once the property is claimed.
5. Minor: bean names derived from nested types change
Commit 1 changes bean(Outer.Helper) from outer$Helper to helper. I agree with the new name, and nothing in-tree depends on the old one. If the beans DSL has shipped in an 8.0 milestone, a one-line note in the 8.0 upgrade guide would help anyone who looked a bean up by the old name.
6. Nits
IncludedBeansGrailsPlugincarries@AutoConfigurationon the descriptor. Since the plugin path generates and annotates the sibling itself, is that needed? If it is only there to exercise annotation moving, a comment would say so.GrailsApplicationBuilder.configurationClassesis aCollectionwhileGrailsUnitTest#getConfigurationClasses()returns aSet. Harmless, but one type would read better.
Verified as correct
- Compatibility claim holds. Across
*/src/test, the only classes that implement a Grails testing trait and declare a nested@Configuration/@AutoConfigurationare this PR's new specs. No existing test has abeansproperty, aconfigurationClassesmember or aregisterPluginDiscoveryBeanoverride. - The restore step only restores the test's own definitions. It matches on the
@Beanmethod's declaring class, the configuration class or a class nested in it, and leaves an included plugin's beans in place, as documented. - Configuration-class discovery.
nestedConfigurationClassesmirrors Spring's default-configuration-class rule: static, non-private, non-final, meta-annotated@Configuration. It walks base classes outermost first, so a subclass wins a shared name. - Registrar ordering.
beanRegistrar()drains inpostProcessBeanDefinitionRegistryaftersuper(the DSL), matching boot. - Included plugins'
beansblocks are registered only when the derived name is also inAutoConfiguration.imports, so a merely matching name is never picked up.registerPluginDiscoveryBeanis idempotent now that it is called twice. simpleName()handles source-compiled nesting viagetOuterClass()and precompiled types viaClass#getSimpleName(), and falls back safely when a type cannot be loaded.isUnitTestmatches the trait by name throughimplementsInterface, sograils-beans-dslgains no dependency on the testing support, and inherited traits count.@Sharedreporting works on both the explicit@GrailsBeanspath and the implicit convention path.
…ompiled it Review follow-ups: - A beans block or nested @configuration does not replace a framework configuration's unconditional bean (CodecsConfiguration's codecLookup), as an application's @bean does not; doWithSpring() and beanRegistrar() do. The unit testing guide said the block replaces any bean the context provides. It now names what gives way to it (the harness's stand-ins and @ConditionalOnMissingBean framework beans) and points to beanRegistrar() for the rest. ReplaceFrameworkBeanSpec pins all four hooks. - MixedBeanHooksSpec uses every hook in one test: each contributes its beans, a shared name resolves beanRegistrar() > doWithSpring() > beans block > nested @configuration, and a condition in the block does not see a beanRegistrar() bean. The guide says both. The class the block compiles to is now put after a test's hand-written nested classes explicitly, rather than by getDeclaredClasses() order, which Java does not specify. - A @shared beans field is reported only when it holds a closure that declares beans; a @shared map or other value is left alone. - The convention reads a Spock-moved closure where it is and moves it back only once the property is claimed, so an unrelated beans closure stays where Spock put it. - The upgrade guide notes that bean(Outer.Helper) is now named helper, not outer$Helper. - GrailsApplicationBuilder.configurationClasses is a Set, like the hook, and the test plugin says why it carries @autoConfiguration.
|
@matrei, thanks for the thorough review. These are addressed in 58b9974:
Locally I ran the specs touched: the testing-support specs, |
jdaugherty
left a comment
There was a problem hiding this comment.
conditional beans still behave differently in tests. Unit tests register included plugins' doWithSpring/beanRegistrar beans too late for any @ConditionalOnMissingBean check to see them. That includes the plugin's own beans block. The PR's latest fixture change shows it.
So IncludedPluginBeansSpec passes on a bean the plugin would never create in an app. A test that looks the type up gets NoUniqueBeanDefinitionException. One of two things:
- register included plugins' beans first in tests, as an app does, or
- document the difference and stop the spec depending on it.
|
|
||
| A test that already has a static nested `@Configuration` class meant for something other than its own context — one built for an `ApplicationContextRunner`, say — now has it registered there as well; override `getConfigurationClasses()` to leave it out. | ||
|
|
||
| **A `beans` block names a nested type's bean by its simple name.** In earlier 8.0 milestones `bean(Outer.Helper)`, with no name given, registered a bean named `outer$Helper`. It is now `helper`, the type's decapitalized simple name, as documented. Name it explicitly, `bean('outer$Helper', Outer.Helper)`, if anything looks it up by the old name. |
There was a problem hiding this comment.
The beans DSL first shipped in 8.0.0-RC1, so "earlier 8.0 milestones" may not register with someone upgrading from the RC:
| **A `beans` block names a nested type's bean by its simple name.** In earlier 8.0 milestones `bean(Outer.Helper)`, with no name given, registered a bean named `outer$Helper`. It is now `helper`, the type's decapitalized simple name, as documented. Name it explicitly, `bean('outer$Helper', Outer.Helper)`, if anything looks it up by the old name. | |
| **A `beans` block names a nested type's bean by its simple name.** In 8.0.0-RC1 `bean(Outer.Helper)`, with no name given, registered a bean named `outer$Helper`. It is now `helper`, the type's decapitalized simple name, as documented. Name it explicitly, `bean('outer$Helper', Outer.Helper)`, if anything looks it up by the old name. |
There was a problem hiding this comment.
Adjusted in 8e96d24, but with one correction: the beans DSL first shipped in 8.0.0-M6, not RC1. #16019 is an ancestor of v8.0.0-M6 (tagged 2026-08-26), and the outer$Helper derivation was there from the start. So the note now reads "In 8.0.0-M6 and 8.0.0-RC1, bean(Outer.Helper) with no name given registered a bean named outer$Helper."
| 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 -> |
There was a problem hiding this comment.
Thanks, the harness defaults now behave as an application's would. On names you're right: I ran the early phase the way EarlyPluginRegistrationOrderingSpec does, with IncludedBeansGrailsPlugin discovered and its generated auto-configuration registered after it, and an application @Bean named registeredGreeting loses to the plugin's beanRegistrar() bean there too. The test and the application agree on name precedence.
Conditions still differ, and not only the test's own. The harness registers included plugins' doWithSpring and beanRegistrar beans after the configuration classes are parsed, so an included plugin's conditionalOnMissingBean() beans cannot back off from them either. The fixture shows this now. IncludedBeansGrailsPlugin's registrar registers registeredGreeting, an IncludedGreeting, and its beans block declares bean(IncludedGreeting).conditionalOnMissingBean():
| Context | getBeansOfType(IncludedGreeting) |
|---|---|
| Application (early phase, then the plugin's auto-configuration) | [registeredGreeting] |
Unit test including includedBeans |
[includedGreeting, registeredGreeting] |
So IncludedPluginBeansSpec passes on a bean the plugin would not register in an application. A test that includes such a plugin and asks for the type (getBean(IncludedGreeting), say) gets NoUniqueBeanDefinitionException, where the application has a single bean. This is the back-off the upgrade guide describes under "Plugin beans win @ConditionalOnMissingBean races". The limitation the guide now states for a test's own conditions is the same gap, since an application's configuration does see plugin beans.
Could the harness register included plugins' doWithSpring and beanRegistrar beans before the configuration classes are parsed, as the early phase does? That would close both gaps. If that is out of scope, "Spring configuration from plugins" should say that an included plugin's conditional beans do not back off from plugin beans in a unit test. IncludedPluginBeansSpec should then stop depending on that: give registeredGreeting a type of its own, and pin the difference in a spec named for it.
| @@ -387,6 +391,8 @@ class GlobalGrailsClassInjectorTransformation implements ASTTransformation, Comp | |||
| if (beansProperty == null || !classNode.getAnnotations(GRAILS_BEANS_ANNOTATION).isEmpty()) { | |||
There was a problem hiding this comment.
Verified at 58b9974: the convention and @GrailsBeans paths both report a @Shared block with the new message, and an unrelated @Shared beans map compiles as before.
| @@ -0,0 +1,3 @@ | |||
| <plugin name='includedBeans' version='1.0'> | |||
There was a problem hiding this comment.
Confirmed: the resource is gone, and all 31 of the module's tests pass on the descriptor the transform generates.
…cation's A unit test registers an included plugin's doWithSpring and beanRegistrar beans after its configuration classes are read; an application's early phase registers them first. So a @ConditionalOnMissingBean bean, the plugin's own or the test's, does not back off from them in a test, and both are registered where an application has one. The test plugin's registrar bean had the type of its beans-block conditional bean, so IncludedPluginBeansSpec passed on a bean the plugin would not create in an application. It now has a type of its own, RegisteredGreeting, and a conditional fallback of that type pins the difference in IncludedPluginConditionalBeansSpec, alongside the test's own conditional bean in TestConditionalBeanOverIncludedPluginSpec. "Spring configuration from plugins" says so and to look such a bean up by name. The upgrade note names the releases that derived outer$Helper: the beans DSL shipped in 8.0.0-M6, so 8.0.0-M6 and 8.0.0-RC1.
…s read An application's early phase registers plugin doWithSpring and beanRegistrar beans before the configuration classes are read, so a @ConditionalOnMissingBean bean backs off from them. The unit-test harness registered them afterwards, from its GrailsApplicationPostProcessor, so in a test neither the framework's auto-configurations, nor an included plugin's own beans block, nor the test's configuration could back off from a plugin bean, and a test that looked the type up could find two where an application has one. The harness now adds a post-processor to the context by hand, which Spring runs ahead of configuration class parsing. It has the test's TestRuntimeGrailsApplicationPostProcessor register the plugin beans there, with the plugins that processor loads and the test's doWithConfig already applied, so a plugin reading config while it registers beans sees it. GrailsApplicationPostProcessor gains isPluginBeanRegistrationDone(), true once the early phase has run, which the harness overrides so its later pass does not register them twice; applyPluginBeanRegistrars is now protected for it. The harness's own defaults keep giving way to plugin beans, as they did. Two things change for existing tests, both to what an application does: a @ConditionalOnMissingBean bean backs off from an included plugin's bean, and a test's doWithSpring bean replaces an included plugin's bean of the same name. The upgrade guide says so. The specs that pinned the difference now pin the parity: IncludedPluginConditionalBeansSpec (a plugin's conditional bean backs off from its own registered bean; the test's doWithConfig reaches the plugin), TestConditionalBeanOverIncludedPluginSpec, and DoWithSpringOverIncludedPluginBeanSpec. All three fail without the early registration.
The harness found the class a plugin's beans block compiles to by the default name only, so a plugin using @GrailsBeans(autoConfigurationName = ...) was not matched through getIncludePlugins(), and the guide told such a test to list the class in getConfigurationClasses() instead. A plugin can only rename the class by writing the annotation out, and the annotation stays in the plugin's class file (CLASS retention). The harness now reads the name from there with Spring's ASM, resolving it as the transform does: a bare name in the plugin's package, a qualified one as written. Nothing changes in what the build generates. GeneratedAutoConfigurationNameSpec checks the default, a qualified and a bare rename against the classes the build generated; the two renames fail without the read. Registering the class once found stays IncludedPluginBeansSpec's.
matrei
left a comment
There was a problem hiding this comment.
Second review of #16399
Head: d68a8b1b7e (8 commits) · Base: 8.0.x @ 4f1a5c84fd · reviewed the changes since 34aaea15b6.
What I ran (in a separate worktree of the head, --no-build-cache):
:grails-testing-support-core:test: all 38 of the PR's tests pass- Three probes, described below: two specs added to that module, and one mutation of
GrailsApplicationBuilder
Thanks for the follow-ups. Everything from my first round is addressed:
- the guide now describes what a
beansblock does not replace, andReplaceFrameworkBeanSpecpins it; MixedBeanHooksSpecpins mixed hooks, the guide gives the precedence, andBeansConfigurationis now sorted last explicitly;- the
@Sharedreport checks for DSL declarations first; - the reclaim now happens only after the property is claimed;
- the upgrade note is in;
- both nits are done.
Registering included plugins' beans ahead of the configuration classes (174b7680d9) is a good call too. It removes the conditional-bean gap rather than documenting it.
What remains:
1. Since 174b7680d9, a beans block no longer replaces the proxyHandler stand-in
CoreGrailsPlugin's registrar registers proxyHandler only as a fallback, when no definition exists yet. The core plugin is always included, and its registrar now runs before the test's configuration is read, so nothing is there yet and the fallback is always registered. The test's @Bean is then skipped, since a plugin's definition wins over it. In the harness's registerBeans, proxyHandler is restored as a plugin bean, not as the test's.
A probe spec with bean('proxyHandler', ProbeProxyHandler) and bean('messageSource', TestMessageSource) in a beans block:
| Commit | messageSource replaced |
proxyHandler replaced |
|---|---|---|
8e96d24249 |
yes | yes |
d68a8b1b7e |
yes | no, DefaultProxyHandler |
Going by the code, an application's @Bean proxyHandler meets the same fallback in the early phase (I have not booted an application to confirm). So the behaviour itself follows the parity argument. unitTesting.adoc:54, though, says the block replaces "the context's own stand-ins for framework beans, such as messageSource", and proxyHandler is one of those stand-ins. Could the sentence exclude a stand-in that an included plugin also registers, naming proxyHandler from core, and a spec pin it?
2. doWithConfig does not reach a condition in a beans block
doWithConfig changes grailsApplication.config and reaches ${...} placeholders through grailsPlaceholderConfigurer. It never reaches the Spring Environment, which is what .conditionalOnProperty(...) and .conditionalOnExpression(...) are evaluated against. A probe:
class ProbeDoWithConfigConditionSpec extends Specification implements GrailsUnitTest {
Closure doWithConfig() {
{ config -> config.'probe.feature.enabled' = 'true' }
}
def beans = {
bean('probeFeature', String).conditionalOnProperty('probe.feature.enabled', havingValue: 'true') { 'on' }
bean('probeValue', String) { @Value('${probe.feature.enabled:unset}') String v -> v }
}
}config.getProperty('probe.feature.enabled') == 'true': passesapplicationContext.getBean('probeValue') == 'true': passesapplicationContext.environment.getProperty('probe.feature.enabled'):nullapplicationContext.containsBean('probeFeature'): false
The same was already true of the framework's own @ConditionalOnProperty auto-configurations, so this is not a regression. But the PR gives tests the DSL's property conditions, and unitTesting.adoc:152 now says included plugins register their beans "after the test's doWithConfig is applied". A reader could take that to cover a plugin's beans-block conditions too. The toggle is the obvious thing to reach for in a test, and it fails silently.
Could doWithConfig's values be added to the environment as a property source before the configuration classes are read? If that is out of scope, a note in the guide would do: conditions read the Spring environment, so set their properties in application.yml under the test resources, or as system properties, not in doWithConfig.
3. Keeping a plugin bean over a harness default is not pinned
GrailsApplicationBuilder.groovy:337-341 keeps what the included plugins registered under a harness default's name. With that condition replaced by if (false), all 38 of the module's tests still pass. The only in-tree case, core's proxyHandler, has the same class as the harness default, so the two cannot be told apart. A spec where the fixture plugin registers a harness-default name with a type of its own (messageSource, say) would pin it.
4. Doc: "the beans of included plugins" in "Using more than one"
unitTesting.adoc:113 says a condition on one of the test's beans sees "the beans of included plugins, which are registered first". That holds for a plugin's doWithSpring and beanRegistrar beans only. A plugin's beans block is an auto-configuration, read after the test's configuration, and, as TestConditionalBeanOverIncludedPluginSpec shows, it backs off from the test's bean rather than the reverse. Suggest: "the doWithSpring and beanRegistrar beans of included plugins, which are registered first".
5. Nits
GrailsApplicationBuilder.groovy:447looks upgrailsApplicationPostProcessoras aTestRuntimeGrailsApplicationPostProcessor.registerGrailsAppPostProcessorBeanis a protected extension point, and a subclass that registers a different post-processor now fails on refresh withBeanNotOfRequiredTypeException. Nothing in-tree does this. Skipping the early step when the bean is not of that type would keep the hook usable.GrailsUnitTestnow importsGrailsBeansASTTransformation, a compiler class, andUNIT_TEST_CONFIGURATION_NAMEbecame public only for this. A constant on the testing side, or one on the publicGrailsBeans, would keep the runtime trait away from the transform.
Verified as correct
- Harness defaults vs plugin beans. "As before" holds: on the base, plugin
doWithSpringbeans were flushed afterregisterBeanswrote the defaults, and plugin registrars were applied after that, so plugins already won. - No double registration. The harness's
isPluginBeanRegistrationDone()override takes effect in the base'spluginBeanRegistrationDonechecks. If it didn't, the plugin registrars would run again after the test'sdoWithSpring, andDoWithSpringOverIncludedPluginBeanSpecwould fail. doWithConfigbefore plugindoWithSpring. Getting the post-processor early triggerssetApplicationContext, thencustomizeGrailsApplication, beforeregisterPluginBeans, asIncludedPluginConditionalBeansSpecshows. (This coversgrailsApplication.config, not theEnvironment; see 2.)- Renamed generated classes. The class-file read resolves a qualified and a bare
autoConfigurationName(pinned byGeneratedAutoConfigurationNameSpec), and falls back to the default name when the class file cannot be read.
- Publish what a test's doWithConfig changes to the Spring Environment as a property source, ahead of the others, before the configuration classes are read, so .conditionalOnProperty(...) and the framework's @ConditionalOnProperty see it (DoWithConfigEnvironmentSpec) - Document that a beans block does not replace a stand-in an included plugin also registers (core's proxyHandler), pin it, and pin that beanRegistrar() does - Pin that an included plugin's bean wins over a harness default of the same name (the fixture plugin registers its own messageSource) - Skip the early plugin registration when grailsApplicationPostProcessor is not the harness's, so registerGrailsAppPostProcessorBean stays a usable extension point (OtherPostProcessorSpec) - Move UNIT_TEST_CONFIGURATION_NAME to the public GrailsBeans annotation, so GrailsUnitTest no longer imports the transform - Guide: a condition on a test bean sees the doWithSpring and beanRegistrar beans of included plugins
|
@matrei thanks for the second pass, and for the probes. All five are addressed in 1.
2.
3. Keeping a plugin bean over a harness default. The fixture plugin's registrar now also registers 4. "Using more than one". Now reads "the 5. Nits.
Verified with targeted runs on
The PR description's Compatibility and Validation sections are updated to match. |
This comment has been minimized.
This comment has been minimized.
matrei
left a comment
There was a problem hiding this comment.
Third review of #16399
Head: 47835cb838 (9 commits) · Base: 8.0.x @ 4f1a5c84fd · reviewed the changes since d68a8b1b7e.
What I ran (in a separate worktree of the head, --no-build-cache):
:grails-testing-support-core:test: all 44 of the module's tests pass, plus a probe spec (below)- Three mutations of
GrailsApplicationBuilder, each run against the spec that should catch it - CodeNarc (main and test) and Checkstyle for
grails-testing-support-core, and Checkstyle forgrails-beans-dsl: no violations
Thanks, everything from the second round is addressed:
proxyHandler. The guide now says abeansblock does not replace a stand-in that an included plugin also registers.ReplacingHarnessDefaultsSpecpins that, andBeanRegistrarOverPluginStandInSpecpins thatbeanRegistrar()still replaces it.doWithConfigand theEnvironment. WhatdoWithConfigchanges is now added as the first property source, before the configuration classes are read. My probe checked it beyond the flat string key inDoWithConfigEnvironmentSpec. A nested map (config.probe = [nested: [flag: true], ...]) reaches a@ConditionalOnPropertyon a hand-written nested@Configurationclass. A number reaches an@Value Integer, and a list reaches both@Value Listand the indexedprobe.list[1]key.- Plugin bean over a harness default. This is pinned by
IncludedPluginBeanOverHarnessDefaultSpec. - The guide sentence on conditions now names the
doWithSpringandbeanRegistrarbeans of included plugins. - Nits. The lookup of
grailsApplicationPostProcessoris no longer hard-typed, andUNIT_TEST_CONFIGURATION_NAMEmoved toGrailsBeans.
Each new spec fails without its fix:
| Mutation | Spec | Result |
|---|---|---|
publishToEnvironment(...) call removed |
DoWithConfigEnvironmentSpec |
fails |
type check before the post-processor lookup replaced by if (false) |
OtherPostProcessorSpec |
fails |
keep condition in registerBeans replaced by if (false) |
IncludedPluginBeanOverHarnessDefaultSpec |
fails |
One doc nit remains, not blocking:
unitTesting.adoc:54says "thecoreplugin, which is always included". The same page's WARNING under "Spring configuration from plugins" says that overridinggetIncludePlugins()drops the defaults,coreamong them.corestill comes in as a dependency of most plugins (controllers, say), but not always. "which is included by default" would be accurate. The PR description has the same wording ("the always-includedcoreplugin").
Approving. The nit can be fixed before merge.
|
@matrei thanks for the approval and the extra probes on nested maps, numbers and lists. The nit is fixed in |
Plugins and applications moved off the bean builder DSL to
beanRegistrar()(#15934, #15994) and thebeansDSL (#16019, #16292), but unit tests were left ondoWithSpring().GrailsUnitTesthad nobeanRegistrar()hook, and a test had no way to use thebeansDSL at all. Including a plugin throughgetIncludePlugins()also stopped bringing in all of its beans once they moved into abeansblock, because the test harness registers onlyorg.grailsauto-configurations, so a third-party plugin's generatedFooAutoConfigurationnever reached a unit test.defineBeans(plugin)cannot fill that gap: abeansblock is an auto-configuration, and the context has already refreshed by then.With this, a test declares its beans the way an application or a plugin does:
Commits: the three below, then review follow-ups (harness defaults,
@Shared, hook precedence, registering included plugins' beans first, anddoWithConfigreaching the SpringEnvironment).1. Name a nested type's bean by its simple name in the
beansDSLbean(Outer.Helper)derived its name fromClassNode#getNameWithoutPackage(), the binaryOuter$Helper, so it registeredouter$Helperrather than the documented default, the type's decapitalized simple namehelper. Nested types are the usual shape of a test's fixtures, which is how this surfaced. The simple name now comes from the outer class when the type is compiled alongside it, and fromClass#getSimpleName()for a type that is already compiled. No in-treebeansblock derives a name from a nested type, so none of their bean names change.2. Give unit tests
beanRegistrar()and configuration classes; deprecatedoWithSpring()beanRegistrar()onGrailsUnitTest: aBeanRegistrarfor the test's context, applied where an application's is at boot, afterdoWithSpring(), so a registrar bean wins a name conflict with the DSL.getConfigurationClasses(): configuration classes registered ahead of the framework's auto-configurations, as an application's own configuration is, so an auto-configuration's@ConditionalOnMissingBeanbacks off from the beans they declare. By default these are the static nested@Configurationclasses of the test and of any test it extends, following the convention Spring's own test support uses. Overriding it registers the returned classes instead.beansblocks. For each plugingetIncludePlugins()includes, the class itsbeansblock compiles to is registered with the auto-configurations. The name is the one@GrailsBeansgave it: the plugin'sautoConfigurationName, read from its class file since the annotation has CLASS retention, or else the default (FooGrailsPlugin→FooAutoConfiguration). A class is registered only if the build listed it in the auto-configuration imports.doWithSpring()onGrailsUnitTestis deprecated, matchingPluginandGrailsApplicationLifeCycle.3. Compile a unit test's
beansblock without an annotationA
beansproperty on anApplicationclass or a plugin descriptor is compiled by convention; this extends the convention to any class implementingorg.grails.testing.GrailsUnitTest, which every Grails testing trait does. The trait is matched by name, so the beans DSL still does not depend on the testing support.@Beanmethods: Spring would construct an instance of its own to call them, outside Spock, where field initializers such as aMock()cannot run.@Configuration(proxyBeanMethods = false)class,BeansConfiguration. That is the shape agroup(...)already compiles to, with the same sibling-call and anonymous-class checks. Commit 2 registers it like any nested configuration class, so a base spec's block applies to the specs that extend it.BeansConfigurationclass is reported.beansblock cannot reach its descriptor's. The unit testing guide says to declare dependencies as closure parameters.field = valuestatement that assigns thebeansfield. That statement is found by the field it assigns, not by Spock's method name. Without this the block would be found empty and skipped silently.The unit testing guide now leads with the
beansblock, thenbeanRegistrar(), then configuration classes, withdoWithSpringmarked deprecated. The@GrailsBeansJavadoc and the Spring chapter describe the unit-test host. The upgrade guide and What's New cover the deprecation and the new forms.Compatibility
doWithConfigalready applied. Two things change for existing tests, both to what an application does. A@ConditionalOnMissingBeanbean (the framework's, an included plugin's own, or the test's) now backs off from an included plugin'sdoWithSpringorbeanRegistrarbean. And a test'sdoWithSpringbean now replaces an included plugin's bean of the same name, where the plugin's used to win.GrailsApplicationPostProcessorgains a protectedisPluginBeanRegistrationDone()for this, andapplyPluginBeanRegistrarsis now protected.beanRegistrar(),doWithSpring(), an included plugin's bean, thebeansblock, a nested@Configurationclass. A framework configuration's unconditional bean also wins over thebeansblock, as over an application's@Bean. The harness's own defaults, such asmessageSource, give way to the test's configuration and to plugin beans.proxyHandleris one of those defaults, but thecoreplugin, included by default, registers it too, and the plugin's bean wins, so abeansblock does not replace it;beanRegistrar()does.doWithConfigsets now reaches the SpringEnvironment, as a property source ahead of the others, added before the configuration classes are read. A property condition sees it:.conditionalOnProperty(...)in a test's or an included plugin'sbeansblock, and the framework's@ConditionalOnProperty. It used to reach onlygrailsApplication.configand${}placeholders, so a condition on it failed without any error.@Configurationclass meant for something else (anApplicationContextRunner, say) now has it registered in its own context too. The upgrade guide says to overridegetConfigurationClasses()to leave it out. No test in this repository is affected.beansproperty is treated as the convention treats applications and plugins: left alone unless the closure contains DSL declarations, and reported if only some of its statements are declarations. No test in this repository has abeansproperty.GrailsApplicationBuildergainsbeanRegistrarandconfigurationClassesproperties, andregisterPluginDiscoveryBeanreturns the registered discovery if it is asked twice. The discovery is now created before the auto-configurations are chosen. A subclass whoseregisterGrailsAppPostProcessorBeanregisters a post-processor of another type skips the early plugin registration and keeps that post-processor's own.GrailsBeansgains the constantUNIT_TEST_CONFIGURATION_NAME("BeansConfiguration"), which the testing support reads, so the runtime trait does not depend on the transform.Validation
At commit 3 (
a3c673e), module and regression suites:grails-beans-dsl424,grails-core684,grails-testing-support-core20, databinding 33, GSP 666, test-suite-web 435, -uber 576, -persistence 101, beans-dsl examples 11. All had 0 failures.The review follow-ups since then were checked with targeted specs, and CI runs the full suites on the head. The last one (
174b7680d9) changes when every unit test registers included plugins' beans, so CI is what proves it broadly.grails-testing-support-core: all 44 tests pass on47835cb838.GrailsBeansASTTransformationSpec(402) andGlobalGrailsClassInjectorTransformationSpec(56) pass.174b7680d9:EarlyPluginRegistrationOrderingSpec, GSP'sApplicationTagLibTests,SpyBeanSpec,StaticCallbacksSpec,TestInstanceCallbacksSpec,GrailsUnitTestMixinGrailsApplicationAwareSpec,ChainMethodWithRequestDataValueProcessorSpec,TagLibWithServiceMockTests,CommandObjectsSpecandMarshallerRegistrarSpec.GeneratedAutoConfigurationNameSpecchecks the default name, a qualified rename and a bare rename against the classes the build generated. The two renames fail without the class-file read.@Sharedreport and its DSL check, the restored harness defaults, configuration-before-auto-configuration order, and the three plugin-parity specs. From the second review, each of these also fails without its fix:DoWithConfigEnvironmentSpec, without the property source;IncludedPluginBeanOverHarnessDefaultSpec, with the keep condition replaced byif (false);OtherPostProcessorSpec, with the hard-typed lookup (BeanNotOfRequiredTypeException);proxyHandlerfeature ofReplacingHarnessDefaultsSpec, with the early registration skipped.grails-beans-dsl,grails-coreorgrails-testing-support-core; re-run forgrails-beans-dslandgrails-testing-support-coreon47835cb838.