Stop GeocoderTest calling Google - #904
Merged
Merged
Conversation
testGeocode built a Geocoder directly and geocoded a real address against the live Maps API. When that call failed - network, key or quota - the helper returned false and the test errored with "Trying to access array offset on value of type bool", as it did on the #900 run. Nothing was wrong with the branch: a unit test simply depended on a third party being reachable. Move the HTTP call behind a protected fetch(), the same idea as the existing googleKey() seam, so the parsing can be tested with a canned response. The assertions on the parsed latitude, longitude and country code are unchanged. Also covers the paths that had no test: no results, a failed request (the one that produced the error above), and the ForceGeocodeFailure short circuit. GeocoderMock overrides geocode() outright, so tests resolving the geocoder from the container are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
edwh
added a commit
that referenced
this pull request
Sep 5, 2026
develop gained #904 (a fetch() seam so GeocoderTest could exercise the parsing without calling Google) while this branch was replacing the whole method with the geocoder package. Same method, incompatible approaches. Taking this branch's: with the provider chain resolved from the container there is nothing to seam - a test binds its own provider instead, which is what the code comment here already promised. That also drops googleKey(), and reverseGeocode(), which was defined and never called anywhere in the codebase. GeocoderTest is rewritten onto that mechanism: it builds real Geocoder\Model\Address objects and binds a provider returning them. Both container keys have to be replaced, not just one. geocoder-laravel binds the string 'geocoder' through to ProviderAndDumperAggregator and registers that as a singleton, so replacing only the string leaves the real Mapbox chain reachable - and a test that passes then is quietly talking to the network. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
tests/Unit/GeocoderTest.phpbuilt aGeocoderdirectly and geocoded a real address against the live Google Maps API. When that call fails — network, key, or quota —geocode()returnsfalseand the test errors:That is what failed CI on #900 (
feat/ords-repairs-export), which has nothing to do with geocoding. PHPUnit passed ondevelop's later run, so it comes and goes with the API — a unit test depending on a third party being reachable.Change
Move the HTTP call behind a
protected fetch(), the same idea as the existinggoogleKey()seam ("We have this so that we can change the key in testing"), and have the test subclass it with a canned response. The assertions on parsed latitude, longitude and country code are unchanged, so what the test was actually checking — that a Google response is parsed correctly — is still checked.Three paths that had no coverage are now tested: no results, a failed request (exactly the case that produced the error above), and the
ForceGeocodeFailureshort circuit.GeocoderMockoverridesgeocode()outright, so tests resolving the geocoder from the container are unaffected —GroupEditGeocodeFallbackTeststill passes.Verification