Stabilize recurring CI flaky tests in functional/example apps

[jamesfredley]

The Problem

CI runs of gradle.yml and groovy-joint-workflow.yml had recurring flaky failures. Mining ~100 recent failures surfaced a handful of environment/timing-sensitive tests in the functional/example apps that fail intermittently and erode confidence in the pipeline.

All changes here are confined to functional/example test apps under grails-test-examples/. No framework runtime behavior is altered.

The Fix

Root-cause synchronization hardening (via the Page Object convention already used elsewhere) where feasible, plus the project's existing Spock @Retry convention as a backstop only where a root-cause fix was not available.

Test	Symptom	Fix
Scaffolding UserControllerSpec / UserCommunityControllerSpec "User list"	WaitTimeoutException - navigation lands on the Spring Security login page because the session wasn't authenticated yet	Harden LoginPage.login() / LogoutPage.logout() to wait for a definitive signal (login form gone / present) instead of a transient title change
AsyncPromiseSpec "async task handles success without error" (app1 + hibernate7/app1)	Intermittent HTTP 500 instead of 200 (async timing); the JDK HttpClient doesn't throw on non-2xx, so a transient 500 surfaces as an assertJson(200, ...) failure	@Retry(count = 2, delay = 500, exceptions = [AssertionError]) on the method
GreetingControllerFunctionalSpec "renders response" (test-phases)	Geb timing - pageSource.contains('Hello') evaluated before render finished	Introduce GreetingPage and move the render-timing wait into its at check (Page Object pattern), then switch the spec to to(GreetingPage)

Note: the @Retry backstops originally added to the scaffolding and Greeting specs were later removed once the Page Object / definitive-signal fixes addressed the root cause - retries mask real regressions, so they were dropped where a genuine fix exists (verified green under Docker).

Investigated, intentionally not changed

