From 5a82d60305af5cb825bf44afb7954293a2dc4d97 Mon Sep 17 00:00:00 2001
From: Jonathan Shook <jshook@gmail.com>
Date: Wed, 2 Sep 2026 20:19:36 +0000
Subject: [PATCH] fix(expr-api): render datetime seconds even on a whole minute
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The ISO forms of LocalTime and LocalDateTime omit the seconds field
when the second is 00, so currentTime(), currentDateTime(),
currentTimeInZone() and millisToDateTime() each returned a shorter
string once a minute — HH:mm instead of HH:mm:ss. That failed
testCurrentTime in CI roughly one run in sixty, and silently handed
workloads a differently-shaped timestamp the rest of the time.

Each now formats with an explicit pattern rather than toString(), which
also makes the truncatedTo(SECONDS) calls redundant.

The new test pins the boundary through millisToDateTime, whose instant
comes from the caller: the wall-clock functions can only be asserted
for shape, which catches this one second in sixty. It builds a
timestamp on a whole minute, requires the seconds to be there, and
repeats at second 30 so a fix in the other direction is caught too.
---
 .../main/resources/lib/groovy/datetime.groovy | 16 ++++++++----
 .../nb/api/expr/BundledLibrariesTest.java     | 26 +++++++++++++++++++
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/nb-apis/expr-api/src/main/resources/lib/groovy/datetime.groovy b/nb-apis/expr-api/src/main/resources/lib/groovy/datetime.groovy
index 842c033de..0eacfa987 100644
--- a/nb-apis/expr-api/src/main/resources/lib/groovy/datetime.groovy
+++ b/nb-apis/expr-api/src/main/resources/lib/groovy/datetime.groovy
@@ -76,7 +76,10 @@ def currentDate() {
 )
 @ExprExample(expectNotNull = true)
 def currentTime() {
-    LocalTime.now().truncatedTo(ChronoUnit.SECONDS).toString()
+    // Explicit pattern, not toString(): LocalTime's ISO form omits the
+    // seconds field entirely when the second is 00, so toString() emits
+    // HH:mm once a minute and HH:mm:ss the rest of the time.
+    LocalTime.now().format(DateTimeFormatter.ofPattern("HH:mm:ss"))
 }
 
 @ExprFunctionSpec(
@@ -85,7 +88,8 @@ def currentTime() {
 )
 @ExprExample(expectNotNull = true)
 def currentDateTime() {
-    LocalDateTime.now().truncatedTo(ChronoUnit.SECONDS).toString()
+    // See currentTime(): the ISO form drops seconds at second 00.
+    LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss"))
 }
 
 @ExprFunctionSpec(
@@ -266,11 +270,12 @@ def millisToDate(millis) {
 )
 @ExprExample(expectNotNull = true)
 def millisToDateTime(millis) {
+    // See currentTime(): the ISO form drops seconds at second 00, so a
+    // timestamp landing on a whole minute rendered as yyyy-MM-ddTHH:mm.
     Instant.ofEpochMilli(millis as long)
            .atZone(ZoneId.systemDefault())
            .toLocalDateTime()
-           .truncatedTo(ChronoUnit.SECONDS)
-           .toString()
+           .format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss"))
 }
 
 @ExprFunctionSpec(
@@ -300,5 +305,6 @@ def currentDateInZone(timezone) {
 )
 @ExprExample(expectNotNull = true)
 def currentTimeInZone(timezone) {
-    LocalTime.now(ZoneId.of(timezone)).truncatedTo(ChronoUnit.SECONDS).toString()
+    // See currentTime(): the ISO form drops seconds at second 00.
+    LocalTime.now(ZoneId.of(timezone)).format(DateTimeFormatter.ofPattern("HH:mm:ss"))
 }
diff --git a/nb-apis/expr-api/src/test/java/io/nosqlbench/nb/api/expr/BundledLibrariesTest.java b/nb-apis/expr-api/src/test/java/io/nosqlbench/nb/api/expr/BundledLibrariesTest.java
index fdffbcf03..fd65eb13a 100644
--- a/nb-apis/expr-api/src/test/java/io/nosqlbench/nb/api/expr/BundledLibrariesTest.java
+++ b/nb-apis/expr-api/src/test/java/io/nosqlbench/nb/api/expr/BundledLibrariesTest.java
@@ -19,6 +19,10 @@ package io.nosqlbench.nb.api.expr;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
 import org.junit.jupiter.api.Tag;
 
 import java.util.Map;
@@ -122,6 +126,28 @@ class BundledLibrariesTest {
         assertTrue(result.matches("\\d{2}:\\d{2}:\\d{2}"));
     }
 
+    /// The seconds field is optional in the ISO forms of `LocalTime` and
+    /// `LocalDateTime`: `toString()` omits it entirely when the second
+    /// is `00`, so a shape assertion against the wall clock only catches
+    /// the defect one second in sixty. `millisToDateTime` takes its
+    /// instant from the caller and shares the same rendering, so the
+    /// boundary can be pinned exactly rather than waited for.
+    @Test
+    void timesOnAWholeMinuteStillRenderTheirSeconds() {
+        long onTheMinute = ZonedDateTime.now(ZoneId.systemDefault())
+            .truncatedTo(ChronoUnit.MINUTES).toInstant().toEpochMilli();
+        String atSecondZero = processor.process("{{= millisToDateTime(" + onTheMinute + ") }}", null, Map.of());
+        assertTrue(atSecondZero.matches("\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}"),
+            "a timestamp on a whole minute must still render seconds, got: " + atSecondZero);
+        assertTrue(atSecondZero.endsWith(":00"), "and those seconds are 00, got: " + atSecondZero);
+
+        String atSecondThirty = processor.process(
+            "{{= millisToDateTime(" + (onTheMinute + 30_000) + ") }}", null, Map.of());
+        assertTrue(atSecondThirty.matches("\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}"),
+            "and a non-zero second renders the same width, got: " + atSecondThirty);
+        assertTrue(atSecondThirty.endsWith(":30"), atSecondThirty);
+    }
+
     @Test
     void testCurrentDateTime() {
         String result = processor.process("{{= currentDateTime() }}", null, Map.of());