jdaugherty commented on code in PR #14668:
URL: https://github.com/apache/grails-core/pull/14668#discussion_r2058255218
##########
.github/workflows/gradle.yml:
##########
@@ -212,20 +213,18 @@ jobs:
with:
develocity-access-key: ${{ secrets.GRAILS_DEVELOCITY_ACCESS_KEY }}
- name: "📤 Publish Grails-Core Snapshot Artifacts"
- id: publish
env:
GRAILS_PUBLISH_RELEASE: 'false'
MAVEN_PUBLISH_URL: ${{ secrets.GRAILS_NEXUS_PUBLISH_SNAPSHOT_URL }}
MAVEN_PUBLISH_USERNAME: ${{ secrets.NEXUS_USER }}
MAVEN_PUBLISH_PASSWORD: ${{ secrets.NEXUS_PW }}
- run: >
- ./gradlew publish
+ run: ./gradlew publish
docs:
if: github.repository_owner == 'apache' && github.event_name == 'push'
- needs: [publish]
- runs-on: ubuntu-latest
+ needs: [ publish ]
+ runs-on: ubuntu-24.04
permissions:
- contents: write
+ contents: write # TODO: Comment why this is needed
Review Comment:
This isn't needed because we will commit to a different repo
##########
grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomAutoTimestampSpec.groovy:
##########
@@ -94,6 +93,7 @@ class CustomAutoTimestampSpec extends GormDatastoreSpec {
def now = new Date()
r.created = new Date(now.time)
r.modified = r.created
+ sleep(50) // give the save a chance to set a different time
Review Comment:
Can't this just be sleep(1)?
##########
grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomAutoTimestampSpec.groovy:
##########
@@ -22,15 +22,9 @@ import grails.gorm.annotation.AutoTimestamp
import grails.gorm.tests.GormDatastoreSpec
import grails.persistence.Entity
import org.grails.datastore.gorm.events.AutoTimestampEventListener
-import spock.lang.IgnoreIf
-import spock.lang.Retry
-import spock.lang.Stepwise
import static grails.gorm.annotation.AutoTimestamp.EventType.CREATED
-@Retry // this test is flaky on CI due to
https://github.com/apache/grails-data-mapping/issues/1877
Review Comment:
I think this is still failing because there is a shared, static variable
thats being updated separately by another test. The comment should stay
##########
grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/TckTestSuite.groovy:
##########
@@ -21,14 +21,12 @@ package org.grails.datastore.gorm
import grails.gorm.tck.NotInListSpec
import org.junit.platform.suite.api.SelectClasses
import org.junit.platform.suite.api.Suite
-import spock.lang.IgnoreIf
/**
* Use this class to run tck classes against the current implementation.
*
* @author graemerocher
*/
-@IgnoreIf({ os.windows }) // Fails on windows even though the NotInListSpec is
in the TCK and runs
Review Comment:
I ran this multiple times and it kept failing in CI. James F couldn't
reproduce locally though. We never could explain why it was failing. We don't
need it for CI, so I suggest we just disable if GITHUB_REF is present
##########
grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomAutoTimestampSpec.groovy:
##########
@@ -63,24 +59,27 @@ class CustomAutoTimestampSpec extends GormDatastoreSpec {
def now = new Date()
r.created = new Date(now.time)
r.modified = r.created
+ sleep(50) // give the save a chance to set a different time
r.save(flush:true, failOnError:true)
session.clear()
r = RecordCustom.get(r.id)
then:"the custom lastUpdated and dateCreated are set"
- now < r.modified
- now < r.created
+ now.time < r.modified.time
+ now.time < r.created.time
when:"An entity is modified"
Date previousCreated = r.created
Date previousModified = r.modified
r.name = "Test 2"
+ sleep(50) // give the save a chance to set a different time
Review Comment:
sleep(1)?
##########
grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomAutoTimestampSpec.groovy:
##########
@@ -41,19 +35,21 @@ class CustomAutoTimestampSpec extends GormDatastoreSpec {
r = RecordCustom.get(r.id)
then:"the custom lastUpdated and dateCreated are set"
- r.modified != null && r.modified < new Date()
- r.created != null && r.created < new Date()
+ r.modified != null
+ r.created != null
when:"An entity is modified"
Date previousCreated = r.created
Date previousModified = r.modified
r.name = "Test 2"
+ sleep(50) // give the save a chance to set a different time
Review Comment:
sleep(1)
##########
grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomAutoTimestampSpec.groovy:
##########
@@ -106,16 +106,15 @@ class CustomAutoTimestampSpec extends GormDatastoreSpec {
Date previousCreated = r.created
Date previousModified = r.modified
r.name = "Test 2"
+ sleep(50) // give the save a chance to set a different time
Review Comment:
Doesn't this just need to be sleep(1)?
##########
grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomAutoTimestampSpec.groovy:
##########
@@ -41,19 +35,21 @@ class CustomAutoTimestampSpec extends GormDatastoreSpec {
r = RecordCustom.get(r.id)
then:"the custom lastUpdated and dateCreated are set"
- r.modified != null && r.modified < new Date()
- r.created != null && r.created < new Date()
+ r.modified != null
Review Comment:
You could add a sleep(1) and then restore these time checks
##########
grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomAutoTimestampSpec.groovy:
##########
@@ -63,24 +59,27 @@ class CustomAutoTimestampSpec extends GormDatastoreSpec {
def now = new Date()
r.created = new Date(now.time)
r.modified = r.created
+ sleep(50) // give the save a chance to set a different time
Review Comment:
sleep(1)?
--
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]
