test: cover the twilight API with astral-generated fixture data - #84
Open
ceeK wants to merge 1 commit into
Open
Conversation
The six twilight properties -- civil, nautical and astronomical sunrise and sunset -- had no coverage at all; only official sunrise/sunset were tested, leaving three quarters of the public date API unverified. Adds the six columns to all 237 cities in CorrectResults.json, generated from the astral package (an independent NOAA-based implementation) rather than from Solar itself, so the tests verify the zeniths are astronomically correct and not merely self-consistent. Regenerating the existing sunrise/sunset columns with astral reproduced the committed values to within 173s (median 29s), validating the source before it was trusted for the new columns. No pre-existing field value changed. Twilight events are stored on the local solar day, matching the fixture's existing convention, by selecting the occurrence nearest local mean noon derived from longitude alone -- rather than copying Solar's own +/-1 day heuristic, which would have baked that heuristic into the fixture meant to test it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PYPBCzQiXBvadC3y6Ae6FH
ceeK
force-pushed
the
chowell/fix-fixture-city-name-encoding
branch
from
August 31, 2026 15:50
d6fc2a2 to
2efa319
Compare
ceeK
marked this pull request as ready for review
August 31, 2026 15:53
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
The six twilight properties —
civilSunrise/Sunset,nauticalSunrise/Sunset,astronomicalSunrise/Sunset— had no coverage; only official sunrise/sunset were tested, leaving three quarters of the public date API unverified.This adds the six columns to all 237 cities in
CorrectResults.jsonand six parameterized tests mirroring the existing ones, at the same 5-minute accuracy.Where the expected values come from
Generated with the
astralpackage — an independent NOAA-based implementation — never from Solar itself, so the tests verify the zenith constants are astronomically right rather than merely self-consistent.The source was validated before being trusted: regenerating the existing
sunrise/sunsetcolumns with astral reproduced the committed values to a max of 173s (median 29s), comfortably inside the 5-minute tolerance.Two things worth a reviewer's attention
The fixture stores events on the local solar day, not the UTC day. For far-eastern/western cities (Wellington, Mexico City) the committed timestamps sit on the neighbouring UTC date, matching Solar's
shouldBeYesterday/shouldBeTomorrowshift. I reproduced this by picking the occurrence nearest local mean noon, derived from longitude alone, rather than copying Solar's own heuristic — otherwise a bug in that heuristic would be baked into the fixture meant to test it.No nulls were needed. All 237 capitals get all three twilights on 2017-02-09, so
Cityuses non-optionalDates consistent with the existing fields. Adding a polar city later would need optionals; it currentlyfatalErrors with a message naming the missing key.Verification
swift test: 20 tests pass, including 1,422 new city assertions — no extra tolerance needed, even at Nuuk and Reykjavík.🤖 Generated with Claude Code