matrei commented on code in PR #15654:
URL: https://github.com/apache/grails-core/pull/15654#discussion_r3354033919
##########
.github/workflows/gradle.yml:
##########
@@ -18,6 +18,7 @@ on:
push:
branches:
- '[0-9]+.[0-9]+.x'
+ - '8.0.x-hibernate7.*'
Review Comment:
This should not be necessary, the `pull_request:` below should trigger
running the workflow.
##########
.github/workflows/codeql.yml:
##########
@@ -30,10 +30,12 @@ on:
push:
branches:
- '[4-9]+.[0-9]+.x'
+ - '8.0.x-hibernate7.*'
pull_request:
# The branches below must be a subset of the branches above
Review Comment:
Hmm, I wonder why we are filtering pull_request target branches?
##########
.github/workflows/groovy-joint-workflow.yml:
##########
@@ -17,10 +17,12 @@ name: "CI - Groovy Joint Validation Build"
on:
push:
branches:
- - '[4-9]+.[0-9]+.x'
+ - '[0-9]+.[0-9]+.x'
+ - '8.0.x-hibernate7.*'
pull_request:
branches:
Review Comment:
I wonder why we are filtering pull_request target branches?
##########
grails-geb/src/testFixtures/groovy/grails/plugin/geb/WebDriverContainerHolder.groovy:
##########
@@ -363,7 +359,18 @@ class WebDriverContainerHolder {
container.host !=
ContainerGebConfiguration.DEFAULT_HOSTNAME_FROM_CONTAINER
}
- private static DockerImageName createDockerImageName(String browserName) {
+ private DockerImageName createDockerImageName(String browserName) {
+ // If a template is provided (e.g. via
grails.geb.container.image.template in local.properties),
Review Comment:
We should not mention `local.properties` here. That is specific to
`grails-core` repository Gradle setup.
##########
grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/QueryEventsSpec.groovy:
##########
@@ -33,39 +31,42 @@ import
org.grails.datastore.mapping.query.event.PreQueryEvent
/**
* Tests for query events.
*/
-// TODO: the application context is null on hibernate tck tests, so this test
errors on the add of the application listener
-@IgnoreIf({ System.getProperty('hibernate5.gorm.suite') ||
System.getProperty('hibernate6.gorm.suite') ||
System.getProperty('mongodb.gorm.suite') })
class QueryEventsSpec extends GrailsDataTckSpec {
SpecQueryEventListener listener
+ boolean contextAvailable = false
Review Comment:
We should document why this property is necessary.
##########
.github/workflows/rat.yml:
##########
@@ -17,14 +17,12 @@ name: "Licensing - RAT Report"
on:
push:
branches:
- - '[4-9]+.[0-9]+.x'
- - '[3-9]+.[3-9]+.x'
- - license-audit
+ - '[0-9]+.[0-9]+.x'
+ - '8.0.x-hibernate7.*'
pull_request:
branches:
Review Comment:
I wonder why we are filtering pull_request target branches?
##########
grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/SessionCreationEventSpec.groovy:
##########
@@ -31,20 +29,22 @@ import
org.grails.datastore.mapping.core.SessionCreationEvent
/**
* Test case that session creation events are fired.
*/
-// TODO: the application context is null on hibernate tck tests, so this test
errors on the add of the application listener
-@IgnoreIf({ System.getProperty('hibernate5.gorm.suite') ||
System.getProperty('hibernate6.gorm.suite') ||
System.getProperty('mongodb.gorm.suite') })
class SessionCreationEventSpec extends GrailsDataTckSpec {
- @Override
+ Listener listener
+ boolean contextAvailable = false
Review Comment:
Document?
##########
grails-data-hibernate5/grails-plugin/src/test/groovy/grails/test/mixin/hibernate/HibernateSpecSpec.groovy:
##########
@@ -26,6 +26,7 @@ import grails.test.hibernate.HibernateSpec
*/
class HibernateSpecSpec extends HibernateSpec {
+
Review Comment:
?
##########
grails-test-examples/gorm/src/integration-test/groovy/gorm/GormCriteriaQueriesSpec.groovy:
##########
@@ -24,15 +24,14 @@ import spock.lang.Unroll
import grails.gorm.DetachedCriteria
import grails.gorm.transactions.Rollback
import grails.testing.mixin.integration.Integration
-
/**
* Tests for GORM Criteria Queries - both createCriteria() and
DetachedCriteria.
*
* Criteria queries provide a type-safe, programmatic way to build
* complex queries without writing HQL strings.
*/
@Rollback
-@Integration
+@Integration(applicationClass = Application)
Review Comment:
Why was `(applicationClass = Application)` added?
--
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]
