Fix test_actual_config_files: read expectations from default.yml - #27
Merged
Conversation
The test ran the merger against the real config/ directory but asserted hardcoded San Francisco coordinates, duplicating data it did not own. 64ff0c9 (2026-03-23) "Replace site-specific defaults with generic placeholders" deliberately changed those values to Greenwich, and the test has failed ever since — unnoticed, because this repo's suite has never run in CI. The test's purpose, per its own docstring, is that the merger propagates the real config into tar1090.env; the particular coordinates are incidental. It now reads them from default.yml. Verified in both directions: changing default.yml's location no longer breaks the test, while breaking the merger's propagation still does. The sibling test_tar1090_env_uses_location_rx keeps its literal values — it writes its own fixture, so those are its data rather than a claim about the repo's config.
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.
Part of 86cb4jfk7. This repo's suite has never run in CI; running it gives 18 passed, 1 failed.
Root cause
test_actual_config_filesruns the merger against the realconfig/directory, then asserted hardcoded San Francisco coordinates:64ff0c9(2026-03-23, "Replace site-specific defaults with generic placeholders") deliberately changed those to Greenwich. The config change was correct; the test duplicated data it did not own and was never updated. It has failed for five months without anyone knowing.The fix
The test's purpose, per its own docstring, is "merge with actual config files from retina-node repo" — that the merger propagates the real config into
tar1090.env. The particular coordinates are incidental, so it now reads them fromdefault.yml.Its sibling
test_tar1090_env_uses_location_rxkeeps its literal37.7644— that test writes its owndefault.yml, so those are its fixture data rather than a claim about the repo's config. That difference is exactly why one broke and one did not.Verification, both directions
Updating the numbers would have made the test pass while leaving it just as brittle, so the fix was checked against both failure modes:
default.yml's location to arbitrary values → test still passes (it no longer duplicates config data)Suite: 19 passed, 0 failed.
🤖 Generated with Claude Code
https://claude.ai/code/session_013ZcazeseXVpu8XrYA2tE1V