• DirtyCheckingAfterListenerSpec stale @PendingFeatureIf - already resolved on 8.0.x by 05368b662d.
• SaveWithInvalidEntitySpec - Hibernate 5 spec already carries the root-cause fix from 05368b662d; the Hibernate 7 mirror is intentionally @Ignored (documented NPE, #10604), so neither can be the CI failure.
• JsonViewTemplateResolverSpec - no shared mutable state and fresh instances per feature; a fast unit test, so @Retry would risk masking real regressions. Left unchanged.

Verification note

These are ContainerGebSpec / functional / integration tests that require Docker and a full app boot and are intermittent by nature, so confirmation is CI-bound - a single green local run can't prove a flake is fixed. Changes were kept conservative and lean on the established @Retry convention plus Page Object root-cause fixes.

[Copilot]

Pull request overview

This PR stabilizes recurring CI flakes in grails-test-examples/ functional/integration test apps by adding narrowly-scoped Spock @Retry backstops and hardening Geb synchronization so navigation waits on definitive logged-in/logged-out signals rather than transient page-title changes.

Changes:

• Add @Retry backstops to several functional/integration specs to reduce intermittent CI failures.
• Harden scaffolding example app’s login/logout page objects to wait for deterministic authentication state changes (login form present/absent).
• Replace an eager pageSource.contains(...) assertion with a waitFor { ... } synchronization in the test-phases example.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
grails-test-examples/test-phases/src/functional-test/groovy/testphases/GreetingControllerFunctionalSpec.groovy	Adds waitFor for rendered content plus a WaitTimeoutException-scoped @Retry backstop.
grails-test-examples/scaffolding/src/integrationTest/groovy/com/example/UserControllerSpec.groovy	Adds class-level @Retry for WaitTimeoutException to mitigate Geb timing flakes.
grails-test-examples/scaffolding/src/integrationTest/groovy/com/example/UserCommunityControllerSpec.groovy	Adds class-level @Retry for WaitTimeoutException to mitigate Geb timing flakes.
grails-test-examples/scaffolding/src/integrationTest/groovy/com/example/pages/LogoutPage.groovy	Makes logout wait for a definitive logged-out signal (login form present) instead of only title change.
grails-test-examples/scaffolding/src/integrationTest/groovy/com/example/pages/LoginPage.groovy	Makes login wait for a definitive authenticated signal (login form gone) instead of only title change.
grails-test-examples/hibernate7/app1/src/integration-test/groovy/functionaltests/async/AsyncPromiseSpec.groovy	Adds method-level @Retry to reduce intermittent async HTTP assertion flake impact.
grails-test-examples/app1/src/integration-test/groovy/functionaltests/async/AsyncPromiseSpec.groovy	Adds method-level @Retry to reduce intermittent async HTTP assertion flake impact.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[borinquenkid]

Focused LGTM

[jdaugherty]

We should not be fixing tests with retry. They burn cpu cycles instead of fixing the underlying issue.

[borinquenkid]

Stabilizes recurring CI flaky tests identified by mining ~100 recent failures of the gradle.yml and groovy-joint-workflow.yml workflows. Uses the project's existing Spock @Retry convention (already used in GitHubCreateControllerSpec and MicronautErsatzRoundtripSpec) plus root-cause synchronization hardening where feasible.

All changes are confined to functional/example test apps under grails-test-examples/; no framework runtime behavior is altered.

@jdaugherty Fredley did address the deterministic issues from the non-deterministic.

[jdaugherty]

Stabilizes recurring CI flaky tests identified by mining ~100 recent failures of the gradle.yml and groovy-joint-workflow.yml workflows. Uses the project's existing Spock @Retry convention (already used in GitHubCreateControllerSpec and MicronautErsatzRoundtripSpec) plus root-cause synchronization hardening where feasible.
All changes are confined to functional/example test apps under grails-test-examples/; no framework runtime behavior is altered.

@jdaugherty Fredley did address the deterministic issues from the non-deterministic.

@borinquenkid I'm ok with the wait's being added, I am not ok with the retry. We need to address the tests to make them non-flaky by adding waits. There are still @Retry changes in this PR.

[jamesfredley]

@jdaugherty even the smartest combination of AI models could not find a fix for these flaky tests, if you have one please commit it. Or maybe we remove them.

[jdaugherty]

We just need to turn on recording and run locally to find the cause. It's likely a wait or slow performance issue. @jamesfredley the tests do test functionality and every time we've added retries, we've just caused our runs to be longer. We can open a ticket as future work or mute the tests in the mean time and they won't be a problem on the PRs.

[borinquenkid]

We just need to turn on recording and run locally to find the cause. It's likely a wait or slow performance issue. @jamesfredley the tests do test functionality and every time we've added retries, we've just caused our runs to be longer. We can open a ticket as future work or mute the tests in the mean time and they won't be a problem on the PRs.

lets @pending and add an ISSUE. thanks @jdaugherty

[jdaugherty]

@borinquenkid I disagree with pending since they're not always failing.

[borinquenkid]

@borinquenkid I disagree with pending since they're not always failing.

You are correct
@Ignore("Muted due to Geb timing flake. See ISSUE #10604")

[jdaugherty]

@borinquenkid I disagree with pending since they're not always failing.

You are correct @Ignore("Muted due to Geb timing flake. See ISSUE #10604")

I get that we want to take the quick solution here and I understand that AI hasn't been able to figure this out, but AI can't figure out a lot of things. This is one of those issues that's a true technical debt and not good for the project long term. We need to fix these issues. If someone wants to dig into these issues and fix them, that's great. But I am a hard -1 on masking these problems. I've been burned in the past by this on too many projects and it leads to a longer running test run with more problems longer term.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.5462%. Comparing base (b00e83e) to head (0bed2fb).
⚠️ Report is 82 commits behind head on 8.0.x.

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.0.x     #15793        +/-   ##
==================================================
+ Coverage     49.5403%   49.5462%   +0.0059%     
- Complexity      16922      16923         +1     
==================================================
  Files            1999       1999                
  Lines           93754      93753         -1     
  Branches        16420      16419         -1     
==================================================
+ Hits            46446      46451         +5     
+ Misses          40145      40138         -7     
- Partials         7163       7164         +1     

see 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[borinquenkid]

We added PageObject to the test @jdaugherty

[jamesfredley]

https://github.com/apache/grails-core/actions/runs/28542759715/job/84626412824 best example of the need for these flaky tests to be fixed, since one test failed the snapshot is not published.

[jamesfredley]

@jdaugherty I would say getting past these is a high priority. I have been trying to get a snapshot published with the Groovy 5 changes since yesterday on 8.0.x and these or others keep getting in the way. There was also the need to extend the time to publish to nexus, but these flaky tests are failing most snapshot publishes.

[jdaugherty]

@jamesfredley can you show me where these runs failed? The only failure I see is with https://develocity.apache.org/s/si4mliaeifl7e and it looks like a valid failure to me.

[jdaugherty]

I believe all of the waitFor changes have been merged independently of this except GreetingControllerFunctionalSpec.

[jdaugherty]

let's replace waitFor(30) with waitFor

[jdaugherty]

We need to remove the hard coded 30, we set these on the test run and increase them as necessary

[borinquenkid]

Agreed on dropping the hardcoded 30. One option, in case it's useful: rather than falling back to a single global timeout, make the escalation opt-in and scoped to just this call site, so it doesn't change the default for every other test using this Page Object, but still gives the flaky path more patience without re-running the interaction the way @Retry would:

void login(String username, String password, List<Number> waitTimeouts = [5]) {
    usernameField = username
    passwordField = password
    loginButton.click()

    waitForEscalating(waitTimeouts) {
        title != pageTitle && $('input', name: 'username').empty
    }
}

private <T> T waitForEscalating(List<Number> timeouts, Closure<T> condition) {
    def remaining = timeouts.iterator()
    while (true) {
        Number t = remaining.next()
        try {
            return waitFor(t, condition)
        } catch (geb.waiting.WaitTimeoutException e) {
            if (!remaining.hasNext()) throw e
        }
    }
}

Default callers get plain waitFor(5) (matches grails.geb.timeouts.timeout's default). Only the flaky spec would opt in with login(username, password, [5, 15, 45]). Same idea would apply to LogoutPage.groovy. Just floating this as an option — happy to go with plain waitFor() too if that's preferred.

[jamesfredley]

Done - removed the hard-coded 30 so it now uses the configured grails.geb.timeouts.timeout (set to 30 in gradle/functional-test-config.gradle and increasable per test run). Pushed in 21670d3.

[jdaugherty]

We need to remove the hard coded 30, we set these on the test run and increase them as necessary

[jamesfredley]

Done - same fix here, dropped the hard-coded 30 so logout() uses the configured grails.geb.timeouts.timeout. Pushed in 21670d3.