You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "jamesfredley (via GitHub)" <gi...@apache.org> on 2026/02/17 03:25:08 UTC
[PR] test: stabilize flaky scaffolding Geb tests for slow CI environments [grails-core]
jamesfredley opened a new pull request, #15394:
URL: https://github.com/apache/grails-core/pull/15394
## Summary
Stabilizes flaky `UserControllerSpec` and `UserCommunityControllerSpec` Geb tests in `grails-test-examples/scaffolding` that intermittently fail on slower CI runners (observed on JDK 25 in [Functional Tests (25)](https://github.com/apache/grails-core/actions/runs/22083891105/job/63814394408?pr=15393)).
## Problem
The test expects page title `User List` but intermittently gets `Please sign in` because:
1. **No wait after login** — `setup()` clicks the login button then immediately proceeds to the feature method. On slower environments, the login POST hasn't completed, so the session cookie isn't set when `go 'user/index'` executes, causing Spring Security to redirect to the login page.
2. **No waitFor on assertions** — `title == 'User List'` asserts immediately without waiting for async page load, unlike every other Geb spec in this repo.
These were the **only two Geb specs** not updated with `waitFor` patterns during the stability improvements in e8adf0143f.
## Changes
- Add `waitFor { title != 'Please sign in' }` after login click to verify auth completes before proceeding
- Wrap `title == 'User List'` assertions with `waitFor { }` for async page load handling
- Add `waitFor` for logout button visibility before clicking in `cleanup()`
- Remove redundant `go '/'` before `to LoginPage` (the `to` command navigates directly)
## Evidence
| Signal | Detail |
|--------|--------|
| Same test passed on `7.0.x` | [Run 22018179657](https://github.com/apache/grails-core/actions/runs/22018179657) ✅ |
| Same test passed on JDK 17 & 21 | Same CI run, only JDK 25 failed |
| Failure pattern | Classic auth timing flake — `Please sign in` instead of `User List` |
| Established pattern | All other Geb specs use `waitFor` extensively (see `ScaffoldingFunctionalSpec`, `FieldsValidationSpec`, `PaginationSpec`) |
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: stabilize flaky scaffolding Geb tests for slow CI environments [grails-core]
Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei merged PR #15394:
URL: https://github.com/apache/grails-core/pull/15394
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: stabilize flaky scaffolding Geb tests for slow CI environments [grails-core]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #15394:
URL: https://github.com/apache/grails-core/pull/15394#discussion_r2817614994
##########
grails-test-examples/scaffolding/src/integrationTest/groovy/com/example/UserCommunityControllerSpec.groovy:
##########
@@ -16,43 +16,31 @@
* specific language governing permissions and limitations
* under the License.
*/
-
package com.example
import com.example.pages.LoginPage
+import com.example.pages.LogoutPage
+import com.example.pages.CommunityUserListPage
import grails.plugin.geb.ContainerGebSpec
import grails.testing.mixin.integration.Integration
-@Integration(applicationClass = Application)
+@Integration
class UserCommunityControllerSpec extends ContainerGebSpec {
void setup() {
- go '/'
- to LoginPage
- username = 'test@grails.org'
- password = 'letmein'
- loginButton.click()
+ to(LoginPage).login()
}
void cleanup() {
- try {
- go 'logout'
- $('input', value: 'Log Out').click()
- }
- catch (ignored) {
- // ignored
- }
+ to(LogoutPage).logout()
Review Comment:
`cleanup()` no longer ignores logout failures; it always does `to(LogoutPage).logout()`. If the session is already unauthenticated (or `setup()` failed), `/logout` may redirect to the login page and the `LogoutPage.at` check will fail during teardown, turning unrelated issues into spec failures. Consider adding a guard (e.g., skip when at `LoginPage`) or restoring a try/catch around logout to keep cleanup resilient.
```suggestion
try {
to(LogoutPage).logout()
} catch (Exception ignored) {
// Ignore logout failures to keep cleanup resilient
}
```
##########
grails-test-examples/scaffolding/src/integrationTest/groovy/com/example/UserControllerSpec.groovy:
##########
@@ -16,43 +16,31 @@
* specific language governing permissions and limitations
* under the License.
*/
-
package com.example
import com.example.pages.LoginPage
+import com.example.pages.LogoutPage
+import com.example.pages.UserListPage
import grails.plugin.geb.ContainerGebSpec
import grails.testing.mixin.integration.Integration
-@Integration(applicationClass = Application)
+@Integration
class UserControllerSpec extends ContainerGebSpec {
void setup() {
- go '/'
- to LoginPage
- username = 'test@grails.org'
- password = 'letmein'
- loginButton.click()
+ to(LoginPage).login()
}
void cleanup() {
- try {
- go 'logout'
- $('input', value: 'Log Out').click()
- }
- catch (ignored) {
- // ignored
- }
+ to(LogoutPage).logout()
Review Comment:
`cleanup()` used to swallow logout/navigation errors, but it now unconditionally navigates to `/logout` and performs an `at` check via `to(LogoutPage)`. If the session is already logged out (or login failed in `setup()`), `/logout` can redirect back to the login page and cause teardown to fail the spec for an unrelated reason. Consider reintroducing a guard (e.g., only attempt logout when not at `LoginPage`, or wrap logout in a try/catch) so cleanup doesn’t introduce new flakiness or mask the original failure cause.
```suggestion
try {
// Only attempt logout if we're not already on the login page.
if (!(page instanceof LoginPage)) {
to(LogoutPage).logout()
}
} catch (Exception ignored) {
// Swallow cleanup errors to avoid masking the original test failure.
}
```
##########
grails-test-examples/scaffolding/src/integrationTest/groovy/com/example/pages/LogoutPage.groovy:
##########
@@ -16,19 +16,23 @@
* specific language governing permissions and limitations
* under the License.
*/
-
package com.example.pages
import geb.Page
class LogoutPage extends Page {
- static url = 'logout'
-
- static at = { title == 'Confirm Log Out?' }
+ static String pageTitle = 'Confirm Log Out?'
+ static url = 'logout'
+ static at = { title == pageTitle }
static content = {
logoutForm { $('form') }
- logoutButton { $('button', value: 'Log Out') }
+ logoutButton { $('button') }
+ }
+
+ void logout() {
+ logoutButton.click()
+ waitFor { title != pageTitle }
Review Comment:
`logoutButton { $('button') }` is very broad and can become ambiguous if the logout confirmation page ever contains additional buttons (layout/header, JS widgets, etc.). Since you already define `logoutForm`, consider scoping the selector to the form and/or matching on submit semantics (type/text) so the click is deterministic.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: stabilize flaky scaffolding Geb tests for slow CI environments [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #15394:
URL: https://github.com/apache/grails-core/pull/15394#issuecomment-3912259646
So I thought I had added waitFor and there was push back to instead set the global property that was added to change the default wait. I might be thinking of the spring security project instead. @matrei are you ok with this approach?
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: stabilize flaky scaffolding Geb tests for slow CI environments [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #15394:
URL: https://github.com/apache/grails-core/pull/15394#issuecomment-3915790737
Looks @matrei cleaned up this. I'm good to merge this if he is.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org