publiclab / leaflet-blurred-location

A Leaflet-based interface for selecting a "blurred" or low-resolution location, to preserve privacy
https://publiclab.github.io/leaflet-blurred-location/examples/
GNU General Public License v3.0
35 stars 32 forks source link

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

Open nstjean opened 4 years ago

nstjean commented 4 years ago

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 commented 4 years ago

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

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

jywarren commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

nstjean commented 4 years ago

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 commented 4 years ago

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

nstjean commented 4 years ago

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 commented 4 years ago

Awesome!!!