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.

Sign up for GitHub

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

Jump to bottom

Testing: Mock the google maps geocoding API in Jasmine #235

Merged
merged 5 commits into from
Jan 27, 2020
Merged

Testing: Mock the google maps geocoding API in Jasmine #235

merged 5 commits into from
Jan 27, 2020

Conversation

nstjean
Copy link
Contributor

@nstjean nstjean commented Jan 20, 2020

#233

  • Removes all reliance on the google API during tests (does not require a key or api call)
  • Using mocked data created 3 tests in geocoding_spec.js for the functions getPlacenameFromCoordinates and geocodeStringAndPan
  • Removed the "Can't find variable: google" warning in the other test spec files that would happen when the blurredLocation object was initialized
  • I will be trying to move the mock object and the response object to a helper file, but I'm having some issues getting jasmine to recognize any helper files.

Be aware that if the google maps api ever changes we will also have to change our mock object in testing.

I suspect I will have to do a similar mock/stub in LBLD and possibly LEL to correct the same error with the blurredLocation object in testing.

@nstjean
Copy link
Contributor Author

nstjean commented Jan 20, 2020

I figured out how to move it to a helper file! The existing support/jasmine.json file was NOT being checked by the script. I had to add a helper directory in Gruntfile.js!

@nstjean nstjean mentioned this pull request Jan 21, 2020
Closed
sagarpreet-chadha
sagarpreet-chadha approved these changes Jan 21, 2020
View reviewed changes
Copy link
Collaborator

@sagarpreet-chadha sagarpreet-chadha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FANTASTIC!!!

src/ui/Interface.js
@@ -29,7 +29,7 @@ module.exports = function Interface (options) {
options.onDrag = options.onDrag || function onDrag() {
function onPlacenameReturned(result) {
result = (result) ? result : options.placenameDisplayOnError; // this makes jasmine pass, other formats don't
$("#"+options.placenameDisplayId).val(result);
$("#"+options.placenameDisplayId).val(result.trim());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@nstjean
Copy link
Contributor Author

nstjean commented Jan 21, 2020

I have added error checks in Geocoding.js so that the functions will only execute if there is a google object - either a real one or a mock one. It was a bit tricky getting it set up so that it didn't mess up the mock object I just created in the tests, but I think I got it! I tested this on gh-pages, you can see that "the search for a place name" and "placename" display still work properly : https://publiclab.github.io/leaflet-blurred-location/examples/index.html

If this works properly it SHOULD mean that my fixes in LBLD and LEL are no longer needed! This should mean no warnings or errors even when there's no google api key given.

@jywarren
Copy link
Member

Awesome!

This was referenced Jan 21, 2020
Closed
Closed
@nstjean
Copy link
Contributor Author

nstjean commented Jan 22, 2020

@jywarren This is all set to merge! And once merged can you publish to npm?

@nstjean nstjean mentioned this pull request Jan 22, 2020
Closed
@jywarren jywarren merged commit 151a628 into publiclab:main Jan 27, 2020
@jywarren
Copy link
Member

Sorry I missed this! Doing it now, thank you!!!

@jywarren
Copy link
Member

Done!

@nstjean nstjean deleted the google-maps-mock branch January 28, 2020 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sagarpreet-chadha sagarpreet-chadha sagarpreet-chadha approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nstjean @jywarren @sagarpreet-chadha