Step 1 (PR-E): Introduce MongoDatastoreSpec base class for mongo TCK specs#15685
Step 1 (PR-E): Introduce MongoDatastoreSpec base class for mongo TCK specs#15685jamesfredley wants to merge 2 commits into
Conversation
Two coordinated changes that together reduce noise in every mongo TCK
spec by hiding the generic-parameter scaffolding behind a small base
class:
1. Rename the mongo test tree from `grails.gorm.tests` to
`grails.gorm.specs` to match the convention being adopted across
the data-mapping test suite. Includes the mongo-local domain
classes (Plant, Pet, Face, Nose, Person) and three mongo-local
specs (DirtyCheckEmbeddedCollectionSpec, FindNativeSpec,
listener/PersistenceEventListenerSpec). Updates 18 non-renamed
mongo specs to import the new package paths.
2. Add a thin abstract base class:
abstract class MongoDatastoreSpec extends GrailsDataTckSpec<GrailsDataMongoTckManager> {
}
and refactor ~107 mongo specs to extend `MongoDatastoreSpec`
instead of repeating `GrailsDataTckSpec<GrailsDataMongoTckManager>`
at every declaration site.
The two changes are committed together because the rename is a
prerequisite for the base class - `MongoDatastoreSpec` lives at the
new `grails.gorm.specs`-adjacent location and assumes the surrounding
mongo specs reference the renamed packages.
This change was originally pulled forward into the hibernate7 staging
branch as commits 01c27f9 (mongo portion only) and c8cb43f on
PR #15654. @sbglasius questioned whether the refactor belonged in
that PR. Extracting it here so it can land on 8.0.x on its own merits
as a prerequisite for the Hibernate 7 work.
Once merged here, the corresponding portion of "Pull forward package
changes" and the whole "Pull forward base tck test" commit on the
hibernate7 staging branch should be removed since they will arrive
through the next merge of 8.0.x.
Assisted-by: claude-code:claude-4.7-opus
There was a problem hiding this comment.
Pull request overview
This PR refactors the MongoDB TCK/spec test suite to reduce generic boilerplate and to clarify the purpose/ownership of Mongo-local domain/spec classes.
Changes:
- Rename the Mongo-local test package tree from
grails.gorm.teststograils.gorm.specs(domain classes + a few Mongo-local specs) and update imports accordingly. - Introduce
MongoDatastoreSpecas a thin base class (GrailsDataTckSpec<GrailsDataMongoTckManager>) for Mongo specs. - Mechanically update Mongo specs to extend
MongoDatastoreSpecinstead of repeatingGrailsDataTckSpec<GrailsDataMongoTckManager>.
Reviewed changes
Copilot reviewed 114 out of 114 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| grails-data-mongodb/core/src/test/groovy/org/apache/grails/data/mongo/core/MongoDatastoreSpec.groovy | Adds Mongo-specific base Spock spec to remove repeated generic parameters. |
| grails-data-mongodb/core/src/test/groovy/grails/gorm/specs/Plant.groovy | Renames Mongo-local domain package to grails.gorm.specs. |
| grails-data-mongodb/core/src/test/groovy/grails/gorm/specs/Pet.groovy | Renames Mongo-local domain package to grails.gorm.specs. |
| grails-data-mongodb/core/src/test/groovy/grails/gorm/specs/Person.groovy | Renames Mongo-local domain package to grails.gorm.specs. |
| grails-data-mongodb/core/src/test/groovy/grails/gorm/specs/Face.groovy | Renames Mongo-local domain package to grails.gorm.specs. |
| grails-data-mongodb/core/src/test/groovy/grails/gorm/specs/Nose.groovy | Renames Mongo-local domain package to grails.gorm.specs. |
| grails-data-mongodb/core/src/test/groovy/grails/gorm/specs/DirtyCheckEmbeddedCollectionSpec.groovy | Moves to grails.gorm.specs and switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/grails/gorm/specs/FindNativeSpec.groovy | Moves to grails.gorm.specs and switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/grails/gorm/specs/listener/PersistenceEventListenerSpec.groovy | Moves to grails.gorm.specs.listener and switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/grails/mongodb/cascade/MongoCascadeSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/grails/mongodb/bootstrap/FailOnErrorSetupSpec.groovy | Updates import to grails.gorm.specs.Plant. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/AggregateMethodSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/AssignedIdentifierSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/AutowireServicesSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BasicArraySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BasicCollectionsSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BasicCollectionTypeSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BatchUpdateDeleteSpec.groovy | Switches to MongoDatastoreSpec and updates Plant import. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BeforeInsertUpdateSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BeforeUpdatePropertyPersistenceSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BigDecimalSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BrokenManyToManyAssociationSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CascadeDeleteOneToOneSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CascadeDeleteSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CircularBidirectionalOneToManySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CircularEmbeddedListSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CircularOneToManySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ClearCollectionSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CustomCollectionAndAttributeMappingSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CustomIdProxySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CustomMongoEventListenerSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CustomTypeMarshallingSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DBObjectConversionSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DbRefWithEmbeddedSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DefaultSortOrderSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DirtyCheckUpdateSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DisableVersionSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DisjunctionQuerySpec.groovy | Switches to MongoDatastoreSpec and updates domain imports. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DistinctPropertySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DocumentMappingSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedAssociationSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedBiDirectionalSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedCollectionAndInheritanceSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedCollectionWithIdSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedCollectionWithOneToOneSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedHasManyWithBeforeUpdateSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedListWithCustomTypeSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedMapSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedSetAssignedIdSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedSimpleObjectSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedStringListInsideEmbeddedCollectionSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedUnsetSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedWhereClauseSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedWithCustomFieldMappingSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedWithIdSpecifiedSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedWithNonEmbeddedAssociationsSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedWithNonEmbeddedCollectionsSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedWithinEmbeddedAssociationSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EnumCollectionSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EnumTypeSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EventsWithAbstractInheritanceSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/FindOrCreateWhereSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/GeoJSONTypePersistenceSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/GeospacialQuerySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/GetAllSpec.groovy | Switches to MongoDatastoreSpec and updates domain imports. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/GetAllWithStringIdSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/GreaterThanAndLessThanCriteriaSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/HasOneSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/HintQueryArgumentSpec.groovy | Switches to MongoDatastoreSpec and updates Person import. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/InListQuerySpec.groovy | Switches to MongoDatastoreSpec and updates domain imports. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/IndexAttributesAndCompoundKeySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/IndexWithInheritanceSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/InheritanceQueryingSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/InheritanceWithSingleEndedAssociationSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/InnerEnumSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/IsNullSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/JakartaValidationSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/LastUpdatedSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/LikeQuerySpec.groovy | Switches to MongoDatastoreSpec and updates Pet import. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ListOneToManyOrderingSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MapOfDomainsSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MarkDirtyFalseSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MongoDynamicPropertyOnEmbeddedSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MongoEntityConfigSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MongoGormEnhancerSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MongoResultsListIndexSpec.groovy | Switches to MongoDatastoreSpec and updates Person import. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MongoTypesSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/NegateInListSpec.groovy | Switches to MongoDatastoreSpec and updates Person import. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/NegationEnumSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/NullifyPropertySpec.groovy | Switches to MongoDatastoreSpec and updates domain imports. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/NullsAreNotStoredSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ObjectIdPersistenceSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ObjectIdPropertySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/OneToManyWithInheritanceSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/OneToOneIntegritySpec.groovy | Switches to MongoDatastoreSpec and updates domain imports. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/OneToOneNoReferenceSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/OptimisticLockingWithExceptionSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/OrderWithPaginationSpec.groovy | Switches to MongoDatastoreSpec and updates Plant import. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ProjectionsSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/QueriesWithIdenticallyNamedPartsSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ReadConcernArgumentSpec.groovy | Switches to MongoDatastoreSpec and updates FQCN usages to grails.gorm.specs.Person. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ReadManyObjectsSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ResultsWithGroovyCollectionMethodsSpec.groovy | Switches to MongoDatastoreSpec and updates Plant import. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/SchemalessSpec.groovy | Switches to MongoDatastoreSpec and updates Plant import. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/SessionCachingSpec.groovy | Switches to MongoDatastoreSpec and updates Person import. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/SetRetrievalSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/SimpleHasManySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/StatelessSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/SwitchDatabaseAtRuntimeSpec.groovy | Switches to MongoDatastoreSpec and updates Person import. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/TestSearchSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/TransientPropertySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/WhereQueryInCriteriaSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/WriteConcernSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/bugs/GPMongoDB295Spec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/connections/MongoConnectionSourcesSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/connections/MultipleDataSourceConnectionsSpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/connections/MultiTenancySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/connections/SchemaBasedMultiTenancySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/connections/SingleTenancySpec.groovy | Switches to MongoDatastoreSpec. |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/multitenancy/MongoStaticApiMultiTenancySpec.groovy | Switches to MongoDatastoreSpec. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This commit reverts the portions of this branch that have been split out into standalone PRs against 8.0.x. The goal is to shrink the PR-A review surface to focus on the actual hibernate5 -> hibernate7 clone and let the unrelated cleanups be reviewed on their own merits. Once PRs B/C/D/E land on 8.0.x and 8.0.x is merged into this branch, the reverted changes will arrive through the merge so the final state of stage-hibernate7 is unchanged - only the diff visible on this PR is reduced. Reverted content: Async defensive cleanup (PR #15682, PR-B) 9 files in grails-async/. Reverts e3cad41. Null-safety guards, @OverRide annotations, catch(Exception ignored) over catch(Throwable), constant-on-left equality, varargs. addAllDomainClasses helper + adoption (PR #15683, PR-C) Helper method on GrailsDataTckManager plus all callers across the data mapping test suites. Reverts ed0916d plus all orphan callers added by subsequent commits (50 specs) so the branch compiles without the helper method. MongoDatastoreSpec base class + mongo package rename (PR #15685, PR-E) Reverts c8cb43f (MongoDatastoreSpec introduction + ~107 spec refactors) and the mongo portion of 01c27f9 (8 file renames grails.gorm.tests -> grails.gorm.specs plus 18 import-update modifications in non-renamed mongo specs). DetachedCriteriaSpec command-chain syntax (PR #15684, PR-D) 18 `eq(...)` and `like(...)` parens restorations in the TCK DetachedCriteriaSpec. Reverts the parens hunk of ee4ab14 while leaving the rest of that commit (setupSpec addition, import reorder, and the other 35 TCK file changes) intact. NOT reverted (intentionally kept in PR-A): - The hibernate5 -> hibernate7 baseline clone (~100k lines, the actual work being reviewed) - 89 hibernate5 and 90 hibernate7 grails.gorm.tests -> grails.gorm.specs package renames (non-mongo portion of 01c27f9, since these touch the cloned tests and reverting would require re-doing both sides) - 18 grails-datamapping-core-test package renames (non-mongo portion of 01c27f9) - All other "Pull forward..." commits (styling, build config, test additions, etc.) which the reviewers have not specifically objected to Assisted-by: claude-code:claude-4.7-opus
16 mongo specs in this PR called manager.addAllDomainClasses(...), a helper method that does not exist on 8.0.x. The helper is introduced by a sibling PR (#15683, the GrailsDataTckManager helper extraction) that this PR does not depend on. The setupSpec calls were inherited from the staging branch where both changes already coexisted. On 8.0.x in isolation, the helper method is undefined and the specs would throw MissingMethodException at runtime. Converting all 16 callers to the existing manager.domainClasses.addAll pattern so this PR is self-contained against 8.0.x. After both this PR and #15683 land, the callers can be optionally converted to use the helper in a follow-up (or naturally via the next merge of 8.0.x into the staging branch), but that is not a prerequisite for this PR. Files updated: - DisjunctionQuerySpec.groovy - EmbeddedHasManyWithBeforeUpdateSpec.groovy - EmbeddedWithIdSpecifiedSpec.groovy - GetAllSpec.groovy - HintQueryArgumentSpec.groovy - InListQuerySpec.groovy - LikeQuerySpec.groovy - MongoResultsListIndexSpec.groovy - NegateInListSpec.groovy - NullifyPropertySpec.groovy - OneToOneIntegritySpec.groovy - OrderWithPaginationSpec.groovy - ReadConcernArgumentSpec.groovy - ResultsWithGroovyCollectionMethodsSpec.groovy - SchemalessSpec.groovy - SessionCachingSpec.groovy Verified locally with ./gradlew :grails-data-mongodb-core:compileTestGroovy. Assisted-by: claude-code:claude-4.7-opus
matrei
left a comment
There was a problem hiding this comment.
I am against this change as it in my view does not add anything of value but rather hides that tests are of type GrailsDataTckSpec<GrailsDataMongoTckManager>.
sbglasius
left a comment
There was a problem hiding this comment.
I don't see the need to add MongoDatastoreSpec but on the other hand, it's not keeping me awake at night.
|
Without having to copy a large block of code GrailsHibernatePersistentEntity createPersistentEntity(Class clazz) {
return createPersistentEntity(clazz, getGrailsDomainBinder())
} |
|
Originally the base class was there to spin up docker containers, but it was cleaned up to the point where the class no longer serves a purpose. I'm fine having a common base class to find usages of mongo, effectively making this a marker interface / trait. But to @matrei point it's not technically needed anymore. |
Step 1 (PR-E) prerequisite for Hibernate 7 work - extracted from the staging branch so it can be reviewed on its own merits.
Context
This was originally split across two commits on the hibernate7 staging branch (
01c27f9a77mongo portion +c8cb43f2ec) on PR #15654. @sbglasius questioned whether the MongoDatastoreSpec refactor was needed at all ("if needed, ok"), so it had to live somewhere with a clear scope. Extracting it here against8.0.xso the question can be settled on its own merits without holding up the Hibernate 7 clone.Scope
Two coordinated changes committed together because the second depends on the first:
1. Rename mongo test tree
grails.gorm.tests→grails.gorm.specs8 file renames + 18 import-update modifications:
Plant.groovy,Pet.groovy,Face.groovy,Nose.groovy,Person.groovyDirtyCheckEmbeddedCollectionSpec.groovy,FindNativeSpec.groovy,listener/PersistenceEventListenerSpec.groovyMongo is self-contained here - the renames cover both its specs AND its own copies of the domain classes those specs reference. No coupling to
grails-datamapping-core-testor other modules.2. Add
MongoDatastoreSpecbase classPlus refactoring ~107 mongo specs to extend
MongoDatastoreSpecinstead of repeatingGrailsDataTckSpec<GrailsDataMongoTckManager>at every declaration site.Files changed
114 files: 8 renames + 1 new file (MongoDatastoreSpec) + 105 modified mongo specs.
Verified
./gradlew :grails-data-mongodb-core:compileTestGroovypasses cleanly. The rename + base class refactor compiles consistently on8.0.x.Why a separate PR
@sbglasius's review of the staging PR specifically called this out:
Extracting it here gives the question a clear, focused venue. If the refactor is wanted, it can land here and flow into
stage-hibernate7via the next merge of8.0.x. If it's rejected, this PR can be closed and the corresponding commits removed from the staging branch via rebase.Authorship preserved from the original commits by @jdaugherty.
Related