Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use sling http client and add improved test assertions #10

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

robarchibald
Copy link
Contributor

@robarchibald robarchibald commented Oct 11, 2021

This one is a bit more extensive, but hopefully you'll find it useful as well. This PR starts with the github.com/dghubble/sling package. It's a pretty simple HTTP client package that makes it easy to build an API SDK like this one without having to roll your own api.go with all the methods you created there. I find it super useful and I love the reduction in lines of code in the overall SDK package. With the addition of sling, this package can just focus on getting input in the right format and providing the right structs for deserializing the JSON we got back from geocodio.

To make sure all the output structs were correct, I used github.com/mypricehealth/jsonassert to verify all the JSON can be unmarshalled and marshalled back to JSON correctly. In doing so, I found a few places where the prior structs weren't setup correctly so I updated accordingly;

  1. The HouseholdIncome struct was using JSON tags on the fields, but they all contained commas in them. Unfortunately, commas in JSON tags don't work in Go (https://stackoverflow.com/a/25984685) due to the way tags are implemented. I reverted this back to a map[string]CensusDataPoint to get it working
  2. CongressionalDistrict.Proportion had the wrong tag
  3. Removed deprecated CongressionalDistrict
  4. Added canada.go to enable the new riding and statscan features from geocodio
  5. Fixed spelling on Zip.RecordType and SchoolDistrict.Secondary

Finally, I did a few other things to clean things up.

  1. Moved address.go and env.go, location.go, and fields.go into geocodio.go since they were such small files I didn't feel like they needed their own files.
  2. Added the ability to geocode using either the single string like was already supported, or using the new InputAddress struct which offers address, city, state, zip fields separately. I enabled this capability for both single addresses and for batches.
  3. Removed the deprecated EnvOldAPIKey
  4. Introduced github.com/stretchr/testify/assert to reduce the number of lines doing test asserts.

@gf3
Copy link

gf3 commented Oct 21, 2021

@robarchibald will this allow users to swap out the HTTP client for testing purposes? i would love to avoid making $$$ API calls while running my test suite

@robarchibald
Copy link
Contributor Author

@robarchibald will this allow users to swap out the HTTP client for testing purposes? i would love to avoid making $$$ API calls while running my test suite

Yes. Sling allows you to mock the API calls using an HTTP Doer interface instead of a real HTTP client. I've done this with other API's I've built where I made a file-based HTTP Doer and then tested the code using example JSON files.

@gf3
Copy link

gf3 commented Oct 22, 2021

@robarchibald that's fantastic news!

@stevepartridge this feature would be huge for us wrt to testing, any chance on this getting merged and released?

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