test: cover TimeSpan rendering and wide day components - #479
Open
NickJosevski wants to merge 1 commit into
Open
Conversation
#458 fixed FromTimeSpan but left the tests thinner than the parser it guards. ToTimeSpan was only exercised through the round trip, which cannot tell a formatting change from a matching parsing change, so the optional day and fractional-second components were unpinned in both directions. Assert the rendered strings directly, and add the input shapes the parser handles but nothing asserted: a day component wider than two digits, which the server does not pad, a day and a fraction together, and colon-separated field counts either side of three. Cases taken from #435 by @Scott-Emberson, whose parser fix #458 landed independently; these are the parts of that work still worth keeping. Co-authored-by: Scott Emberson <8268155+Scott-Emberson@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 4, 2026
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
1 participant
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.
Test-only. Salvages what is still useful from #435 / #452 after #458 fixed the underlying parser independently.
Why
#458 rewrote
FromTimeSpanin bothpkg/machinesandpkg/machinepoliciesand added tests, but those tests are thinner than the parser they guard:ToTimeSpanis only exercised through the round trip.assert.Equal(t, d, FromTimeSpan(ToTimeSpan(d)))passes just as happily if both sides change together, so the wire format — the part the server actually reads — is unpinned. The day and fractional-second components are only rendered when non-zero, and none of that conditional formatting is asserted anywhere."not-a-timespan"is covered;"00:00"and"00:00:00:00"are not, and those are the shapes thelen(parts) != 3guard exists for.What
Both copies of the test file get the same four additions, matching how the two packages are kept byte-identical:
TestToTimeSpanRendersDotNetFormat"37500.00:00:00""01.00:00:00.50000""00:00","00:00:00:00"No production code changes —
duration_formatter.gois untouched in both packages. All cases pass againstmainas it stands; they are regression protection, not a fix.Attribution
The cases come from #435 by @Scott-Emberson, preserved as a co-author on the commit. His diagnosis of the day-component bug (#434) was correct and is what #458 went on to fix by another route; this keeps the coverage his PR would have added. #434, #435 and #452 are being closed against #458.