jdaugherty commented on code in PR #16399:
URL: https://github.com/apache/grails-core/pull/16399#discussion_r4106626393
##########
grails-testing-support-core/src/main/groovy/org/grails/testing/GrailsApplicationBuilder.groovy:
##########
@@ -170,9 +178,18 @@ class GrailsApplicationBuilder {
beanFactory.allowCircularReferences =
environment.getProperty(Settings.SPRING_MAIN_ALLOW_CIRCULAR_REFERENCES,
Boolean, Boolean.TRUE)
def classLoader = this.class.classLoader
+ // The test's own configuration first, as an application's comes
before auto-configuration:
+ // parsed ahead of them, its beans are what an auto-configuration's
@ConditionalOnMissingBean sees.
+ configurationClasses?.each { Class<?> configurationClass ->
Review Comment:
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.
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -1695,6 +1695,12 @@ class MyGrailsPlugin extends Plugin {
Both hooks may be used on the same plugin during migration; when a registrar
and the DSL register a bean under the same name, the registrar's definition
wins.
+**Unit tests follow.** `doWithSpring()` on the `GrailsUnitTest` trait — and so
on every testing trait — is deprecated too. A test now declares its beans in a
`beans` block with the `beans` DSL, as an application or plugin does; overrides
`beanRegistrar()`, applied where an application's is; or declares a static
nested `@Configuration` class, which its context registers ahead of the
framework's auto-configurations. A `beans` block compiles into one such class,
named `BeansConfiguration`, so a test that already declares a nested class of
that name must rename it. A test that includes a plugin through
`getIncludePlugins()` also gets the beans the plugin's `beans` block declares,
which `defineBeans(plugin)` cannot apply. See <<unitTesting,Unit Testing>>.
+
+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.
Review 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:
```suggestion
**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.
```
##########
grails-testing-support-core/src/test/resources/META-INF/grails-plugin.xml:
##########
@@ -0,0 +1,3 @@
+<plugin name='includedBeans' version='1.0'>
Review Comment:
Confirmed: the resource is gone, and all 31 of the module's tests pass on
the descriptor the transform generates.
##########
grails-core/src/main/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformation.groovy:
##########
@@ -387,6 +391,8 @@ class GlobalGrailsClassInjectorTransformation implements
ASTTransformation, Comp
if (beansProperty == null ||
!classNode.getAnnotations(GRAILS_BEANS_ANNOTATION).isEmpty()) {
Review Comment:
Verified at 58b99742f1: the convention and `@GrailsBeans` paths both report
a `@Shared` block with the new message, and an unrelated `@Shared` `beans` map
compiles as before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
