Skip to content

Give unit tests beanRegistrar() and configuration classes; deprecate doWithSpring() - #16399

Merged
jdaugherty merged 10 commits into
apache:8.0.xfrom
codeconsole:feat/testing-bean-registrar-8.0.x
Sep 26, 2026
Merged

jdaugherty merged 10 commits into
apache:8.0.xfrom
codeconsole:feat/testing-bean-registrar-8.0.x

Conversation

@codeconsole

@codeconsole codeconsole commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Plugins and applications moved off the bean builder DSL to beanRegistrar() (#15934, #15994) and the beans DSL (#16019, #16292), but unit tests were left on doWithSpring(). GrailsUnitTest had no beanRegistrar() hook, and a test had no way to use the beans DSL at all. Including a plugin through getIncludePlugins() also stopped bringing in all of its beans once they moved into a beans block, because the test harness registers only org.grails auto-configurations, so a third-party plugin's generated FooAutoConfiguration never reached a unit test. defineBeans(plugin) cannot fill that gap: a beans block 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:

class ReportServiceSpec extends Specification implements ServiceUnitTest<ReportService> {

    def beans = {
        bean(SomeHelper)
        bean('reportCache', ReportCache) { SomeHelper someHelper -> }
    }
}

Commits: the three below, then review follow-ups (harness defaults, @Shared, hook precedence, registering included plugins' beans first, and doWithConfig reaching the Spring Environment).

1. Name a nested type's bean by its simple name in the beans DSL

bean(Outer.Helper) derived its name from ClassNode#getNameWithoutPackage(), the binary Outer$Helper, so it registered outer$Helper rather than the documented default, the type's decapitalized simple name 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 that is already compiled. No in-tree beans block derives a name from a nested type, so none of their bean names change.

2. Give unit tests beanRegistrar() and configuration classes; deprecate doWithSpring()

  • beanRegistrar() on GrailsUnitTest: a BeanRegistrar for the test's context, applied where an application's is at boot, after doWithSpring(), 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 @ConditionalOnMissingBean backs off from the beans they declare. By default these are the static nested @Configuration classes 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.
  • Included plugins' beans blocks. For each plugin getIncludePlugins() includes, the class its beans block compiles to is registered with the auto-configurations. The name is the one @GrailsBeans gave it: the plugin's autoConfigurationName, 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() on GrailsUnitTest is deprecated, matching Plugin and GrailsApplicationLifeCycle.

3. Compile a unit test's beans block without an annotation

A beans property on an Application class or a plugin descriptor is compiled by convention; this extends the convention to any class implementing org.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.

  • The test class itself cannot host the @Bean methods: Spring would construct an instance of its own to call them, outside Spock, where field initializers such as a Mock() cannot run.
  • So a test's beans compile onto a generated static nested @Configuration(proxyBeanMethods = false) class, BeansConfiguration. That is the shape a group(...) 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.
  • A test that already declares a BeansConfiguration class is reported.
  • A bean body cannot reach the test's own fields or methods, as a plugin's beans block cannot reach its descriptor's. The unit testing guide says to declare dependencies as closure parameters.
  • Spock moves a specification's field initializers into a generated method before the transform runs. The closure is taken back from the field = value statement that assigns the beans field. 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 beans block, then beanRegistrar(), then configuration classes, with doWithSpring marked deprecated. The @GrailsBeans Javadoc and the Spring chapter describe the unit-test host. The upgrade guide and What's New cover the deprecation and the new forms.

Compatibility

  • Included plugins' beans are registered before the test's configuration is read, as an application's early phase registers them, with the test's doWithConfig already applied. Two things change for existing tests, both to what an application does. A @ConditionalOnMissingBean bean (the framework's, an included plugin's own, or the test's) now backs off from an included plugin's doWithSpring or beanRegistrar bean. And a test's doWithSpring bean now replaces an included plugin's bean of the same name, where the plugin's used to win. GrailsApplicationPostProcessor gains a protected isPluginBeanRegistrationDone() for this, and applyPluginBeanRegistrars is now protected.
  • Name precedence in a test matches an application. From last to first: beanRegistrar(), doWithSpring(), an included plugin's bean, the beans block, a nested @Configuration class. A framework configuration's unconditional bean also wins over the beans block, as over an application's @Bean. The harness's own defaults, such as messageSource, give way to the test's configuration and to plugin beans. proxyHandler is one of those defaults, but the core plugin, included by default, registers it too, and the plugin's bean wins, so a beans block does not replace it; beanRegistrar() does.
  • What doWithConfig sets now reaches the Spring Environment, 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's beans block, and the framework's @ConditionalOnProperty. It used to reach only grailsApplication.config and ${} placeholders, so a condition on it failed without any error.
  • A test that already has a static nested @Configuration class meant for something else (an ApplicationContextRunner, say) now has it registered in its own context too. The upgrade guide says to override getConfigurationClasses() to leave it out. No test in this repository is affected.
  • A test with an unrelated beans property 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 a beans property.
  • GrailsApplicationBuilder gains beanRegistrar and configurationClasses properties, and registerPluginDiscoveryBean returns the registered discovery if it is asked twice. The discovery is now created before the auto-configurations are chosen. A subclass whose registerGrailsAppPostProcessorBean registers a post-processor of another type skips the early plugin registration and keeps that post-processor's own.
  • GrailsBeans gains the constant UNIT_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-dsl 424, grails-core 684, grails-testing-support-core 20, 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 on 47835cb838.
  • GrailsBeansASTTransformationSpec (402) and GlobalGrailsClassInjectorTransformationSpec (56) pass.
  • Ten existing harness users pass on 174b7680d9: EarlyPluginRegistrationOrderingSpec, GSP's ApplicationTagLibTests, SpyBeanSpec, StaticCallbacksSpec, TestInstanceCallbacksSpec, GrailsUnitTestMixinGrailsApplicationAwareSpec, ChainMethodWithRequestDataValueProcessorSpec, TagLibWithServiceMockTests, CommandObjectsSpec and MarshallerRegistrarSpec.
  • GeneratedAutoConfigurationNameSpec checks 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.
  • Each fix has a spec that fails without it. That covers the nested-type name, the implicit convention for a Spock spec, the reclaimed initializer, the claim-before-reclaim order, the @Shared report 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 by if (false);
    • OtherPostProcessorSpec, with the hard-typed lookup (BeanNotOfRequiredTypeException);
    • the proxyHandler feature of ReplacingHarnessDefaultsSpec, with the early registration skipped.
  • Checkstyle and CodeNarc: no violations in grails-beans-dsl, grails-core or grails-testing-support-core; re-run for grails-beans-dsl and grails-testing-support-core on 47835cb838.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, so IncludedPluginBeansSpec no longer passes on a bean the plugin wouldn't create in an application.
  • The plugin also has a conditional fallback of that type. IncludedPluginConditionalBeansSpec pins the difference: the test gets [registeredGreeting, fallbackGreeting] where an application gets registeredGreeting alone.
  • TestConditionalBeanOverIncludedPluginSpec pins 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 @ConditionalOnMissingBean bean doesn't back off from an included plugin's doWithSpring/beanRegistrar beans in a unit test, whether it's the plugin's or the test's, and that such beans should be looked up by name.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 TestRuntimeGrailsApplicationPostProcessor register the plugin beans, using the plugins it loaded and with the test's doWithConfig already applied.

  • grails-core hook: GrailsApplicationPostProcessor gains isPluginBeanRegistrationDone(), which is true once the early phase has run. The harness overrides it so its later pass doesn't register the plugin beans twice. applyPluginBeanRegistrars is now protected for it.

  • Harness defaults: they still give way to plugin beans, as before.

  • Specs, now pinning parity:

    • IncludedPluginConditionalBeansSpec: the plugin's conditional fallbackGreeting backs off from its own registeredGreeting, and a plugin doWithSpring bean reading config sees the test's doWithConfig.
    • TestConditionalBeanOverIncludedPluginSpec: the test's conditional bean backs off.
    • DoWithSpringOverIncludedPluginBeanSpec: a test's doWithSpring replaces 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()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in 34aaea1. The included-plugin specs pass on the descriptor the global transform writes into the test output.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed: the resource is gone, and all 31 of the module's tests pass on the descriptor the transform generates.

@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.07018% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.8565%. Comparing base (4f1a5c8) to head (a17acb1).
⚠️ Report is 97 commits behind head on 8.0.x.

Files with missing lines Patch % Lines
...s/compiler/beans/GrailsBeansASTTransformation.java 76.4151% 11 Missing and 14 partials ⚠️
...org/grails/testing/GrailsApplicationBuilder.groovy 78.0952% 4 Missing and 19 partials ⚠️
...ion/GlobalGrailsClassInjectorTransformation.groovy 90.0000% 0 Missing and 1 partial ⚠️
...in/groovy/org/grails/testing/GrailsUnitTest.groovy 75.0000% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@                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     
Files with missing lines Coverage Δ
.../boot/config/GrailsApplicationPostProcessor.groovy 73.3728% <100.0000%> (+0.1585%) ⬆️
...ion/GlobalGrailsClassInjectorTransformation.groovy 85.9438% <90.0000%> (+0.4066%) ⬆️
...in/groovy/org/grails/testing/GrailsUnitTest.groovy 75.0000% <75.0000%> (ø)
...org/grails/testing/GrailsApplicationBuilder.groovy 83.9806% <78.0952%> (-3.6385%) ⬇️
...s/compiler/beans/GrailsBeansASTTransformation.java 84.2313% <76.4151%> (-0.5453%) ⬇️

... and 19 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…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 matrei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 new ReplacingBeansSpec
  • Two specs of mine, added to that module and included below: ReplaceFrameworkBeanSpec (finding 1: 4 tests, 2 failures) and MixedBeanHooksSpec (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 beans block now replaces the harness's own defaults, such as messageSource;
  • a @Shared block is reported instead of being dropped;
  • an included plugin's precedence over a beans block is documented and pinned;
  • the duplicate test grails-plugin.xml is 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 to beanRegistrar() 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 > beans block > nested @Configuration. The last step depends on Class#getDeclaredClasses() order. BeansConfiguration comes 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 by beanRegistrar() (or doWithSpring). 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 or getBean(Type) then fails with NoUniqueBeanDefinitionException.
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

  • IncludedBeansGrailsPlugin carries @AutoConfiguration on 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.configurationClasses is a Collection while GrailsUnitTest#getConfigurationClasses() returns a Set. 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/@AutoConfiguration are this PR's new specs. No existing test has a beans property, a configurationClasses member or a registerPluginDiscoveryBean override.
  • The restore step only restores the test's own definitions. It matches on the @Bean method'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. nestedConfigurationClasses mirrors 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 in postProcessBeanDefinitionRegistry after super (the DSL), matching boot.
  • Included plugins' beans blocks are registered only when the derived name is also in AutoConfiguration.imports, so a merely matching name is never picked up. registerPluginDiscoveryBean is idempotent now that it is called twice.
  • simpleName() handles source-compiled nesting via getOuterClass() and precompiled types via Class#getSimpleName(), and falls back safely when a type cannot be loaded.
  • isUnitTest matches the trait by name through implementsInterface, so grails-beans-dsl gains no dependency on the testing support, and inherited traits count.
  • @Shared reporting works on both the explicit @GrailsBeans path 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.
@codeconsole

Copy link
Copy Markdown
Contributor Author

@matrei, thanks for the thorough review. These are addressed in 58b9974:

  1. Unconditional framework beans. I kept the behaviour and narrowed the docs, since an application's @Bean relates to an unconditional configuration bean the same way. The guide now says a beans block replaces the harness's stand-ins (such as messageSource) and framework beans declared @ConditionalOnMissingBean. It also says it does not replace a framework configuration's unconditional bean (such as codecLookup) or an included plugin's bean, and points to beanRegistrar() for those. ReplaceFrameworkBeanSpec, adapted from yours, pins all four hooks: the beans block and the nested @Configuration get DefaultCodecLookup, while doWithSpring() and beanRegistrar() replace it.
  2. Mixing hooks. MixedBeanHooksSpec, adapted from yours, pins four things: every hook contributes its beans; a shared name resolves beanRegistrar() > doWithSpring() > beans block > nested @Configuration; the generated class comes after the hand-written nested classes; and a condition in the block does not see a beanRegistrar() bean, so both widgets are registered. A new "Using more than one" section in the guide gives the precedence, and says that a condition on one of the test's beans sees only the test's configuration classes read before it. BeansConfiguration is now placed after a test's hand-written nested classes explicitly (a stable sort per class level), not by getDeclaredClasses() order.
  3. @Shared report. It now fires only when the moved initializer is a closure that declares beans (a bean/field/method/group call). A @Shared def beans = [someKey: 'someValue'] compiles as before. There's a spec in GlobalGrailsClassInjectorTransformationSpec.
  4. Reclaim before claim. The convention now reads the Spock-moved closure where it is, via a new movedInitializer, and moves it back only once the property is claimed. An unrelated def beans = { … } keeps its initializer in $spock_initializeFields. The spec fails if the reclaim is moved back ahead of the claim decision.
  5. Nested type names. Added a note to the 8.0 upgrade guide: bean(Outer.Helper) is now helper, not outer$Helper, so name it explicitly if anything looks it up by the old name.
  6. Nits. @AutoConfiguration on IncludedBeansGrailsPlugin is required: a Plugin using @GrailsBeans must carry it, and the transform moves it onto the generated sibling. A comment now says so. GrailsApplicationBuilder.configurationClasses is now a Set<Class<?>>, like the hook.

Locally I ran the specs touched: the testing-support specs, GlobalGrailsClassInjectorTransformationSpec (56) and GrailsBeansASTTransformationSpec (402), all passing, and codeStyle is clean in all three modules. CI covers the rest.

@jdaugherty jdaugherty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The beans DSL first shipped in 8.0.0-RC1, so "earlier 8.0 milestones" may not register with someone upgrading from the RC:

Suggested change
**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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 matrei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 beans block does not replace, and ReplaceFrameworkBeanSpec pins it;
  • MixedBeanHooksSpec pins mixed hooks, the guide gives the precedence, and BeansConfiguration is now sorted last explicitly;
  • the @Shared report 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': passes
  • applicationContext.getBean('probeValue') == 'true': passes
  • applicationContext.environment.getProperty('probe.feature.enabled'): null
  • applicationContext.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:447 looks up grailsApplicationPostProcessor as a TestRuntimeGrailsApplicationPostProcessor. registerGrailsAppPostProcessorBean is a protected extension point, and a subclass that registers a different post-processor now fails on refresh with BeanNotOfRequiredTypeException. Nothing in-tree does this. Skipping the early step when the bean is not of that type would keep the hook usable.
  • GrailsUnitTest now imports GrailsBeansASTTransformation, a compiler class, and UNIT_TEST_CONFIGURATION_NAME became public only for this. A constant on the testing side, or one on the public GrailsBeans, would keep the runtime trait away from the transform.

Verified as correct

  • Harness defaults vs plugin beans. "As before" holds: on the base, plugin doWithSpring beans were flushed after registerBeans wrote 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's pluginBeanRegistrationDone checks. If it didn't, the plugin registrars would run again after the test's doWithSpring, and DoWithSpringOverIncludedPluginBeanSpec would fail.
  • doWithConfig before plugin doWithSpring. Getting the post-processor early triggers setApplicationContext, then customizeGrailsApplication, before registerPluginBeans, as IncludedPluginConditionalBeansSpec shows. (This covers grailsApplication.config, not the Environment; see 2.)
  • Renamed generated classes. The class-file read resolves a qualified and a bare autoConfigurationName (pinned by GeneratedAutoConfigurationNameSpec), 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
@codeconsole

Copy link
Copy Markdown
Contributor Author

@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.

@testlens-app

This comment has been minimized.

@matrei matrei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 for grails-beans-dsl: no violations

Thanks, everything from the second round is addressed:

  1. proxyHandler. The guide now says a beans block does not replace a stand-in that an included plugin also registers. ReplacingHarnessDefaultsSpec pins that, and BeanRegistrarOverPluginStandInSpec pins that beanRegistrar() still replaces it.
  2. doWithConfig and the Environment. What doWithConfig changes is now added as the first property source, before the configuration classes are read. My probe checked it beyond the flat string key in DoWithConfigEnvironmentSpec. A nested map (config.probe = [nested: [flag: true], ...]) reaches a @ConditionalOnProperty on a hand-written nested @Configuration class. A number reaches an @Value Integer, and a list reaches both @Value List and the indexed probe.list[1] key.
  3. Plugin bean over a harness default. This is pinned by IncludedPluginBeanOverHarnessDefaultSpec.
  4. The guide sentence on conditions now names the doWithSpring and beanRegistrar beans of included plugins.
  5. Nits. The lookup of grailsApplicationPostProcessor is no longer hard-typed, and UNIT_TEST_CONFIGURATION_NAME moved to GrailsBeans.

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:54 says "the core plugin, which is always included". The same page's WARNING under "Spring configuration from plugins" says that overriding getIncludePlugins() drops the defaults, core among them. core still 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-included core plugin").

Approving. The nit can be fixed before merge.

@codeconsole

Copy link
Copy Markdown
Contributor Author

@matrei thanks for the approval and the extra probes on nested maps, numbers and lists. The nit is fixed in a17acb1c20. The guide now says the core plugin "is included by default", matching the WARNING about getIncludePlugins(), and the PR description's Compatibility section says the same. Doc-only change.

@jdaugherty
jdaugherty merged commit c09bfad into apache:8.0.x Sep 26, 2026
87 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants