Run the config-merger test suite in CI - #29
Merged
Merged
Conversation
19 tests have existed in this repo and never run in CI. That is how test_actual_config_files came to assert San Francisco coordinates for five months after 64ff0c9 deliberately replaced them with generic placeholders. The repo has no Python packaging, so the config-merger's dependencies are named directly in the workflow. They mirror config-merger/Dockerfile, which installs the same two unpinned — those two lines need to stay in step. Tests live under config-merger/test/ rather than tests/, so the path is explicit. Verified that a wrong path exits non-zero, so a broken step fails rather than silently collecting nothing.
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.
Second half of 86cb4jfk7. #27 fixed the failing test; this makes the suite load-bearing.
Why
19 tests have existed here and never run in CI. That is precisely how
test_actual_config_filescame to assert San Francisco coordinates for five months after64ff0c9deliberately replaced the site-specific defaults with generic placeholders. The config change was correct; nothing was watching the test that disagreed.The change
Two things worth noting for review:
Dependencies are named directly. This repo has no
requirements.txtorpyproject.toml, so there is nowhere else to put them. They mirrorconfig-merger/Dockerfile, which installspyyaml mergedeepunpinned — those two lines now need to stay in step, and the workflow comment says so.The test path is explicit, because tests live under
config-merger/test/rather thantests/.Verification
Ran the exact CI commands in a clean venv from a pristine checkout: 19 passed, exit 0.
Also checked the failure mode that would matter most here:
pytestagainst a path that does not exist exits 4, not 0. A mistyped path fails the step rather than collecting nothing and reporting success — which, given this repo just spent five months not noticing a broken test, seemed worth confirming rather than assuming.Adds about 2 seconds to the job.
🤖 Generated with Claude Code
https://claude.ai/code/session_013ZcazeseXVpu8XrYA2tE1V