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

Stubbed results do not maintain consistency between coordinates and lat, long #1258

Closed
derekprior opened this issue Jan 17, 2018 · 5 comments
Milestone

Comments

@derekprior
Copy link

The README contains the following code for stubbing Geocoder results:

Geocoder.configure(:lookup => :test)

Geocoder::Lookup::Test.add_stub(
  "New York, NY", [
    {
      'coordinates'  => [40.7143528, -74.0059731],
      'address'      => 'New York, NY, USA',
      'state'        => 'New York',
      'state_code'   => 'NY',
      'country'      => 'United States',
      'country_code' => 'US'
    }
  ]
)

Expected behavior

When using the stub above for a test that attempts to Geocode an ActiveRecord model via the geocode method should succeed and set latitude and longitude on the record appropriately.

Actual behavior

Latitude and longitude remain nil despite coordinates being defined in the stub.

The geocode method relies on calling latitude and longitude on the result. Geocorder::Result::Base defines latitude and longitude in terms of coordinates, but the test result is essentially a hash with no logic to synchronize lat and long with coordinates.

It seems I have to set coordinates, latitude and longitude in stubs to cover all bases. In my own test helpers for stubbing geocoder responses, I've added the following to compensate for this.

  def sync_lat_long_and_coordinates(options)
    if options[:coordinates].present?
      options[:latitude] = options[:coordinates].first
      options[:longitude] = options[:coordinates].last
    elsif options[:latitude].present? && options[:longitude].present?
      options[:coordinates] = [options[:latitude], options[:longitude]]
    end

    options
  end

Environment info

  • Geocoder version: 1.4.4
  • Rails version: 5.1
@jheiss
Copy link

jheiss commented Jan 19, 2018

Looks like this is related to #1197

@alexreisner
Copy link
Owner

alexreisner commented Jan 19, 2018

Thanks @derekprior and @jheiss. The intention of the Test lookup/result is to be very generic, and to do very little (since all APIs return different data, it's meant to be very flexible). That being said, ALL results return lat/lon based on the coordinates array, so I think there's a good case to be made for implementing latitude and longitude methods in the Test lookup which will keep things in sync. As I think is clear from my odd response on #1197, I thought it was already like this (oops!).

I'm marking this as a bug, because it conflicts with what the README says. However, it potentially breaks tests, so it warrants a minor release.

@sergiopantoja
Copy link

Another temporary workaround 👍

Geocoder::Lookup::Test.add_stub(
  "New York, NY", [
    {
      'coordinates'  => [40.7143528, -74.0059731],
      'address'      => 'New York, NY, USA',
      'state'        => 'New York',
      'state_code'   => 'NY',
      'country'      => 'United States',
      'country_code' => 'US',
      'latitude'     => 40.7143528,
      'longitude'    => -74.0059731
    }
  ]
)

@CaioBianchi
Copy link

Having the same issue here. Coordinates remain nil no matter what I pass in.

@alexreisner alexreisner added this to the v1.5.0 milestone Apr 28, 2018
alexreisner added a commit that referenced this issue Jun 15, 2018

Verified

This commit was signed with the committer’s verified signature.
MrAlias Tyler Yahn
viz: add `coordinates` attribute and remove `latitude` and `longitude`.

Fixes #1258 and #1302.
@alexreisner
Copy link
Owner

Fixed in v1.5 branch: b284f71

alexreisner added a commit that referenced this issue Jun 15, 2018

Verified

This commit was signed with the committer’s verified signature.
MrAlias Tyler Yahn
viz: add `coordinates` attribute and remove `latitude` and `longitude`.

Fixes #1258 and #1302.
alexreisner added a commit that referenced this issue Jun 17, 2018

Verified

This commit was signed with the committer’s verified signature.
MrAlias Tyler Yahn
viz: add `coordinates` attribute and remove `latitude` and `longitude`.

Fixes #1258 and #1302.
alexreisner added a commit that referenced this issue Jun 18, 2018

Verified

This commit was signed with the committer’s verified signature.
MrAlias Tyler Yahn
viz: add `coordinates` attribute and remove `latitude` and `longitude`.

Fixes #1258 and #1302.
alexreisner added a commit that referenced this issue Jun 18, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
viz: add `coordinates` attribute and remove `latitude` and `longitude`.

Fixes #1258 and #1302.
alexreisner added a commit that referenced this issue Jul 31, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
viz: add `coordinates` attribute and remove `latitude` and `longitude`.

Fixes #1258 and #1302.
iarie pushed a commit to iarie/geocoder that referenced this issue Oct 10, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
viz: add `coordinates` attribute and remove `latitude` and `longitude`.

Fixes alexreisner#1258 and alexreisner#1302.
Br3nda added a commit to Growstuff/growstuff that referenced this issue Jun 19, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Br3nda added a commit to Growstuff/growstuff that referenced this issue Jun 19, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Br3nda added a commit to Growstuff/growstuff that referenced this issue Jun 19, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this issue Jan 15, 2021
GHSA-864j-6qpp-cmrr

Also, version 1.5.0 broke some of our tests, so I updated those as well:

> Test lookup fixtures should now return coordinates
> and NOT latitude/longitude attributes (see
> #1258). This may break some people's tests.

https://github.com/alexreisner/geocoder/blob/master/CHANGELOG.md#150-201
alexreisner/geocoder#1258

Refs: #2834
shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this issue Jan 20, 2021
GHSA-864j-6qpp-cmrr

Also, version 1.5.0 broke some of our tests, so I updated those as well:

> Test lookup fixtures should now return coordinates
> and NOT latitude/longitude attributes (see
> #1258). This may break some people's tests.

https://github.com/alexreisner/geocoder/blob/master/CHANGELOG.md#150-201
alexreisner/geocoder#1258

Refs: #2834
shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this issue Jan 26, 2021
GHSA-864j-6qpp-cmrr

Also, version 1.5.0 broke some of our tests, so I updated those as well:

> Test lookup fixtures should now return coordinates
> and NOT latitude/longitude attributes (see
> #1258). This may break some people's tests.

https://github.com/alexreisner/geocoder/blob/master/CHANGELOG.md#150-201
alexreisner/geocoder#1258

Refs: #2834
shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this issue Jul 21, 2023
These test stubs need to return `coordinates` and not `latitude` and
`longitude`.

alexreisner/geocoder#1197 (comment)
alexreisner/geocoder#1258

Refs: #3571
shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this issue Jul 21, 2023
These test stubs need to return `coordinates` and not `latitude` and
`longitude`.

alexreisner/geocoder#1197 (comment)
alexreisner/geocoder#1258

Refs: #3571
shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this issue Jul 26, 2023
These test stubs need to return `coordinates` and not `latitude` and
`longitude`.

alexreisner/geocoder#1197 (comment)
alexreisner/geocoder#1258

Refs: #3571
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants