Copilot commented on code in PR #15441:
URL: https://github.com/apache/grails-core/pull/15441#discussion_r2844010695
##########
grails-data-neo4j/grails-datastore-gorm-neo4j/src/test/groovy/grails/gorm/tests/ValidationSpec.groovy:
##########
@@ -38,7 +37,7 @@ class ValidationSpec extends GormDatastoreSpec {
def setup() {
for(cls in domainClasses) {
setupValidator(cls)
- GormEnhancer.findValidationApi(cls).validator = null
+
session.mappingContext.addEntityValidator(persistentEntityFor(cls), null)
Review Comment:
`MappingContext.addEntityValidator(entity, null)` is a no-op
(AbstractMappingContext only stores validators when the argument is non-null),
so this line does not clear/reset any validator state. If the intent is to
avoid leaking a previously-registered mock validator between tests, you’ll need
a different reset mechanism (for example: re-register a real validator instance
for the entity, or explicitly clear the cached validator held by the validation
API).
##########
grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/DeepValidateWithSaveSpec.groovy:
##########
@@ -18,29 +18,39 @@
*/
package grails.gorm.tests
+import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec
+import org.apache.grails.data.testing.tck.domains.ChildEntity
import org.apache.grails.data.testing.tck.domains.TestEntity
import org.apache.grails.data.simple.core.GrailsDataCoreTckManager
-import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec
-import org.grails.datastore.gorm.GormEnhancer
-import org.grails.datastore.gorm.GormInstanceApi
-import org.grails.datastore.gorm.GormValidateable
/**
* Created by graemerocher on 16/02/2017.
*/
class DeepValidateWithSaveSpec extends
GrailsDataTckSpec<GrailsDataCoreTckManager> {
- void "test deep validate parameter"() {
- given:
- def validateable = Mock(GormValidateable)
- validateable.hasErrors() >> true
- def args = [deepValidate: true]
+ void "save with deepValidate: true succeeds for a valid entity"() {
+ given: "a valid TestEntity"
+ def entity = new TestEntity(name: 'testDeepValidate', age: 10, child:
new ChildEntity(name: 'child'))
+
+ when: "saved with deepValidate: true"
+ def saved = entity.save(deepValidate: true, flush: true)
+
+ then: "the entity is persisted without errors"
+ saved != null
+ saved.id != null
+ !saved.hasErrors()
+ }
+
+ void "save with deepValidate: false still saves a valid entity"() {
+ given: "a valid TestEntity"
+ def entity = new TestEntity(name: 'testShallowValidate', age: 10,
child: new ChildEntity(name: 'child'))
- when:
- GormInstanceApi instanceApi = GormEnhancer.findInstanceApi(TestEntity)
- instanceApi.save(validateable, [deepValidate: true])
+ when: "saved with deepValidate: false"
+ def saved = entity.save(deepValidate: false, flush: true)
- then:
- 1 * validateable.validate(args)
+ then: "the entity is persisted without errors"
+ saved != null
+ saved.id != null
+ !saved.hasErrors()
Review Comment:
`save(deepValidate: true/false)` is only being exercised on a valid entity,
so this spec no longer verifies that the `deepValidate` flag is actually
honored (the test will pass even if the flag is ignored). Consider asserting
behavior by installing a `CascadingValidator`/`Validator` mock for `TestEntity`
and verifying the deepValidate argument passed during `save()`, or by crafting
a case where deep vs shallow validation yields different outcomes.
--
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]
