jamesfredley commented on PR #15530:
URL: https://github.com/apache/grails-core/pull/15530#issuecomment-4178160991
The `8.0.x-hibernate7` branch's broad refactoring introduced several test
coverage regressions for non-hibernate7 modules. Tests were accidentally
disabled, gated behind annotations that prevent them from running, or broken by
API changes. The goal was to ensure every test that ran on `spring-boot-4` for
Hibernate 5, MongoDB, and the core in-memory datastore still runs after the
`8.0.x-hibernate7` changes.
## Scope
All test modules **except** `grails-data-hibernate7`. Specifically:
- `grails-datamapping-tck` - Shared TCK test specs (consumed by all
datastore modules)
- `grails-datamapping-core-test` - Core in-memory datastore tests
- `grails-data-hibernate5` - Hibernate 5 module tests
- `grails-data-mongodb` - MongoDB module tests
- `grails-async` - Async services plugin tests
- `grails-gsp`, `grails-test-suite-uber`, `grails-test-suite-web`,
`grails-datastore-core`, `grails-test-examples`, `build-logic`
## Analysis Methodology
1. Full `git diff spring-boot-4...8.0.x-hibernate7` across all 731 changed
test files
2. Categorized every `@Ignore`, `@IgnoreIf`, `@PendingFeature`,
`@PendingFeatureIf`, and `@Requires` annotation added in the merge
3. Cross-referenced each annotation against `spring-boot-4` baseline to
distinguish new additions from pre-existing or renamed (`hibernate6` to
`hibernate7`) ones
4. Checked for deleted test files (none found)
5. Verified each suppression's scope to confirm whether it blocks
non-hibernate7 execution
## Issues Found and Fixed
### Category A: TCK Specs with Missing or Incorrect Gating
**Commit:** `20131c0903` - fix: restore non-hibernate7 test coverage
#### 1. FindByMethodSpec (TCK)
**File:**
`grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/FindByMethodSpec.groovy`
**Problem:** Most test methods present on `spring-boot-4` were removed in
the merge, and the remaining ones were restructured with different gating.
**State on `spring-boot-4`:**
- No class-level annotations (no `@Requires`, no `@IgnoreIf`)
- Test features: `Test Using AND Multiple Times In A Dynamic Finder`, `Test
Using OR Multiple Times In A Dynamic Finder`, `testBooleanPropertyQuery` (with
`findAllBypassedByName`, `findNotBypassed`), `Test findOrCreateBy For A Record
That Does Not Exist In The Database`, `Test findOrCreateBy With An AND Clause`,
`Test findOrCreateBy Throws Exception If An OR Clause Is Used`, `Test
findOrSaveBy For A Record That Does Not Exist In The Database`, `Test
findOrSaveBy For A Record That Does Exist In The Database`, `Test patterns
which shold throw MissingMethodException`
- One method-level annotation: `@IgnoreIf({
System.getProperty('hibernate5.gorm.suite') })` on `Test optimistic locking`
(only present in OptimisticLockingSpec, not here - this spec had zero
suppression annotations)
**State on `8.0.x-hibernate7` (before fix):**
- No class-level annotations
- Only 4 features remained: `Test Using AND Multiple Times In A Dynamic
Finder` (rewritten to simpler assertion), `Test findOrCreateBy For A Record
That Does Not Exist` (renamed/simplified), `Test Hib5 pattern ... should throw
MissingMethodException` (gated with `@Requires(hibernate5)`), `Test Hib7
pattern ... should throw ...` (gated with `@Requires(hibernate7)`)
- All OR tests, boolean property tests, findOrSaveBy tests, and the original
findOrCreateBy variants were removed
**State on this branch (after fix):**
- All spring-boot-4 test methods restored plus the new hibernate7-specific
error pattern test retained from 8.0.x-hibernate7
- 3 complex dynamic finder features gated with `@Requires(hibernate5 ||
hibernate7 || mongodb)` because the simple in-memory datastore cannot handle
multi-property AND/OR dynamic finders: `Test Using AND Multiple Times In A
Dynamic Finder`, `Test Using OR Multiple Times In A Dynamic Finder`,
`testBooleanPropertyQuery`
- `Test patterns which shold throw MissingMethodException` gated with
`@IgnoreIf(hibernate7)` since hibernate7 has its own pattern test
- New `Test Hib7 pattern ... should throw ...` retained with
`@Requires(hibernate7)` from 8.0.x-hibernate7
- **Difference from spring-boot-4:** 3 features now have
`@Requires(hibernate5 || hibernate7 || mongodb)` where spring-boot-4 had no
gating. This is necessary because the simple in-memory datastore returns empty
results for complex dynamic finders - these tests would have failed silently or
been untested on core before too, they just weren't gated.
#### 2. OptimisticLockingSpec (TCK)
**File:**
`grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/OptimisticLockingSpec.groovy`
**State on `spring-boot-4`:**
- No class-level `@Requires` (ran for all datastores)
- 3 features: `Test versioning`, `Test optimistic locking` (with
`@IgnoreIf(hibernate5)`), `Test optimistic locking disabled with 'version
false'`
**State on `8.0.x-hibernate7` (before fix):**
- Class-level `@Requires(hibernate5 || hibernate7)` added - **excluded
MongoDB entirely**
- Same 3 features, `@IgnoreIf(hibernate5)` replaced by
`@IgnoreIf(hibernate5)` (unchanged)
- `Test versioning` was the only feature that could run on MongoDB before,
now blocked
**State on this branch (after fix):**
- Class-level `@Requires(hibernate5 || hibernate7 || mongodb)` - MongoDB
restored
- `@IgnoreIf(mongodb)` added on `Test optimistic locking` and `Test
optimistic locking disabled with 'version false'` because these use
Hibernate-specific transaction manager APIs
- `Test versioning` runs on all three datastores (hibernate5, hibernate7,
mongodb)
- Two new `withNewSession`-based features added: `Test optimistic locking
with withNewSession` and `Test optimistic locking disabled with 'version false'
using withNewSession`, gated with `@IgnoreIf(hibernate5 || hibernate7)` so they
only run on mongodb. These restore the spring-boot-4 `withNewSession` testing
pattern for mongodb alongside the new `withTransaction`-based tests for
hibernate.
- **Difference from spring-boot-4:** spring-boot-4 had no class-level gate
so the spec ran everywhere, and used `withNewSession` for optimistic locking
which worked on all datastores. Our branch adds an explicit `@Requires` that
includes all three real datastores. The two transaction-specific features have
`@IgnoreIf(mongodb)` (these use `transactionManager.commit` which is not
applicable to MongoDB). The two new `withNewSession` features have
`@IgnoreIf(hibernate5 || hibernate7)` to complement the transaction-based ones.
Net effect: all three datastores get optimistic locking coverage through
different mechanisms.
#### 3. PagedResultSpec (TCK)
**File:**
`grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/PagedResultSpec.groovy`
**State on `spring-boot-4`:**
- No annotations at all - ran for all datastores including core-test
**State on `8.0.x-hibernate7` (before fix):**
- `@Requires(hibernate5 || mongodb)` added - **excluded both core-test and
hibernate7**
- New `setupSpec()` with `manager.addAllDomainClasses([Person])`
**State on this branch (after fix):**
- `@Requires(hibernate5 || mongodb || core.gorm.suite)` - core-test restored
- hibernate7 is still excluded from this spec because a new companion
`PagedResultSpecHibernate` handles hibernate-specific paging (see Intentional
Test Splits section)
- **Difference from spring-boot-4:** spring-boot-4 had no `@Requires` at
all. Our branch has an explicit gate that includes hibernate5, mongodb, and
core but excludes hibernate7 (which now has its own companion spec). This is a
net improvement in clarity - the coverage is equivalent, just split across two
specs for hibernate7.
#### 4. OneToOneWithProxiesSpec (core-test)
**File:**
`grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/OneToOneWithProxiesSpec.groovy`
**State on `spring-boot-4`:**
- No annotations on the class or any feature method
- Ran freely in core-test
**State on `8.0.x-hibernate7` (before fix):**
- `@spock.lang.Requires(hibernate5 || hibernate7 || mongodb)` added on `Test
persist and retrieve unidirectional many-to-one` feature method
- Since core-test sets none of those properties, this feature was completely
skipped in the only module that runs this spec
**State on this branch (after fix):**
- `@spock.lang.Requires` removed - feature runs on all datastores again
- **Matches spring-boot-4:** Exact same state as spring-boot-4. No
annotations, no gating.
#### 5. EnumSpec (TCK)
**File:**
`grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/EnumSpec.groovy`
**State on `spring-boot-4`:**
- No class-level annotations
- Features: `Test save()`, `Test findByEnInList()` (with
`@Issue('GPMONGODB-248')`, uses `findByEn()` single-result finders), `Test
findBy()`, `Test findBy() with clearing the session`, `Test countBy()`
- All features ran ungated on all datastores including core-test
**State on `8.0.x-hibernate7` (before fix):**
- `Test findByEnInList()` was rewritten to `Test findByInList()` using
`findAllByEn()` and `findAllByEnInList()` (list finders) with
`@Requires(hibernate5 || hibernate7 || mongodb)` gate
- `Test findBy()` was rewritten to `Test findAllBy()` using `findAllByEn()`
with `@Requires(hibernate5 || hibernate7 || mongodb)` gate
- `Test findBy() with clearing the session` was rewritten to `Test
findAllBy() with clearing the session` using `findAllByEn()` with
`@Requires(hibernate5 || hibernate7 || mongodb)` gate
- A second `Test findAllBy()` feature was added (testing `findAllByEnInList`
multi-value) with `@Requires(hibernate5 || hibernate7 || mongodb)` gate
- The original single-result `findByEn()` tests were removed, eliminating
all ungated enum query coverage from core-test
**State on this branch (after fix):**
- Original `Test findByEnInList()`, `Test findBy()`, and `Test findBy() with
clearing the session` restored with single-result `findByEn()` finders and NO
gates - these run on all datastores including core-test
- All new `findAllBy*` versions retained with their `@Requires` gates for
hibernate5/hibernate7/mongodb
- **Difference from spring-boot-4:** spring-boot-4 had only the
single-result `findByEn()` versions. Our branch keeps those AND adds the new
`findAllBy*` list-result versions. More total coverage.
---
### Category B: Core-Test Specs Disabled by @IgnoreIf
**Commit:** `20131c0903` - fix: restore non-hibernate7 test coverage
All 6 specs had `@IgnoreIf({ System.getProperty('core.gorm.suite') == 'true'
})` added in the merge. Since `core.gorm.suite=true` is the system property set
in the `grails-datamapping-core-test` module, this effectively disabled all 6
specs in the only module they run in.
Three of the specs also had a broken `setupSpec()` due to a refactoring of
`GrailsDataTckManager`:
- The `domainClasses` field was made `private` with an empty default `[]`
- `getDomainClasses()` now returns
`Collections.unmodifiableList(domainClasses)`
- A new `addAllDomainClasses(Collection<Class>)` method was added for safe
mutation
- Old code using `manager.domainClasses.addAll(...)`, `manager.domainClasses
+= [...]`, or `manager.domainClasses << X` throws
`UnsupportedOperationException` at runtime
#### 1. CustomAutoTimestampSpec
**File:**
`grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomAutoTimestampSpec.groovy`
**State on `spring-boot-4`:**
- No class-level annotations
- `setupSpec()` uses `manager.domainClasses.addAll([...])` (old API)
- Feature-level `@Requires(hibernate5 || hibernate7 || mongodb)` on some
features that need real datastores
**State on `8.0.x-hibernate7` (before fix):**
- `@IgnoreIf(core.gorm.suite)` added at class level - **entire spec
disabled**
- `setupSpec()` still uses `manager.domainClasses.addAll([...])` - would
crash even if enabled
**State on this branch (after fix):**
- `@IgnoreIf` removed
- `setupSpec()` changed to `manager.addAllDomainClasses([...])`
- Feature-level `@Requires` annotations preserved (these are legitimate -
some features need a real datastore)
- **Difference from spring-boot-4:** The `setupSpec()` API call changed from
`manager.domainClasses.addAll()` to `manager.addAllDomainClasses()` to match
the new `GrailsDataTckManager` API. Otherwise functionally identical.
#### 2. EmbeddedPropertyQuerySpec
**File:**
`grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/EmbeddedPropertyQuerySpec.groovy`
**State on `spring-boot-4`:**
- No class-level annotations
- `setupSpec()` uses `manager.domainClasses += [Book2, Author2]` (old API)
- Feature-level `@Requires(hibernate5 || hibernate7 || mongodb)` on embedded
property query features
**State on `8.0.x-hibernate7` (before fix):**
- `@IgnoreIf(core.gorm.suite)` added at class level - **entire spec
disabled**
- `setupSpec()` still uses old `domainClasses +=` - would crash
**State on this branch (after fix):**
- `@IgnoreIf` removed
- `setupSpec()` changed to `manager.addAllDomainClasses([Book2, Author2])`
- Feature-level `@Requires` preserved
- **Difference from spring-boot-4:** Only the `setupSpec()` API call
changed. Otherwise functionally identical.
#### 3. WhereMethodEmbeddedInAssociationSpec
**File:**
`grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/WhereMethodEmbeddedInAssociationSpec.groovy`
**State on `spring-boot-4`:**
- No class-level annotations
- `setupSpec()` uses three `manager.domainClasses << Partner`, `<< Contact`,
`<< Address` (old API)
**State on `8.0.x-hibernate7` (before fix):**
- `@IgnoreIf(core.gorm.suite)` added at class level - **entire spec
disabled**
- `setupSpec()` still uses old `domainClasses <<` - would crash
**State on this branch (after fix):**
- `@IgnoreIf` removed
- `setupSpec()` changed to single `manager.addAllDomainClasses([Partner,
Contact, Address])`
- **Difference from spring-boot-4:** Only the `setupSpec()` API call
changed. Otherwise functionally identical.
#### 4. ReadOnlyCriteriaSpec
**File:**
`grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/ReadOnlyCriteriaSpec.groovy`
**State on `spring-boot-4`:**
- No class-level annotations
- Had `import org.apache.grails.data.testing.tck.domains.TestEntity` present
- No `setupSpec()` method
**State on `8.0.x-hibernate7` (before fix):**
- `@IgnoreIf(core.gorm.suite)` added at class level - **entire spec
disabled**
- `TestEntity` import was removed (or never migrated during the package
refactor)
**State on this branch (after fix):**
- `@IgnoreIf` removed
- `import org.apache.grails.data.testing.tck.domains.TestEntity` restored
- Added `setupSpec()` with `manager.addAllDomainClasses([TestEntity])` to
register the domain class
- **Difference from spring-boot-4:** The import was restored. A
`setupSpec()` was added which spring-boot-4 didn't have - this is needed
because the new `GrailsDataTckManager` requires explicit domain class
registration. Functionally identical behavior.
#### 5. OrderBySpec
**File:**
`grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/OrderBySpec.groovy`
**State on `spring-boot-4`:**
- No class-level annotations
- No `setupSpec()` method
- 4 features: `Test order by property name with dynamic finder returning
first result`, `Test order by property name with dynamic finder using max`,
`Test order by with list() method using max`, `Test order by with criteria
using maxResults`
**State on `8.0.x-hibernate7` (before fix):**
- `@IgnoreIf(core.gorm.suite)` added at class level - **entire spec
disabled**
**State on this branch (after fix):**
- `@IgnoreIf` removed
- Added `setupSpec()` with `manager.addAllDomainClasses([TestEntity])`
(needed for new TckManager API)
- `@Requires(hibernate5 || hibernate7 || mongodb)` added on `Test order by
property name with dynamic finder using max` because the simple datastore
returns empty results for `findAllBy*` with `max` pagination parameter
- **Difference from spring-boot-4:** One feature now has `@Requires` that
spring-boot-4 didn't have. The simple datastore never actually supported
`findAllByAgeGreaterThanEquals(40, [sort: "age", order: 'desc', max: 1])` - it
silently returned empty results. The gate makes this explicit. The other 3
features run ungated.
#### 6. QueryAssociationSpec
**File:**
`grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/QueryAssociationSpec.groovy`
**State on `spring-boot-4`:**
- No class-level annotations
- No `setupSpec()` method
- Imports included `PlantCategory` and `TestEntity`
**State on `8.0.x-hibernate7` (before fix):**
- `@IgnoreIf(core.gorm.suite)` added at class level - **entire spec
disabled**
- `PlantCategory` and `Plant` not registered in `addAllDomainClasses`
**State on this branch (after fix):**
- `@IgnoreIf` removed
- Added `setupSpec()` with `manager.addAllDomainClasses([TestEntity,
ChildEntity, PlantCategory, Plant])`
- Added `Plant` import (was missing)
- **Difference from spring-boot-4:** Added `setupSpec()` for domain class
registration (new API requirement) and added the `Plant` import that was
missing. Functionally equivalent - the tests that use
`PlantCategory.addToPlants()` now have the domain class properly registered.
---
### Category C: @DelegateAsync Classloader Regression
**Commit:** `0d8f43deb5` - fix: @DelegateAsync AST transform classloader
regression
#### AsyncTransactionalServiceSpec
**Files:**
-
`grails-async/core/src/main/groovy/org/grails/async/transform/internal/DelegateAsyncTransformation.java`
-
`grails-async/plugin/src/test/groovy/grails/async/services/AsyncTransactionalServiceSpec.groovy`
**State on `spring-boot-4`:**
- `AsyncTransactionalServiceSpec`: No annotations - ran normally and passed
- `DelegateAsyncTransformation.java` line 201:
`getClass().getClassLoader().loadClass("...DefaultDelegateAsyncTransactionalMethodTransformer")`
**State on `8.0.x-hibernate7` (before fix):**
- `AsyncTransactionalServiceSpec`: `@Ignore("FIX THIS LATER")` added on the
test feature - **test completely disabled**
- `DelegateAsyncTransformation.java` line 202:
`Thread.currentThread().getContextClassLoader().loadClass("...DefaultDelegateAsyncTransactionalMethodTransformer")`
- classloader changed
**State on this branch (after fix):**
- `AsyncTransactionalServiceSpec`: `@Ignore` removed, unused imports removed
- test runs and passes
- `DelegateAsyncTransformation.java`: Reverted to
`getClass().getClassLoader()` - matches spring-boot-4
- **Matches spring-boot-4:** Both files restored to functionally identical
state as spring-boot-4. The classloader is `getClass().getClassLoader()` and
the test has no suppression annotations.
**Root Cause:** During Gradle AST compilation, the thread context
classloader does not have access to the plugin module's classes. The
`ServiceLoader.load()` call using the thread context classloader silently fails
to find `DefaultDelegateAsyncTransactionalMethodTransformer` and falls back to
`NoopDelegateAsyncTransactionalMethodTransformer`. This means:
- `@DelegateAsync` services no longer implement `TransactionManagerAware`
- The `setTransactionManager()` method is never generated on async service
wrappers
- The `AsyncTransactionalServiceSpec` test that verifies transactional async
behavior fails
**Why `getClass().getClassLoader()` is correct:** The
`DelegateAsyncTransformation` class is loaded by the Gradle build's classloader
which has visibility into the plugin module. The
`DefaultDelegateAsyncTransactionalMethodTransformer` is in the same classloader
hierarchy. `Thread.currentThread().getContextClassLoader()` in the compilation
context is a different classloader that lacks the plugin module's classes.
---
### Category D: DynamicFinder Boolean+OR Junction Bug
**Commit:** `c6bb085cc5` - fix: DynamicFinder boolean+OR query junction logic
#### DynamicFinder.getJunction()
**File:**
`grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/DynamicFinder.java`
**State on `spring-boot-4`:**
- `DynamicFinder` did not have a `getJunction()` method. Each finder
subclass (`FindAllByFinder`, `FindByFinder`) had its own `buildQuery()` that
constructed the query junction inline.
- For boolean+OR patterns like `findPublishedByTitleOrAuthor`, the logic
correctly produced: `published AND (title OR author)` - a top-level Conjunction
wrapping the boolean criterion and a nested Disjunction for the remaining
expressions.
**State on `8.0.x-hibernate7` (before fix):**
- `buildQuery()` was refactored into the parent `DynamicFinder` class with a
new `getJunction()` method
- The boolean+OR case created a Disjunction, then placed a Conjunction
containing the boolean criterion INSIDE the Disjunction: `(boolean) OR expr1 OR
expr2`
- This produced incorrect queries - the boolean criterion was ORed instead
of ANDed
**State on this branch (after fix):**
- `getJunction()` restructured: when `firstExpressionIsRequiredBoolean()` is
true AND operator is OR, creates a top-level Conjunction containing the boolean
criterion AND a nested Disjunction for remaining expressions
- Produces the correct: `boolean AND (expr1 OR expr2)`
- **Difference from spring-boot-4:** The code structure is different
(centralized `getJunction()` vs inline per-finder logic) but the query
semantics are identical. This is a bug fix to make the refactored code produce
the same results as the pre-refactoring code.
**Impact:** Without this fix, `findPublished*Or*` dynamic finders (and any
finder where the domain class has a boolean property matched by
`firstExpressionIsRequiredBoolean()`) would produce incorrect queries on all
datastores.
## Intentional Changes Reviewed and Confirmed
The following suppressions were added in the merge but are intentional and
do NOT reduce non-hibernate7 coverage.
### Hibernate7-Only Skips
These specs skip only when `hibernate7.gorm.suite=true`. They still run for
Hibernate 5, MongoDB, and core:
| Spec | spring-boot-4 State | 8.0.x-hibernate7 State | Assessment |
|------|---------------------|----------------------|------------|
| DirtyCheckingSpec | No annotations | `@IgnoreIf(hibernate7)` at class
level | **New, intentional.** Covered by dedicated
`DirtyCheckingSpecHibernate7` in the hibernate7 module. Non-hibernate7 coverage
unchanged. |
| NamedQuerySpec | No annotations | `@IgnoreIf(hibernate7)` at class level |
**New, intentional.** Named queries behave differently in Hibernate 7.
Non-hibernate7 coverage unchanged. |
| RLikeSpec | No annotations | `@IgnoreIf(hibernate7)` at class level |
**New, intentional.** Regex-like queries differ in Hibernate 7. Non-hibernate7
coverage unchanged. |
| UpdateWithProxyPresentSpec | No annotations | `@IgnoreIf(hibernate7)` at
class level | **New, intentional.** Proxy handling differs in Hibernate 7.
Non-hibernate7 coverage unchanged. |
### Intentional Test Splits
These specs were intentionally split into base + Hibernate-specific versions
to handle behavioral differences between datastores:
| Base Spec | Companion Spec | spring-boot-4 State | 8.0.x-hibernate7 State
| Assessment |
|-----------|---------------|---------------------|----------------------|------------|
| SizeQuerySpec | SizeQuerySpecHibernate (NEW) | No annotations, used
`withCriteria` for size queries | Base: `@IgnoreIf(hibernate5 \|\|
hibernate7)`, uses `.where{}`. Companion: `@IgnoreIf(mongodb)`, uses
`withCriteria`. | **Intentional split.** spring-boot-4 had one spec; now split
into two covering the same total behavior. SizeQuerySpec runs on mongodb/core,
SizeQuerySpecHibernate runs on hibernate5/hibernate7. Total coverage
equivalent. |
| ValidationSpec | ValidationHibernateSpec (NEW) |
`@PendingFeatureIf(hibernate5)` on 2 features, `@IgnoreIf(neo4j)` on 1 |
Removed one `@PendingFeatureIf(hibernate5)` (improvement), added `hibernate7`
to remaining `@PendingFeatureIf`. New companion with `@Requires(hibernate7)`
covers hibernate7-specific validation. | **Intentional split.** Non-hibernate7
coverage unchanged. The `@PendingFeatureIf(hibernate5)` removal on `Test
validating an object that has had values rejected with an ObjectError` is an
improvement - this test now runs on hibernate5. |
| PagedResultSpec | PagedResultSpecHibernate (NEW) | No annotations, ran
everywhere | Base: `@Requires(hibernate5 \|\| mongodb \|\| core)`. Companion:
`@IgnoreIf(core \|\| mongodb)`. | **Intentional split.** The `core.gorm.suite`
addition was our fix (Category A above). Hibernate-specific paging tests moved
to companion. Non-hibernate7 coverage restored by our fix. |
### Hibernate6-to-Hibernate7 Renames
All references to `hibernate6.gorm.suite` were renamed to
`hibernate7.gorm.suite` throughout the codebase. This is expected - the module
was renamed from Hibernate 6 to Hibernate 7.
| Spec | spring-boot-4 State | 8.0.x-hibernate7 State | Assessment |
|------|---------------------|----------------------|------------|
| GroovyProxySpec | `@IgnoreIf(hibernate5 \|\| hibernate6)` |
`@IgnoreIf(hibernate5 \|\| hibernate7)` | **Simple rename.** `hibernate6`
changed to `hibernate7`. Groovy proxies are not used with Hibernate - this skip
was always present. Still runs on mongodb and core. |
| BuiltinUniqueConstraintWorksWithTargetProxiesConstraintsSpec |
`@PendingFeatureIf(!hibernate5 && !hibernate6 && !mongodb)` on 2 features |
`@PendingFeatureIf(!hibernate5 && !hibernate7 && !mongodb)` on 2 features |
**Simple rename.** `hibernate6` changed to `hibernate7`. Same semantics - these
features are pending on core datastore only. |
| DirtyCheckingAfterListenerSpec | `@PendingFeatureIf(!hibernate5 &&
!hibernate6 && !mongodb)` | `@PendingFeatureIf(!hibernate5 && !mongodb)` |
**Improvement.** `hibernate7` was dropped entirely (not just renamed). This
means the test is now expected to PASS on hibernate7 - the feature was fixed.
Non-hibernate7 behavior unchanged. |
### Pre-Existing Suppressions
| Spec | Module | spring-boot-4 State | 8.0.x-hibernate7 State | Assessment |
|------|--------|---------------------|----------------------|------------|
| UniqueConstraintHibernateSpec | hibernate5 | `@Ignore` on one test (added
in commit `5564ffa81c` on spring-boot-4 branch) | `@Ignore` on the same test |
**Pre-existing.** This `@Ignore` was already present before the merge. No
change in coverage. |
| WhereQueryOldIssueVerificationSpec | hibernate5 | No annotations |
`@PendingFeatureIf(hibernate5)` on one test | **New, intentional.** Marks a
known hibernate5-specific issue as pending. The test still runs on hibernate5
but is expected to fail (pending feature). Does not suppress the test on any
other datastore. |
### TCK OrderBySpec
| Item | spring-boot-4 State | 8.0.x-hibernate7 State | Assessment |
|------|---------------------|----------------------|------------|
| TCK OrderBySpec | No annotations | `@IgnoreIf(core.gorm.suite)` at class
level | **Intentional.** The TCK version uses `createCriteria().list{}`,
`list(sort:)`, and `findAllByAgeGreaterThanEquals()` which need a real
datastore. The core-test module has its own separate `OrderBySpec` (restored in
Category B above) that covers ordering behavior using APIs compatible with the
simple datastore. TCK version still runs for hibernate5, hibernate7, and
mongodb. |
## Modules Scanned with No Issues Found
| Module | Test Files Changed | New Suppressions |
|--------|-------------------|-----------------|
| `grails-data-mongodb/core` | 1 | 0 |
| `grails-data-hibernate5/core` | 50+ | 0 (all pre-existing or intentional) |
| `grails-data-hibernate5/grails-plugin` | 1 | 0 |
| `grails-gsp/grails-taglib` | 1 | 0 |
| `grails-gsp/plugin` | 1 | 0 |
| `grails-test-suite-uber` | 1 | 0 |
| `grails-test-suite-web` | 1 | 0 |
| `grails-datastore-core` | 1 | 0 |
| `grails-test-examples/*` | Multiple | 0 |
| `build-logic/plugins` | 1 | 0 |
## Verification
All three test suites verified green after all changes:
- `./gradlew :grails-datamapping-core-test:test` - BUILD SUCCESSFUL
- `./gradlew :grails-data-hibernate5-core:test` - BUILD SUCCESSFUL
- `./gradlew :grails-data-mongodb-core:test` - BUILD SUCCESSFUL
- `./gradlew :grails-async:test` - BUILD SUCCESSFUL
(AsyncTransactionalServiceSpec PASSED)
## Architecture Notes
### Test System Layers
```
grails-datamapping-tck/src/main/groovy/
Shared TCK test specs, packaged as JAR.
Extracted at build time into build/extracted-tck-classes/ for
hibernate5, hibernate7, mongodb, and core-test modules.
grails-datamapping-core-test/src/test/groovy/
Module-specific tests that run ONLY in this module
with system property core.gorm.suite=true.
grails-data-hibernate5/core/src/test/groovy/
grails-data-hibernate7/core/src/test/groovy/
grails-data-mongodb/core/src/test/groovy/
Module-specific tests for each datastore implementation.
```
### Suite System Properties
| Property | Module |
|----------|--------|
| `core.gorm.suite=true` | `grails-datamapping-core-test` |
| `hibernate5.gorm.suite=true` | `grails-data-hibernate5` modules |
| `hibernate7.gorm.suite=true` | `grails-data-hibernate7` modules |
| `mongodb.gorm.suite=true` | `grails-data-mongodb` modules |
### GrailsDataTckManager API Change
The `8.0.x-hibernate7` merge refactored `GrailsDataTckManager`:
```groovy
// BEFORE (spring-boot-4):
manager.domainClasses.addAll([Foo, Bar]) // worked
manager.domainClasses += [Foo, Bar] // worked
manager.domainClasses << Foo // worked
// AFTER (8.0.x-hibernate7):
manager.addAllDomainClasses([Foo, Bar]) // new API
// All old patterns throw UnsupportedOperationException
// because getDomainClasses() returns Collections.unmodifiableList()
```
## Summary
| Category | Issues Found | Issues Fixed | Tests Restored |
|----------|-------------|-------------|----------------|
| A: TCK gating | 5 specs | 5 specs | ~25 test methods |
| B: Core-test @IgnoreIf | 6 specs | 6 specs | ~30 test methods |
| C: @DelegateAsync regression | 1 spec | 1 spec (+ 1 source file) | 1 test
method |
| D: DynamicFinder bug | 1 source file | 1 source file | Correctness fix for
all boolean+OR finders |
| **Total** | **12 specs + 1 source** | **12 specs + 2 source files** |
**~56 test methods + bug fix** |
### Commit Structure
| Commit | Hash | Files | Description |
|--------|------|-------|-------------|
| 1 | `c6bb085cc5` | 1 | fix: DynamicFinder boolean+OR query junction logic |
| 2 | `0d8f43deb5` | 2 | fix: @DelegateAsync AST transform classloader
regression |
| 3 | `20131c0903` | 11 | fix: restore non-hibernate7 test coverage |
14 files changed, 500 insertions, 72 deletions across 3 commits.
--
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]
