added e2e tests, some minor response parsing fixes #85
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,3 +13,5 @@ test.py | |
| *.pyc | ||
| *.idea/ | ||
| .DS_Store | ||
| **/test_data/* | ||
| !test_data/andorra-latest.osm.pbf | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file not shown.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| version: "3.8" | ||
| services: | ||
| ors: | ||
| container_name: ors-tests | ||
| ports: | ||
| - "8005:8080" | ||
| image: ghcr.io/gis-ops/openrouteservice:latest | ||
| volumes: | ||
| - ../test_data/andorra-latest.osm.pbf:/ors-core/data/osm_file.pbf | ||
| environment: | ||
| - "JAVA_OPTS=-Djava.awt.headless=true -server -XX:TargetSurvivorRatio=75 -XX:SurvivorRatio=64 -XX:MaxTenuringThreshold=3 -XX:+UseG1GC -XX:+ScavengeBeforeFullGC -XX:ParallelGCThreads=4 -Xms1g -Xmx2g" | ||
| - "CATALINA_OPTS=-Dcom.sun.management.jmxremote -Dcom.sun.management.jmxremote.port=9001 -Dcom.sun.management.jmxremote.rmi.port=9001 -Dcom.sun.management.jmxremote.authenticate=false -Dcom.sun.management.jmxremote.ssl=false -Djava.rmi.server.hostname=localhost" | ||
| valhalla: | ||
| container_name: valhalla-tests | ||
| image: gisops/valhalla:latest | ||
| ports: | ||
| - "8002:8002" | ||
| volumes: | ||
| - ../test_data/:/custom_files | ||
|
There was a problem hiding this comment. don't we also have to uncomment this? |
||
| osrm: | ||
| container_name: osrm-tests | ||
| image: osrm/osrm-backend:latest | ||
| ports: | ||
| - "5000:5000" | ||
| volumes: | ||
| - ../test_data/:/data | ||
| command: '/bin/bash -c "osrm-extract -p /opt/car.lua /data/andorra-latest.osm.pbf && osrm-partition /data/andorra-latest.osrm && osrm-customize /data/andorra-latest.osrm && osrm-routed --algorithm=MLD /data/andorra-latest.osrm"' | ||
| graphhopper: | ||
| container_name: graphhopper-tests | ||
| image: israelhikingmap/graphhopper:latest | ||
| ports: | ||
| - "8989:8989" | ||
| volumes: | ||
| - ../test_data/:/graphhopper/data | ||
| command: "-i data/andorra-latest.osm.pbf --host 0.0.0.0" | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| import tests as _test | ||
| from routingpy import OSRM, Graphhopper, Valhalla | ||
|
|
||
|
|
||
| class TestRoutingpyIntegration(_test.TestCase): | ||
| @classmethod | ||
| def setUpClass(cls) -> None: | ||
| cls.valhalla = Valhalla("http://localhost:8002") | ||
| cls.osrm = OSRM("http://localhost:5000") | ||
| # ignore ORS for now, not working in GH Actions | ||
| # (see e.g. https://github.com/gis-ops/routingpy/actions/runs/4022971575/jobs/6913297227) | ||
| # cls.ors = ORS(base_url="http://localhost:8005/ors") | ||
| cls.gh = Graphhopper(base_url="http://localhost:8989") | ||
|
|
||
| def test_valhalla_directions(self): | ||
|
|
||
| directions = self.valhalla.directions( | ||
| [ | ||
| [1.51886, 42.5063], | ||
| [1.53789, 42.51007], | ||
| ], | ||
| "auto", | ||
| ) | ||
|
|
||
| self.assertIsInstance(directions.geometry, list) | ||
| self.assertIsInstance(directions.duration, int) | ||
| self.assertIsInstance(directions.distance, int) | ||
|
|
||
| def test_valhalla_isochrones(self): | ||
|
|
||
| isochrones = self.valhalla.isochrones( | ||
| [ | ||
| [1.51886, 42.5063], | ||
| ], | ||
| "auto", | ||
| [20, 50], | ||
| ) | ||
|
|
||
| self.assertIsInstance(isochrones[1].geometry, list) | ||
| self.assertEqual(isochrones[0].interval, 20) | ||
| self.assertEqual(isochrones[1].interval, 50) | ||
| self.assertEqual(isochrones[1].center, [1.51886, 42.5063]) | ||
|
|
||
| def test_valhalla_matrix(self): | ||
| matrix = self.valhalla.matrix( | ||
| [ | ||
| [1.51886, 42.5063], | ||
| [1.53789, 42.51007], | ||
| [1.53489, 42.52007], | ||
| [1.53189, 42.51607], | ||
| ], | ||
| "auto", | ||
| ) | ||
|
|
||
| self.assertEqual(len(matrix.distances), 4) | ||
| self.assertEqual(len(matrix.durations), 4) | ||
| self.assertEqual(len(matrix.durations[0]), 4) | ||
| self.assertEqual(len(matrix.distances[0]), 4) | ||
|
|
||
| def test_osrm_directions(self): | ||
| directions = self.osrm.directions( | ||
| [ | ||
| [1.51886, 42.5063], | ||
| [1.53789, 42.51007], | ||
| ] | ||
| ) | ||
|
|
||
| self.assertIsInstance(directions.geometry, list) | ||
| self.assertIsInstance(directions.duration, int) | ||
| self.assertIsInstance(directions.distance, int) | ||
|
|
||
| def test_osrm_matrix(self): | ||
| matrix = self.osrm.matrix( | ||
| [ | ||
| [1.51886, 42.5063], | ||
| [1.53789, 42.51007], | ||
| [1.53489, 42.52007], | ||
| [1.53189, 42.51607], | ||
| ], | ||
| "auto", | ||
| ) | ||
|
|
||
| self.assertEqual(len(matrix.distances), 4) | ||
| self.assertEqual(len(matrix.durations), 4) | ||
| self.assertEqual(len(matrix.durations[0]), 4) | ||
| self.assertEqual(len(matrix.distances[0]), 4) | ||
|
|
||
| # def test_ors_directions(self): | ||
| # directions = self.ors.directions( | ||
| # [ | ||
| # [1.51886, 42.5063], | ||
| # [1.53789, 42.51007], | ||
| # ], | ||
| # "driving-car", | ||
| # ) | ||
| # | ||
| # self.assertIsInstance(directions.geometry, list) | ||
| # self.assertIsInstance(directions.duration, int) | ||
| # self.assertIsInstance(directions.distance, int) | ||
| # | ||
| # def test_ors_isochrones(self): | ||
| # isochrones = self.ors.isochrones([1.51886, 42.5063], "driving-car", [20, 50]) | ||
| # | ||
| # self.assertIsInstance(isochrones[1].geometry, list) | ||
| # self.assertEqual(isochrones[0].interval, 20) | ||
| # self.assertEqual(isochrones[1].interval, 50) | ||
| # self.assertAlmostEqual(isochrones[1].center[0], 1.51886, 1) | ||
| # self.assertAlmostEqual(isochrones[1].center[1], 42.5063, 1) | ||
| # | ||
| # def test_ors_matrix(self): | ||
| # matrix = self.ors.matrix( | ||
| # [ | ||
| # [1.51886, 42.5063], | ||
| # [1.53789, 42.51007], | ||
| # [1.53489, 42.52007], | ||
| # [1.53189, 42.51607], | ||
| # ], | ||
| # "driving-car", | ||
| # metrics=["distance", "duration"], | ||
| # ) | ||
| # | ||
| # self.assertEqual(len(matrix.distances), 4) | ||
| # self.assertEqual(len(matrix.durations), 4) | ||
| # self.assertEqual(len(matrix.durations[0]), 4) | ||
| # self.assertEqual(len(matrix.distances[0]), 4) | ||
|
|
||
| def test_graphhopper_directions(self): | ||
| directions = self.gh.directions( | ||
| [ | ||
| [1.51886, 42.5063], | ||
| [1.53789, 42.51007], | ||
| ], | ||
| "car", | ||
| ) | ||
|
|
||
| self.assertIsInstance(directions.geometry, list) | ||
| self.assertIsInstance(directions.duration, int) | ||
| self.assertIsInstance(directions.distance, int) | ||
|
|
||
| def test_graphhopper_isochrones(self): | ||
| isochrones = self.gh.isochrones([1.51886, 42.5063], "car", [50]) | ||
|
|
||
| self.assertIsInstance(isochrones[0].geometry, list) | ||
| self.assertEqual(isochrones[0].interval, 50) | ||
| self.assertAlmostEqual(isochrones[0].center[0], 1.51886, 1) | ||
| self.assertAlmostEqual(isochrones[0].center[1], 42.5063, 1) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand this (and was wrong before too, but don't think it's actually solved now):
locationsis an array right? If you look atparse_isochrone_json, it'll be put ascenterfor an individual isochrone. So down there it's missing an indexing into thecenterarray.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a couple of things here: the parameter is
locationswhen really, it's expecting one location as[lon, lat]so it already represents a single location. On top of that,centeris really just that location but the coordinates are represented as strings:https://github.com/gis-ops/routingpy/blob/73bf55cdcb8871c1c6b9886bc14a5ca8aef9a38f/routingpy/routers/graphhopper.py#L490
The real problem seems to be the confusion about the name of the
locationsparameter: Valhalla, ORS, GraphHopper and HERE all only accept one single location, yet that parameter is consistently named in the plural form. We could either consistently change them tolocationor we accept multiple and fire multiple requests.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What adds even more to the confusion is that Valhalla takes one location but nested into an array, but GraphHopper and ORS take one location (unnested).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost: Valhalla & ORS do accept multiple locations, but only ORS is doing what the code assumes, Valhalla does some weird union stuff with the multiple locations. Since we just came from ORS when we started routingpy, we tried to support that. In the end, I agree, let's just get rid of supporting multiple locations and accept only a single location for isochrones. Major version sort of thing, but no problem to push one, I don't really care tbh.