Copilot commented on code in PR #15561:
URL: https://github.com/apache/grails-core/pull/15561#discussion_r3047031206
##########
gradle/functional-test-config.gradle:
##########
@@ -21,6 +21,27 @@ rootProject.subprojects
.findAll { !(it.name in testProjects) && !(it.name in docProjects) &&
!(it.name in cliProjects) }
.each { project.evaluationDependsOn(it.path) }
+// Determine which Hibernate version to use for general functional tests.
+// Pass -PhibernateVersion=7 to run general functional tests against Hibernate
7 instead of 5.
+def targetHibernateVersion = project.findProperty('hibernateVersion') ?: '5'
+boolean isHibernateSpecificProject =
project.name.startsWith('grails-test-examples-hibernate5') ||
+ project.name.startsWith('grails-test-examples-hibernate7')
+boolean isMongoProject =
project.name.startsWith('grails-test-examples-mongodb')
+boolean isGeneralFunctionalTest = !isHibernateSpecificProject &&
!isMongoProject
+
Review Comment:
`hibernateVersion` is accepted as an arbitrary string and silently falls
back to “Hibernate 5 behavior” for any value other than `'7'`. That can hide
typos/misconfiguration (especially for local runs). Consider normalizing +
validating the value (e.g., allow only `'5'`/`'7'` and throw a
`GradleException` otherwise) so the build fails fast when an unsupported value
is provided.
##########
gradle/functional-test-config.gradle:
##########
@@ -80,6 +122,20 @@ tasks.withType(Test).configureEach { Test task ->
}
}
+ // Skip hibernate5-labeled projects when -PskipHibernate5Tests is set
+ if (project.hasProperty('skipHibernate5Tests')) {
+ if (!isHibernate5) {
+ return false
+ }
+ }
+
+ // Skip hibernate7-labeled projects when -PskipHibernate7Tests is set
+ if (project.hasProperty('skipHibernate7Tests')) {
+ if (!isHibernate7) {
+ return false
+ }
+ }
Review Comment:
The `skipHibernate5Tests` / `skipHibernate7Tests` logic relies on
`isHibernate5` / `isHibernate7` being defined as negated `startsWith(...)`
checks (so `isHibernate5 == false` actually means “this *is* an
hibernate5-labeled project”). This inverted naming makes the skip conditions
hard to reason about and easy to break with future edits. Consider redefining
these booleans to match their names (or renaming them to
`isNotHibernate5Project`/etc.) and then update the `onlyIf` conditions
accordingly.
--
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]
