fix(assert): handle saturated time differences in WithinDuration - #1978
Open
sergioperezcheco wants to merge 1 commit into
Open
sergioperezcheco wants to merge 1 commit into
sergioperezcheco wants to merge 1 commit into
Conversation
Co-Authored-By: GPT-6.1-Sol <noreply@openai.com> Signed-off-by: sergioperezcheco <checo520@outlook.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix
WithinDurationaccepting time differences larger than the maximum representable duration.Changes
Compare the actual time against
expected.Add(-delta)andexpected.Add(delta)instead of using the saturated result oftime.Time.Subfor the decision. Negative tolerances are rejected before negation, including the minimum duration value. The existing failure message and public wrappers remain unchanged.Motivation
For example, two dates 400 years apart currently pass with a tolerance of
time.Duration(1<<63 - 1), although that tolerance is approximately 292 years. Regression tests cover both directions, inclusive boundaries, negative tolerances, clock/location differences, and the assert/require wrappers.The regression tests fail on the original implementation and pass with this change. Locally,
go test -race -count=1 ./...,go vet ./..., formatting, and generation checks pass on Go 1.26.4/macOS arm64. The older-Go and Linux CI matrix has not been run locally.Related issues
No existing issue found for this saturation case.
Implemented with assistance from Hermes Agent.