jamesfredley commented on code in PR #15557:
URL: https://github.com/apache/grails-core/pull/15557#discussion_r3343203267
##########
grails-datamapping-core/src/test/groovy/grails/gorm/annotation/transactions/TransactionalTransformSpec.groovy:
##########
@@ -197,10 +197,6 @@ import grails.gorm.transactions.Transactional
mySpec.getDeclaredMethod('$spock_feature_0_0', Object, Object, Object)
mySpec.getDeclaredMethod('$tt__$spock_feature_0_0', Object, Object,
Object, TransactionStatus)
- and:"The spec can be called"
- mySpec.newInstance().'$tt__$spock_feature_0_0'(2,2,4,new
DefaultTransactionStatus(null, new Object(), true, true, false, false, false,
null))
Review Comment:
Correction to my previous reply - I verified this with a build and the
restored assertions actually fail under Spock 2.4:
```
java.lang.IllegalStateException: Cannot request current iteration in @Shared
context, or feature context
at ...TransactionalTransformSpec.groovy:201
mySpec.newInstance().'$tt__$spock_feature_0_0'(...)
```
The `$tt__$spock_feature_0_0` method being invoked is the transactional
wrapper of a **Spock feature method**, and its body reaches into Spock's
per-iteration context. That context only exists while the Spock runner is
executing the feature - not when the method is invoked directly on
`clazz.newInstance()`. Under Spock 2.3 that access was lenient; Spock 2.4
(#15703) makes it throw. So the direct-invocation assertion genuinely can't be
restored on this stack. The `getDeclaredMethod(...)` checks still verify the
transform generates `$spock_feature_0_0` and its `$tt__` transactional wrapper
with the correct signatures. I've reverted my restore and re-opened this thread.
##########
grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/reflect/ClassPropertyFetcherTests.groovy:
##########
@@ -114,8 +114,8 @@ class ClassPropertyFetcherTests {
}
}
-trait TestTrait<F extends Serializable> {
- F from
+trait TestTrait<T> {
Review Comment:
Correction to my previous reply - restoring the `Serializable` bound breaks
compilation under Groovy 5:
```
ClassPropertyFetcherTests.groovy: Can't have an abstract method in a
non-abstract class.
The class 'DomainWithTrait' must be declared abstract or the method
'F org_grails_datastore_mapping_reflect_TestTrait__from$set(F)' must be
implemented.
class DomainWithTrait implements Serializable, TestTrait<DomainWithTrait>
```
The bounded type parameter `<F extends Serializable>` changes how Groovy 5
generates the trait's property setter so it lands as an abstract method on the
implementing class. Importantly, `DomainWithTrait` itself still `implements
Serializable`, so the datastore behaviour under test (a `Serializable` domain
with a trait-provided property) is unchanged - only the trait's type-parameter
*bound* was relaxed to `<T>`. So this is a required Groovy 5 trait-compilation
workaround rather than an incidental removal. I've reverted my change back to
`<T>` and re-opened this thread.
--
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]
