(3/3) test: add core-class unit tests for the GormRegistry O(M+N) rewrite#15790
Open
borinquenkid wants to merge 4 commits into
Open
(3/3) test: add core-class unit tests for the GormRegistry O(M+N) rewrite#15790borinquenkid wants to merge 4 commits into
borinquenkid wants to merge 4 commits into
Conversation
This was referenced Jun 29, 2026
Harvest the net-new GormRegistry unit specs onto core-impl so the rewrite's core classes have
dedicated coverage they were missing: GormApiResolver and the four datastore selectors
(Preferred/Qualified/ActiveSession/Default), GormRegistry (concurrency, entity registration, factory),
the api registries (Abstract/Static/Instance/Validation) and the GormApiFactory SPI
(Default/GormApiFactory), CurrentTenantHolder, Tenants, the @CurrentTenant transform
(MultiTenantCurrentTenantTransformSpec), MultiTenantEventListener, ConnectionSourceNameResolver,
DefaultTransactionTemplateFactory, and a service transform spec.
All 23 specs pass against core-impl's implementation. Two adjustments from the source specs:
- ServiceTransformSpec: align the no-datastore exception assertion to core-impl's actual message
("No GORM implementations configured" from GormApiResolver.findSingleDatastore).
- Dropped TenantContextProfilingSpec: a timing micro-benchmark (prints durations, asserts only true) —
brittle and non-deterministic, not portable unit coverage.
Tests-only change; no production code touched. Branched from the GormRegistry core-impl work.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
borinquenkid
force-pushed
the
test/gorm-registry-core-tests
branch
from
7bab6ad to
1ccec90
Compare
2 tasks
2 tasks
borinquenkid
changed the title
…to test/gorm-registry-core-tests
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a comprehensive new Spock unit test suite in grails-datamapping-core to cover the new GormRegistry-based O(M+N) GORM API registration/lookup architecture introduced in the prior stacked PRs. It is test-only, focused on validating registry lookups, qualifier/tenant datastore selection, API factory behavior, and related multi-tenancy/service-transform integration points.
Changes:
- Add new unit specs for
GormRegistry,GormApiResolver, API registries, andGormApiFactory/DefaultGormApiFactory. - Add new unit specs for datastore-selection helpers (
Preferred*,Qualified*,ActiveSession*,Default*) and connection-source name resolution. - Add/extend unit coverage for multi-tenancy helpers/event listener behavior and service transformation behavior.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transactions/DefaultTransactionTemplateFactorySpec.groovy | Adds unit coverage for DefaultTransactionTemplateFactory overloads returning GrailsTransactionTemplate. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/QualifiedDatastoreSelectorSpec.groovy | Tests qualifier-based datastore selection (TX-bound, registry, multi-connection, multi-tenant). |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/PreferredDatastoreSelectorSpec.groovy | Tests preferred datastore routing behavior for default and qualified lookups. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/multitenancy/MultiTenantEventListenerSpec.groovy | Adds unit coverage for discriminator multi-tenancy event listener tenant-id assignment behavior. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormValidationApiRegistrySpec.groovy | Verifies validation API registry resolution by qualifier. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormStaticApiRegistrySpec.groovy | Verifies static API registry resolution by qualifier. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormRegistryFactorySpec.groovy | Tests API factory override lookup and resolver exposure on the registry. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormRegistryEntityRegistrationSpec.groovy | Tests API registration and entity-to-datastore registration/normalization behaviors. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormRegistryConcurrencySpec.groovy | Adds concurrency-focused hot-path coverage for normalization and registry lookups. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormInstanceApiSpec.groovy | Tests GormInstanceApi save(validate:false) behavior around skipValidation state. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormInstanceApiRegistrySpec.groovy | Verifies instance API registry resolution by qualifier. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormApiResolverSpec.groovy | Exercises resolver paths (type lookup, qualifier lookup, preferred DS, recursion guard, multi-tenant edge cases). |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormApiRegistrySpec.groovy | Verifies common registry storage/lookup semantics for static/instance/validation registries. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormApiFactorySpec.groovy | Defines/validates GormApiFactory contract expectations (create APIs, dynamic finders). |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/DefaultGormApiFactorySpec.groovy | Tests default factory configuration application and default dynamic finder set. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/DefaultDatastoreSelectorSpec.groovy | Tests tenant-aware default datastore selection behavior. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/ConnectionSourceNameResolverSpec.groovy | Tests resolving connection source names/defaults from a datastore/provider. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/ActiveSessionDatastoreSelectorSpec.groovy | Tests active-session datastore selection behavior. |
| grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/AbstractGormApiRegistrySpec.groovy | Tests AbstractGormApiRegistry core behaviors (register/get/qualify/clear/exceptions). |
| grails-datamapping-core/src/test/groovy/grails/gorm/services/transform/ServiceTransformSpec.groovy | Adds/updates transformation tests to align with registry-based datastore resolution. |
| grails-datamapping-core/src/test/groovy/grails/gorm/services/transform/ServiceTransformClasses.groovy | Supplies entities/services used by the service-transform test suite. |
| grails-datamapping-core/src/test/groovy/grails/gorm/multitenancy/TenantsSpec.groovy | Adds unit coverage for Tenants helper APIs (withId/currentId/withoutId nesting). |
| grails-datamapping-core/src/test/groovy/grails/gorm/multitenancy/CurrentTenantHolderSpec.groovy | Adds unit coverage for CurrentTenantHolder set/get/remove/withTenant behaviors. |
| grails-datamapping-core/src/test/groovy/grails/gorm/annotation/multitenancy/MultiTenantCurrentTenantTransformSpec.groovy | Adds AST transform coverage for @CurrentTenant / @WithoutTenant behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+63
to
+66
| then: | ||
| template != null | ||
| template instanceof GrailsTransactionTemplate | ||
| } |
Comment on lines
+30
to
+33
| static class DummyTenantId extends TenantId { | ||
| DummyTenantId() { super(null, null, "tenantId", Long) } | ||
| org.grails.datastore.mapping.model.PropertyMapping getMapping() { null } | ||
| } |
Comment on lines
+120
to
+125
| startLatch.countDown() // Release all threads | ||
| endLatch.await(5, TimeUnit.SECONDS) | ||
| long endTime = System.currentTimeMillis() | ||
| executor.shutdown() | ||
|
|
||
| println "Concurrency test completed in ${endTime - startTime}ms for ${numThreads * iterationsPerThread} operations" |
Comment on lines
+111
to
+114
| } catch (Throwable e) { | ||
| e.printStackTrace() | ||
| errorCount.incrementAndGet() | ||
| } finally { |
Comment on lines
+47
to
+49
| int numThreads = 10 | ||
| int iterationsPerThread = 100000 | ||
| ExecutorService executor = Executors.newFixedThreadPool(numThreads) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/gorm-registry-core-impl #15790 +/- ##
======================================================================
+ Coverage 49.5761% 49.6674% +0.0913%
- Complexity 17080 17136 +56
======================================================================
Files 1961 1961
Lines 93654 93653 -1
Branches 16454 16454
======================================================================
+ Hits 46430 46515 +85
+ Misses 39972 39893 -79
+ Partials 7252 7245 -7
🚀 New features to boost your workflow:
|
…to test/gorm-registry-core-tests
- DefaultTransactionTemplateFactorySpec: remove unused import; strengthen the TransactionDefinition overload test to assert the definition's propagation/isolation/timeout/readOnly were actually applied to the created template, not just its type. - MultiTenantEventListenerSpec: DummyTenantId declared a Long type while the existing tests exercise String tenant ids, masking the Number-coercion branch in MultiTenantEventListener (ConnectionSource. DEFAULT + numeric tenantId type -> 0L). Fix the fixture's declared type to match, and add coverage for both branches of that condition. - GormRegistryConcurrencySpec: assert on the CountDownLatch#await return value instead of ignoring it (so a stuck worker thread fails the test loudly instead of silently passing), capture worker exceptions instead of printStackTrace-ing them, and reduce iterationsPerThread from 100000 to 20000 to cut flakiness risk under @timeout(10) on slower CI runners (verified stable at ~1s locally, down from ~3.25s). Also correct my own earlier fix on this stack: AbstractStringQueryImplementer's constant-@query substring checks looked like dead/hacky code from the GString-based tests in grails.gorm.services.ServiceTransformSpec, but a second spec (grails.gorm.services.transform.ServiceTransformSpec) uses single-quoted (non-GString) @query values that genuinely depend on them. Restore the checks, narrowed to the actual token shapes used ($wrong / f.wrong, and "from java.lang.String") instead of a bare substring match, so legitimate queries that merely mention those words elsewhere no longer false-positive. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
✅ All tests passed ✅🏷️ Commit: 62f351b Learn more about TestLens at testlens.app. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds the net-new unit-test coverage for the GormRegistry O(M+N) rewrite's core classes that
the core-impl branch (#15780) was missing. Tests-only — no production code is touched.
Stacked on #15780 (
feat/gorm-registry-core-impl), so the diff here is just the new specs.Coverage added (23 specs)
GormApiResolverand the four datastore selectors —PreferredDatastoreSelector,QualifiedDatastoreSelector,ActiveSessionDatastoreSelector,DefaultDatastoreSelectorGormRegistry— concurrency, entity registration, factory lookupAbstractGormApiRegistry,GormStaticApiRegistry,GormInstanceApiRegistry,GormValidationApiRegistry,GormApiRegistry, plusGormInstanceApiGormApiFactorySPI —DefaultGormApiFactory,GormApiFactoryCurrentTenantHolder,Tenants, the@CurrentTenanttransform(
MultiTenantCurrentTenantTransformSpec),MultiTenantEventListenerConnectionSourceNameResolver,DefaultTransactionTemplateFactory, a service-transform specNotes
All 23 specs pass against core-impl's implementation (
:grails-datamapping-core:test, 0 failures; codeStyle clean).Two adjustments from the originally-drafted specs:
ServiceTransformSpec: aligned the no-datastore exception assertion to core-impl's actual message(
"No GORM implementations configured", thrown byGormApiResolver.findSingleDatastore).TenantContextProfilingSpec: a timing micro-benchmark (prints durations, asserts onlytrue) —brittle/non-deterministic and not portable unit coverage.
🤖 Generated with Claude Code