fix(grails-gsp): eliminate race condition in FormTagLib2Tests.testDatePickerTag

[jamesfredley]

Summary

Eliminates the race condition in FormTagLib2Tests.testDatePickerTag that produced an intermittent testDatePickerTagWithMinutePrecision() failure on slow runners (most recently on Build Grails-Core (windows-latest, 25) in PR #15467 / run 25170545020).

What was wrong

testDatePickerTag(Object date, String precision) rendered the date-picker HTML first, then constructed a fresh GregorianCalendar afterwards to derive the expected select-field values. When the two calls fell on opposite sides of a minute (or hour/day/year) boundary, the picker output and the calendar disagreed, producing a flaky assertion such as:

FormTagLib2Tests > testDatePickerTagWithMinutePrecision()
expected: <true> but was: <false>
  at org.grails.web.taglib.AbstractGrailsTagTests.assertXPathExists(AbstractGrailsTagTests.groovy:527)
  at FormTagLib2Tests.assertSelectFieldPresentWithSelectedValue(FormTagLib2Tests.groovy:307)
  at FormTagLib2Tests.validateSelectedMinuteValue(FormTagLib2Tests.groovy:297)
  at FormTagLib2Tests.testDatePickerTag(FormTagLib2Tests.groovy:237)

Fix

Capture the calendar up-front and, when the test passes a null date, forward calendar.getTime() to the picker so both sides agree on a single instant. Behaviour for explicit Date arguments is unchanged. getDatePickerOutput simply forwards the supplied value to the datePicker tag, which itself defaults to new Date() when no value is provided, so passing the captured calendar.getTime() exercises the same code path as null did before but without the race window.

Verification

./gradlew :grails-gsp:test --tests org.grails.web.taglib.FormTagLib2Tests --rerun-tasks

All 11 tests pass on Windows (the runner that exhibited the original flake), including the previously flaky testDatePickerTagWithMinutePrecision().

[Copilot]

Pull request overview

Fixes a flaky FormTagLib2Tests.testDatePickerTagWithMinutePrecision() by ensuring the expected calendar values and the rendered datePicker output are derived from the same instant, avoiding minute/hour/day boundary races on slower CI runners.

Changes:

• Capture a single Calendar “now” upfront in testDatePickerTag(...) to prevent time drift between rendering and assertions.
• When the test passes null as the date, compute a resolved date from the captured calendar and render/assert against that same instant.

---

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

[Copilot]

When date == null, the test now passes an explicit value to datePicker (resolvedDate = calendar.getTime()), which changes the exercised code path: datePicker no longer runs its !value -> value = xdefault defaulting logic. To keep the original behavior ("no value provided" defaults to now) while still removing the race, consider leaving value unset and instead passing the captured instant via the tag's default attribute (i.e., call getDatePickerOutput(null, precision, calendar.getTime()) when date is null, and use calendar for assertions).

[bito-code-review]

The suggestion modifies the test to pass null as the date value when date is null, and uses the captured calendar time as the default parameter in getDatePickerOutput. This preserves the original defaulting logic inside the datePicker tag (treating null as 'no value provided' that defaults to now) while eliminating the race condition by using a single captured instant for both the tag output and assertions. Applying it improves the test's reliability without altering the intended behavior.

grails-gsp/plugin/src/test/groovy/org/grails/web/taglib/FormTagLib2Tests.groovy

Calendar calendar = new GregorianCalendar()
        Object resolvedDate = date
        if (date == null) {
            resolvedDate = calendar.getTime()
        } else if (date instanceof Date) {
            calendar.setTime(date)
        } /*else if (date instanceof TemporalAccessor) {
            ZonedDateTime zonedDateTime
            if (date instanceof LocalDateTime) {
                zonedDateTime = ZonedDateTime.of(date, ZoneId.systemDefault())
            } else if (date instanceof LocalDate) {
                zonedDateTime = ZonedDateTime.of(date, LocalTime.MIN, ZoneId.systemDefault())
            } else {
                zonedDateTime = ZonedDateTime.from(date)
            }
            calendar = GregorianCalendar.from(zonedDateTime)
        }*/

        Document document = getDatePickerOutput(resolvedDate, precision, null)

grails-gsp/plugin/src/test/groovy/org/grails/web/taglib/FormTagLib2Tests.groovy

Calendar calendar = new GregorianCalendar()
        Document document
        if (date == null) {
            document = getDatePickerOutput(null, precision, calendar.getTime())
        } else {
            Object resolvedDate = date
            if (date instanceof Date) {
                calendar.setTime(date)
            } /*else if (date instanceof TemporalAccessor) {
                ZonedDateTime zonedDateTime
                if (date instanceof LocalDateTime) {
                    zonedDateTime = ZonedDateTime.of(date, ZoneId.systemDefault())
                } else if (date instanceof LocalDate) {
                    zonedDateTime = ZonedDateTime.of(date, LocalTime.MIN, ZoneId.systemDefault())
                } else {
                    zonedDateTime = ZonedDateTime.from(date)
                }
                calendar = GregorianCalendar.from(zonedDateTime)
            }*/
            document = getDatePickerOutput(resolvedDate, precision, null)
        }

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 61b7d3c
▶️ Tests: 34174 executed
⚪️ Checks: 39/39 completed

---

Learn more about TestLens at testlens.app.

[jdaugherty]

Lets shorten then comment

[jdaugherty]

Shorten this comment, its just captured upfront to prevent test pillution