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

Testing errors - ReferenceError: Can't find variable: google #233

Open
nstjean opened this issue Jan 16, 2020 · 9 comments
Open

Testing errors - ReferenceError: Can't find variable: google #233

nstjean opened this issue Jan 16, 2020 · 9 comments
Labels

Comments

@nstjean
Copy link
Contributor

nstjean commented Jan 16, 2020

These are the errors we're seeing in LBLD when we try to merge the newest changes from LBL:

FireShot Capture 228 - Job #329 1 - publiclab_leaflet-blurred-location-display - Travis CI_ - travis-ci org

And this is what I'm seeing when I run the tests in LBL from my dev environment:

natalie@natalie-ubuntu: ~-Dev-Ruby-public-lab-leaflet-blurred-location_018

I believe Jasmine is not loading the Google api correctly, or else the global variable is not carrying over into the tests (which makes sense for isolation purposes).

One possible solution is to make up a mock google.maps object to use for testing - this isolates our tests and prevents use of the google api every time we run tests.

Otherwise I need to find a way to get Jasmine to recognize and pull in the global variable.

@nstjean nstjean added the bug label Jan 16, 2020
@nstjean
Copy link
Contributor Author

nstjean commented Jan 16, 2020

I've noticed that there are no tests for any of the functions in Geocoding.js:

  • geocodeStringAndPan
  • getPlacenameFromCoordinates
  • panMapByBrowserGeocode
  • panMapToGeocodedLocation
  • geocodeWithBrowser

I suspect that when I write some they will fail until we get this error figured out.

@jywarren
Copy link
Member

Hmm. Do we have to change this line somehow to use the API differently, as we had in LBL?

https://github.com/publiclab/leaflet-blurred-location-display/blob/73d08add95f0837d36964789f3c84eaa157b7275/spec/javascripts/fixtures/example.html#L7

@nstjean
Copy link
Contributor Author

nstjean commented Jan 17, 2020

We probably do need to update that as well! However it won't solve this problem. I realized that the way the front-end API works when you call the script is this:

window.google = window.google || {};
google.maps = google.maps || {};

But in PhantomJS tests in the console as we are running them there literally is no window object. That's why google.maps keeps returning as undefined in the tests.

I'm going to take a look at LBLD tomorrow to see what's going on there. The text failures are all specifying the same failure point in LBL - the line in Geocoding.js where it uses the google.maps object. So I'm like 99% sure changing the key in LBLD isn't going to fix it, but I'll try that first.

@sagarpreet-chadha
Copy link
Collaborator

Yes LBLD tests used to run fine. The problem is in the testing framework as we have this same error as warning in LBL tests also.
I am thinking that these tests should be independent of Google API and it is infact a good option to mock this API. What do you think @jywarren ?

@nstjean
Copy link
Contributor Author

nstjean commented Jan 17, 2020

Yes the more I think about it the more I think mocking the API is the best choice.

@nstjean
Copy link
Contributor Author

nstjean commented Jan 21, 2020

I created a mock for our testing purposes in LBL, but I've noticed that LBLD and LEL both need a fix implemented to stub the geocoder object so that the tests can pass. I worry about this causing errors when people implement LBL. Should I dig into LBL and try to find a way to code it so that if there's no google api key it doesn't cause these testing errors?

@sagarpreet-chadha
Copy link
Collaborator

Hey now you have created mock API, in the future we can use that only?

@nstjean
Copy link
Contributor Author

nstjean commented Jan 21, 2020

Yes! We will probably have to add on to it for other functions other than Geocoder, but that shouldn't be too difficult to do. We won't need the google api key in testing at all.

@sagarpreet-chadha
Copy link
Collaborator

Awesome!!!

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

No branches or pull requests

3 participants