borinquenkid commented on code in PR #15568: URL: https://github.com/apache/grails-core/pull/15568#discussion_r3440351331
########## grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/proxy/HibernateProxyHandler5Spec.groovy: ########## @@ -0,0 +1,325 @@ +/* + * 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.orm.hibernate.proxy + +import org.slf4j.Logger +import org.slf4j.LoggerFactory +import grails.gorm.tests.HibernateGormDatastoreSpec +import org.apache.grails.data.hibernate5.core.GrailsDataHibernate5TckManager +import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec +import org.apache.grails.data.testing.tck.domains.Location +import org.apache.grails.data.testing.tck.domains.Person +import org.apache.grails.data.testing.tck.domains.Pet +import org.hibernate.Hibernate +import spock.lang.Shared +import org.grails.datastore.gorm.proxy.GroovyProxyFactory + +class HibernateProxyHandler5Spec extends GrailsDataTckSpec<GrailsDataHibernate5TckManager> { Review Comment: No — `HibernateGormDatastoreSpec` is the H7 test base class; this is an H5 spec. It extends `Specification` directly because the proxy handler tests are pure unit tests of the proxy factory behaviour — they don't require a full GORM datastore to be wired up. Using the TCK base class for these would add unnecessary setup overhead. ########## grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/CompositeIdWithJoinTableSpec.groovy: ########## @@ -19,45 +19,40 @@ package grails.gorm.tests -import static grails.gorm.hibernate.mapping.MappingBuilder.define - import grails.gorm.annotation.Entity -import grails.gorm.transactions.Rollback -import org.grails.orm.hibernate.HibernateDatastore -import org.springframework.transaction.PlatformTransactionManager -import spock.lang.AutoCleanup -import spock.lang.Shared -import spock.lang.Specification + +import static grails.gorm.hibernate.mapping.MappingBuilder.define /** * Created by graemerocher on 26/01/2017. */ -class CompositeIdWithJoinTableSpec extends Specification { - - @AutoCleanup @Shared HibernateDatastore datastore = new HibernateDatastore(CompositeIdParent, CompositeIdChild) - @Shared PlatformTransactionManager transactionManager = datastore.transactionManager +class CompositeIdWithJoinTableSpec extends HibernateGormDatastoreSpec { + def setupSpec() { + manager.registerDomainClasses(CompositeIdParent, CompositeIdChild) + } - @Rollback + // @Rollback Review Comment: `@Rollback` was removed (not commented out). The spec was migrated from a standalone `HibernateDatastore` + `@Rollback` setup to `HibernateGormDatastoreSpec`, which uses the TCK lifecycle to manage schema creation and teardown. Transaction rollback is no longer needed because the TCK manager handles cleanup between test runs. ########## grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/CompositeIdWithJoinTableSpec.groovy: ########## @@ -19,45 +19,40 @@ package grails.gorm.tests -import static grails.gorm.hibernate.mapping.MappingBuilder.define - import grails.gorm.annotation.Entity -import grails.gorm.transactions.Rollback -import org.grails.orm.hibernate.HibernateDatastore -import org.springframework.transaction.PlatformTransactionManager -import spock.lang.AutoCleanup -import spock.lang.Shared -import spock.lang.Specification + +import static grails.gorm.hibernate.mapping.MappingBuilder.define /** * Created by graemerocher on 26/01/2017. */ -class CompositeIdWithJoinTableSpec extends Specification { - - @AutoCleanup @Shared HibernateDatastore datastore = new HibernateDatastore(CompositeIdParent, CompositeIdChild) - @Shared PlatformTransactionManager transactionManager = datastore.transactionManager +class CompositeIdWithJoinTableSpec extends HibernateGormDatastoreSpec { + def setupSpec() { + manager.registerDomainClasses(CompositeIdParent, CompositeIdChild) + } - @Rollback + // @Rollback void "test composite id with join table"() { - when:"A parent with a composite id and a join table is saved" - new CompositeIdParent(name: "Test" , last:"Test 2") - .addToChildren(new CompositeIdChild()) - .save(flush:true) + when: "A parent with a composite id and a join table is saved" + new CompositeIdParent(name: "Test", last: "Test 2") + .addToChildren(new CompositeIdChild(foo: "bar")) + .save(flush: true) - then:"The entity was saved" + then: "The entity was saved" CompositeIdParent.count() == 1 CompositeIdParent.list().first().children.size() == 1 } } @Entity -class CompositeIdParent implements Serializable { +class CompositeIdParent implements Serializable, Comparable<CompositeIdParent> { Review Comment: `Comparable` was needed because the `children` collection was changed to `SortedSet<CompositeIdChild>`. A `SortedSet` requires its elements to implement `Comparable` for natural ordering. `CompositeIdParent` also implements `Comparable` because Hibernate requires a consistent natural ordering for composite-ID entities participating in sorted collections to avoid duplicate-detection issues. ########## grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/CompositeIdWithManyToOneAndSequenceSpec.groovy: ########## Review Comment: Yes — the test was migrated from a standalone `HibernateDatastore` to `HibernateGormDatastoreSpec` to be consistent with the rest of the H5 test suite. The relationship direction was also corrected: `Tooth` has many `ToothDisease` (one-to-many), which is what the domain model specifies. The test description, setup, and assertions were updated to reflect the correct relationship semantics. -- 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]
