Skip to content

Correct fractional-hour offset conversion and display - #52

Open
OskarEichler wants to merge 5 commits into
floraison:masterfrom
OskarEichler:codex/fix-fractional-hour-offsets
Open

Correct fractional-hour offset conversion and display#52
OskarEichler wants to merge 5 commits into
floraison:masterfrom
OskarEichler:codex/fix-fractional-hour-offsets

Conversation

@OskarEichler

Copy link
Copy Markdown

Summary

Correct three offset arithmetic/sign errors affecting non-whole-hour timezones:

  • Convert leftover seconds to minutes when rendering a numeric offset.
  • Preserve the negative sign for a zero-hour offset such as -00:30.
  • Display minutes, rather than remaining seconds, in EoTime#to_debug_s.

Reproduction

EtOrbi.get_tzone(19800) currently returns nil because the internal formatter produces "+05:1800"; it now resolves +05:30 with an offset of 19800 seconds. Both 5:30 and 5:45, positive/negative half-hour values, and zero were checked.

EtOrbi.get_tzone("-00:30") currently has a positive 1800-second offset; it now has -1800. Compact "-0030" was checked too.

EtOrbi::EoTime.new(0, "Asia/Kolkata").to_debug_s now contains "+05:30", not "+05:1800".

Verification

  • Ruby 4.0.6 via rbenv; existing Probatio suite, no test files added or modified.
  • Reviewed master baseline and this individual patch each run 270 tests / 412 assertions with exactly the same three pre-existing DST/rday failures from rday / rweek calculation is odd in time zones with DST #51. Failure identities were compared, not just counts.
  • A release-based 1.4.1 branch containing the five separately proposed fixes passes 263 existing tests / 405 assertions with TZInfo 2.0.6 in UTC, Asia/Tokyo, America/Chicago, Europe/Istanbul and the detected local zone.
  • Temporary focused before/after reproductions pass with TZInfo 2.0.6 and 1.2.11. The combined release branch also passes the existing TZInfo 1.2.11 suite at seed 36231. Seed 99751 has the same pre-existing Zulu test-order failure on both unmodified release and patched release: a test mutates the cached TZInfo object's comparison method.
  • Ruby syntax and gem packaging checks pass. Targeted Lint passes on the combined branch with the existing, unrelated AssignmentInCondition warning excluded; no lint configuration was changed.

The current master's unreleased day-delta change was not brought into the consumer branch. No live scheduler or production operations were used. Linux/Windows and other Ruby runtimes were not run locally.

Breaking changes and limitations

No intended API break; malformed/reversed results become the correct offset. Existing whole-hour behavior is preserved. This does not expand the accepted hour range or add second-resolution numeric timezone support. Checked current source, open PRs and offset-related issues before publication.

@jmettraux

jmettraux commented Aug 27, 2026 via email

Copy link
Copy Markdown
Member

@jmettraux

Copy link
Copy Markdown
Member

What is this test wrecking crap?

@jmettraux

Copy link
Copy Markdown
Member

Use assert instead of assert_equal. Do not remove the final blank line.

@OskarEichler

Copy link
Copy Markdown
Author

Updated the regression to use the project assert style and restored the final blank line. The focused module suite passes with 156 tests and 232 assertions.

@jmettraux

Copy link
Copy Markdown
Member

Enumerate the rules in the project assert style in a comment in this pull request.

@OskarEichler
OskarEichler force-pushed the codex/fix-fractional-hour-offsets branch from 51a48eb to c72d3de Compare August 30, 2026 16:26
@OskarEichler

Copy link
Copy Markdown
Author

The project assertion rules I will follow are:

  1. Use Probatio's assert, not Minitest's assert_equal.
  2. With one ordinary argument, assert checks truthiness.
  3. With one hash, it checks each key/value pair for equality.
  4. With one regexp and one or more strings, it checks that the strings match.
  5. Otherwise, it checks that all arguments are equal; in this repository the computed value is written first and the expected value second.
  6. Preserve the surrounding test layout, including the blank line after a test opening and the final blank line in the file.

The current regression uses the two-argument equality form throughout, and c72d3de restores the final blank line. The focused module suite passes: 156 tests, 232 assertions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants