jamesfredley commented on PR #15731:
URL: https://github.com/apache/grails-core/pull/15731#issuecomment-4724165663
## Update: rebased on `8.0.x-hibernate7` and pivoted off BOM substitution
I merged the latest `8.0.x-hibernate7`, resolved the conflicts, and reworked
the functional-coverage approach based on the review discussion. Pushed as
`83e5ebd454` (plus the merge commit `798446f7a1`).
### Addressing the substitution objection
@jdaugherty - you're right, and the arbitrary BOM substitution is gone. No
more `redirectBomToH7`, no `grails-bom` -> `grails-hibernate7-bom` redirect, no
`grails-data-hibernate5` -> `grails-data-hibernate7` swap, and no
`-PhibernateVersion` matrix in the general `functional` job. Default dependency
resolution is back to exactly what it was, so
`dependencies`/`dependencyInsight` debugging and Gradle build caching are
unaffected, and coverage reports attribute cleanly to each module.
@matrei - this also avoids duplicating whole apps and needing shell
scripts/switches to run both versions. The shared `grails-datamapping-tck` is
already run by **both** `grails-data-hibernate5` and `grails-data-hibernate7`
via the `GrailsDataTckManager` ServiceLoader, so a single spec written once
executes natively against both Hibernate lines with no flags. That keeps it DRY
without hiding behavior behind a switch.
### What changed
**Sync / merge (`798446f7a1`)**
- Merged `origin/8.0.x-hibernate7`.
- Resolved 3 conflicts by adopting the base conventions:
`registerDomainClasses()` (replacing `addAllDomainClasses`), kept the base
`setupSpec()` in `Hibernate7OptimisticLockingSpec`, kept the PR's
`HibernateGormStaticApiMappedPropertyEntity` coverage + `AvailableHints` import
in `HibernateGormStaticApiSpec`, and dropped the `@Nonnull` annotation in
`CompositeIdWithJoinTableSpec` to match base.
- Fixed merge-induced stale `grails.gorm.specs.*` imports across 7 specs
(the base relocated `HibernateGormDatastoreSpec` and the multitenancy support
classes to `grails.gorm.tests`), so both core test modules compile again.
**Substitution removal + shared coverage (`83e5ebd454`)**
- `gradle/functional-test-config.gradle`: restored to the base version (all
substitution machinery removed).
- `.github/workflows/gradle.yml`: dropped the `hibernate-version` matrix
from the general `functional` job; **kept** the publish gate that requires the
existing `hibernate7Functional` job (which runs
`grails-test-examples/hibernate7/*`), so H7 functional results still block
publishing.
- `grails-datamapping-tck`: added shared coverage for the version-agnostic
GORM behavior this PR fixed:
- `FindWhereSpec`: `findWhere`/`findAllWhere` null matching (`is null`,
not `= null`), and `findWhere` returning a single instance when multiple rows
match.
- `GormEnhancerSpec`: `getAll` order preservation, including convertible
`String` ids, and a `null` slot for a missing id.
### Verification
- Both core test modules compile after the merge.
- The 6 new TCK specs pass on **both** lanes: `grails-data-hibernate5-core`
(full suite, 680 tests, 0 failures) and `grails-data-hibernate7-core` (22
tests, 0 failures). No behavioral divergence for these contracts between H5.6
and H7.4.
- CodeNarc clean on `grails-datamapping-tck`.
- Hibernate ORM stays on `7.4.1.Final` / Hibernate Tools `7.3.8.Final`
(unchanged).
### Remaining follow-up (TCK-first)
The H7-only architecture tests (domain-binding, HQL infrastructure) stay
module-local since they have no H5 equivalent class. The broader parity sweep
continues with the same pattern:
- Port the remaining version-agnostic behavioral specs (e.g. the rest of
`HibernateGormStaticApiSpec`'s finder/`getAll` contracts, query-setting
coercion) into the TCK so they run on both lines, and add the matching H5
coverage where H5 currently lacks it.
- Add dedicated Hibernate 7 example specs/apps only for behavior that
genuinely cannot move to the TCK: multi-datasource/OSIV (`datasources`), and
association rendering / unique constraints (`views-functional-tests`,
`scaffolding-fields`).
- Any intentional H5.6 -> H7.4 breaking change stays documented in the
migration checklist above and gets a test locking the behavior on each side.
The "Detailed issue coverage" and follow-up tables at the top have been
updated to match.
--
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]
