NamedInstantPatternTest#compatibilityOfLegacyPattern fails in timezones with minute offsets (e.g., GMT+05:30)

[ramanathan1504]

Description

The compatibilityOfLegacyPattern test in NamedInstantPatternTest fails when the build is run in an environment with a timezone that has a non-zero minute offset, such as India Standard Time (GMT+05:30).

The test asserts that a legacy date formatter and a new formatter produce identical output. For the ISO8601_OFFSET_DATE_TIME_HH enum value, the legacy formatter produces a timezone offset with minutes (e.g., +0530), while the new formatter produces an offset with only hours (e.g., +05), causing the assertion to fail.

This was discovered while working on PR #3874.

Configuration

• Version: 2.26.0-SNAPSHOT
• Operating system: [macOS 26]
• JDK: [OpenJDK 17.0.5]
• Timezone: GMT+05:30 (e.g., Asia/Kolkata)

Logs

[INFO] Running org.apache.logging.log4j.core.pattern.NamedInstantPatternTest
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.123 s <<< FAILURE! - in org.apache.logging.log4j.core.pattern.NamedInstantPatternTest
[ERROR] compatibilityOfLegacyPattern(org.apache.logging.log4j.core.pattern.NamedInstantPattern)  Time elapsed: 0.1 s  <<< FAILURE!
org.opentest4j.AssertionFailedError:
[Compatibility failed for pattern: 'ISO8601_OFFSET_DATE_TIME_HH']
expected: "2025-08-17T12:10:08,887+0530"
 but was: "2025-08-17T12:10:08,887+05"
        at org.apache.logging.log4j.core.pattern.NamedInstantPatternTest.compatibilityOfLegacyPattern(NamedInstantPatternTest.java:45)

Reproduction

1. Set the local machine's system timezone to one with a 30 or 45-minute offset. For example, Asia/Kolkata (India Standard Time, GMT+05:30).
2. Check out the main branch of the logging-log4j2 repository.
3. Run the build with mvn clean install.
4. Observe the build failure in the log4j-core-test module, originating from NamedInstantPatternTest.

[vy]

[Compatibility failed for pattern: 'ISO8601_OFFSET_DATE_TIME_HH']
expected: "2025-08-17T12:10:08,887+0530"
 but was: "2025-08-17T12:10:08,887+05"

NamedInstantPattern.ISO8601_OFFSET_DATE_TIME_HH is defined as follows:

ISO8601_OFFSET_DATE_TIME_HH("yyyy-MM-dd'T'HH:mm:ss,SSSx", "yyyy-MM-dd'T'HH:mm:ss,SSSX")

Note the 2nd argument denotes the legacy pattern.
FixedDateFormat, delegated from InstantPatternLegacyFormatter, contains the following:

        /**
         * ISO8601 time format: {@code "yyyy-MM-dd'T'HH:mm:ss,SSSX"} with a time zone like {@code -07}.
         */
        ISO8601_OFFSET_DATE_TIME_HH(
                "yyyy-MM-dd'T'HH:mm:ss,SSSX", "yyyy-MM-dd'T'", 2, ':', 1, ',', 1, 3, FixedTimeZoneFormat.HH),

This certainly does not work, since FixedTimeZoneFormat.HH only encodes hours, whereas in our case, the time zone contains minutes component. Put another way, this is a bug in the legacy FixedDateFormat.

I'm not inclined to fix this – we deprecated legacy formatters for a reason.

@ppkarwasz, unless you've objections, I'd like to close this issue as Not planned.

[ramanathan1504]

@vy
Thanks for the clarification👍🏽. Understood that this is a bug in the legacy formatter and likely not planned since it’s deprecated.
If the decision changes and this needs a fix, I’d be happy to work on it.

[ppkarwasz]

@ppkarwasz, unless you've objections, I'd like to close this issue as Not planned.

This isn’t a bug in FixedDateFormat, because the SimpleDateFormat documentation explicitly uses Indian Standard Time as an example of the X pattern’s output:

For formatting, if the offset value from GMT is 0, "Z" is produced. If the number of pattern letters is 1, any fraction of an hour is ignored. For example, if the pattern is "X" and the time zone is "GMT+05:30", "+05" is produced.

However, I believe this behavior in SimpleDateFormat is quite poorly chosen. Ignoring fractional offsets can lead to misleading results, for instance, a 30-minute meeting could appear shifted by half an hour simply because of the formatter’s rounding. This is most likely why DateTimeFormatter diverged from this convention, and it’s also why we should avoid replicating the SimpleDateFormat behavior here.

Should we fix the test class to skip this case if the zone offset contains fractions of an hour?

if (namedPattern == NamedInstantPattern.ISO8601_OFFSET_DATE_TIME_HH) {
    ZoneOffset offset = ZoneId.systemDefault().getRules().getOffset(Instant.now());

    Assumptions.assumeThat(offset.getTotalSeconds() % 3600)
            .describedAs("Skipping test: ISO8601_OFFSET_DATE_TIME_HH requires a whole-hour offset, but system offset is %s", offset)
            .isEqualTo(0);
}

[vy]

Should we fix the test class to skip this case if the zone offset contains fractions of an hour?

I was reluctant to suggest this, but since you also thought of documenting this in the tests, maybe it is indeed a good idea. @ppkarwasz, shall we also duplicate the test suite such that host tz is configured with some known-to-be-problematic time zones? That is,

 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.time.Instant;
+import java.util.Arrays;
+import java.util.TimeZone;
+import java.util.stream.Stream;
 import org.apache.logging.log4j.core.time.MutableInstant;
 import org.apache.logging.log4j.core.util.internal.instant.InstantPatternFormatter;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.EnumSource;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 class NamedInstantPatternTest {
 
+    private static final TimeZone HOST_TZ = TimeZone.getDefault();
+
+    @AfterAll
+    static void restoreHostTz() {
+        TimeZone.setDefault(HOST_TZ);
+    }
+
+    static Stream<Arguments> timeZonesAndNamedInstantPatterns() {
+        return Stream.of(HOST_TZ.getID(), "IST", "America/Jujuy", "America/Santiago")
+                .distinct()
+                .flatMap(tz -> Arrays.stream(NamedInstantPattern.values())
+                        .map(namedInstantPattern -> Arguments.of(tz, namedInstantPattern)));
+    }
+
     @ParameterizedTest
-    @EnumSource(NamedInstantPattern.class)
-    void compatibilityOfLegacyPattern(NamedInstantPattern namedPattern) {
+    @MethodSource("timeZonesAndNamedInstantPatterns")
+    void compatibilityOfLegacyPattern(TimeZone timeZone, NamedInstantPattern namedPattern) {
+        TimeZone.setDefault(timeZone);
         InstantPatternFormatter legacyFormatter = InstantPatternFormatter.newBuilder()
                 .setPattern(namedPattern.getLegacyPattern())
                 .setLegacyFormattersEnabled(true)

[ppkarwasz]

shall we also duplicate the test suite such that host tz is configured with some known-to-be-problematic time zones?

I don't think it is necessary and changing the default timezone of the system is quite intrusive (other parallel test would fail if we run them in parallel).