Make Geocoding test with remote service optional

[chrismayer]

This ensures that our unit test suite does not fail in case the remote Geocoding service is not available. This service is external and unrelated to Wegue and thus leading to unwanted unit test failures.

If the "real" Geocoding API fails due to network issues in the tests this is now caught and the tests are skipped. Only "real" errors are forwarded, so the test suite can fail.

Note: Based on #471, so this needs to be merged first.

[sronveaux]

Hi @chrismayer

Thanks for adding this, this remote service test is really interesting and important but is a functional test, not a unit test. This means it can fail from time to time which is not good when used inside a build hook like here.

It seems #471 is present inside this PR... depending whether some changes are done inside it before merge, this one will need to be amended afterwards...

I think for this patch to work as expected, you'd need to add the condition I added in commentary otherwise it will not skip the test in case the server never replies at all. You can test this case very easily by forcing the url where requests are done with an IP address from a private range like http://10.255.255.1. You can easily force it directly in GeocoderController.js by setting url: "http://10.255.255.1",

I propose to wait for #471 to be adressed before including this one, or at least it should be curated and include only changes inside Geocoder.spec.js...

[chrismayer]

Thanks for your review @sronveaux . Since #471 is merged now I rebased this PR. I also added an additional check for ECONNABORTED as suggested by you. Please review again. TIA!

[sronveaux]

Hi @chrismayer,

Thanks for taking this back. I dis some test thoroughly and it works like a charm! That's a nice addition for the CI pipeline!

While parsing the modified unit test, I came across a design flaw inside the test suite including it and made a simple suggestion to make the test even more future-proof. However I let you choose whether it should be applied or not, the PR is approved and ready to merge!

[chrismayer]

Thanks for your ongoing review @sronveaux ...I am going to merge now.