From ab91a0568081ffde61ec3e4e5383efe7e567cd2c Mon Sep 17 00:00:00 2001 From: karenzshea Date: Thu, 2 Feb 2017 13:30:01 +0100 Subject: [PATCH] update cucumber suport code to return separate annotations headers --- docs/testing.md | 2 +- features/step_definitions/matching.js | 26 +++++++---- features/support/route.js | 28 +++++------- features/support/shared_steps.js | 16 +++++-- features/testbot/matching.feature | 58 +++++++------------------ features/testbot/weight.feature | 18 ++++---- unit_tests/server/parameters_parser.cpp | 8 +--- 7 files changed, 69 insertions(+), 87 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index e4e33ee1e..96940c9c1 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1,6 +1,6 @@ # Testsuite -OSRM comes with a testsuite containing both unit-tests using the Boost library and cucucmber.js for scenario driven testing. +OSRM comes with a testsuite containing both unit-tests using the Boost library and cucumber.js for scenario driven testing. ## Unit Tests diff --git a/features/step_definitions/matching.js b/features/step_definitions/matching.js index 77f751c80..042505358 100644 --- a/features/step_definitions/matching.js +++ b/features/step_definitions/matching.js @@ -83,7 +83,10 @@ module.exports = function () { duration = json.matchings[0].duration; } - if (headers.has('annotation')) { + // annotation response values are requested by 'a:{type_name}' + var found = false; + headers.forEach((h) => { if (h.match(/^a:/)) found = true; }); + if (found) { if (json.matchings.length != 1) throw new Error('*** Checking annotation only supported for matchings with one subtrace'); annotation = this.annotationList(json.matchings[0]); } @@ -92,11 +95,6 @@ module.exports = function () { if (json.matchings.length != 1) throw new Error('*** Checking geometry only supported for matchings with one subtrace'); geometry = json.matchings[0].geometry; } - - if (headers.has('OSM IDs')) { - if (json.matchings.length != 1) throw new Error('*** Checking annotation only supported for matchings with one subtrace'); - OSMIDs = this.OSMIDList(json.matchings[0]); - } } if (headers.has('turns')) { @@ -111,9 +109,19 @@ module.exports = function () { got.duration = duration.toString(); } - if (headers.has('annotation')) { - got.annotation = annotation.toString(); - } + // if header matches 'a:*', parse out the values for * + // and return in that header + headers.forEach((k) => { + let whitelist = ['duration', 'distance', 'datasources', 'nodes', 'weight']; + if (k.match(/^a:/)) { + let a_type = k.slice(2); + if (whitelist.indexOf(a_type) == -1) + return cb(new Error('Unrecognized annotation field:' + a_type)); + if (!annotation[a_type]) + return cb(new Error('Annotation not found in response: ' + a_type)); + got[k] = annotation[a_type]; + } + }); if (headers.has('geometry')) { if (this.queryParams['geometries'] === 'polyline') { diff --git a/features/support/route.js b/features/support/route.js index 34f0ff3de..0be1f75c0 100644 --- a/features/support/route.js +++ b/features/support/route.js @@ -167,23 +167,17 @@ module.exports = function () { if (!('annotation' in instructions.legs[0])) return ''; - function zip(list_1, list_2, list_3, list_4) - { - let tuples = []; - for (let i = 0; i < list_1.length; ++i) { - tuples.push([list_1[i], list_2[i], list_3[i], list_4[i]]); - } - return tuples; - } - return instructions.legs.map(l => { - const values = zip( l.annotation.weight, l.annotation.duration, l.annotation.distance, l.annotation.datasources); - return values.map(p => { return p.join(':'); }).join(','); - }).join(','); - }; - - this.OSMIDList = (instructions) => { - // OSM node IDs also come from the annotation list - return instructions.legs.map(l => l.annotation.nodes.map(n => n.toString()).join(',')).join(','); + var merged = {}; + instructions.legs.map(l => { + Object.keys(l.annotation).forEach(a => { + if (!merged[a]) merged[a] = []; + merged[a].push(l.annotation[a].join(':')); + }); + }); + Object.keys(merged).map(a => { + merged[a] = merged[a].join(','); + }); + return merged; }; this.lanesList = (instructions) => { diff --git a/features/support/shared_steps.js b/features/support/shared_steps.js index 3f6683695..6a66376b7 100644 --- a/features/support/shared_steps.js +++ b/features/support/shared_steps.js @@ -133,9 +133,19 @@ module.exports = function () { got.locations = (locations || '').trim(); } - if (headers.has('annotation')){ - got.annotation = (annotation || '').trim(); - } + // if header matches 'a:*', parse out the values for * + // and return in that header + headers.forEach((k) => { + let whitelist = ['duration', 'distance', 'datasources', 'nodes', 'weight']; + if (k.match(/^a:/)) { + let a_type = k.slice(2); + if (whitelist.indexOf(a_type) == -1) + return cb(new Error('Unrecognized annotation field', a_type)); + if (!annotation[a_type]) + return cb(new Error('Annotation not found in response', a_type)); + got[k] = annotation[a_type]; + } + }); var putValue = (key, value) => { if (headers.has(key)) got[key] = instructions ? value : ''; diff --git a/features/testbot/matching.feature b/features/testbot/matching.feature index 5c61dd83c..fcfa60418 100644 --- a/features/testbot/matching.feature +++ b/features/testbot/matching.feature @@ -141,12 +141,12 @@ Feature: Basic Map Matching And the contract extra arguments "--segment-speed-file {speeds_file}" When I match I should get - | trace | matchings | annotation | - | ach | ach | 1,1,0,1,1,2,1 | + | trace | matchings | a:duration | + | ach | ach | 1:1,0:1:1:2:1 | Scenario: Testbot - Duration details Given the query options - | annotations | true | + | annotations | duration,nodes | Given the node map """ @@ -167,15 +167,15 @@ Feature: Basic Map Matching And the contract extra arguments "--segment-speed-file {speeds_file}" When I match I should get - | trace | matchings | annotation | - | abeh | abeh | 1:10.008842:1:1:1,0:0:0:2:0,1:10.008842:1:2:0,1:10.008842:1:3:0,1:10.008842:1:4:0,0:0:0:4:0,2:19.906475:2:5:0,1:10.008842:1:6:0 | - | abci | abci | 1:10.008842:1:1:1,0:0:0:2:0,1:10.008842:1:2:0,0:0:0:2:0,1:10.010367:1:3:0 | + | trace | matchings | a:duration | + | abeh | abeh | 1:0,1:1:1,0:2:1 | + | abci | abci | 1:0,1,0:1 | # The following is the same as the above, but separated for readability (line length) When I match I should get - | trace | matchings | OSM IDs | - | abeh | abeh | 1,2,3,2,3,4,5,4,5,6,7 | - | abci | abci | 1,2,3,2,3,2,3,8 | + | trace | matchings | a:nodes | + | abeh | abeh | 1:2:3,2:3:4:5,4:5:6:7 | + | abci | abci | 1:2:3,2:3,2:3:8 | Scenario: Testbot - Regression test for #3037 Given the query options @@ -303,7 +303,7 @@ Feature: Basic Map Matching | abcd | 0 1 2 3 | abcd | # Regression test 1 for issue 3176 - Scenario: Testbot - multiuple segments: properly expose OSM IDs + Scenario: Testbot - multiple segments: properly expose OSM IDs Given the query options | annotations | true | @@ -332,9 +332,9 @@ Feature: Basic Map Matching | fg | no | When I match I should get - | trace | OSM IDs | - | 12 | 1,2,3,4,5,6,7 | - | 21 | 7,6,5,4,3,2,1 | + | trace | a:nodes | + | 12 | 1:2:3:4:5:6:7 | + | 21 | 7:6:5:4:3:2:1 | # Regression test 2 for issue 3176 Scenario: Testbot - same edge: properly expose OSM IDs @@ -360,32 +360,6 @@ Feature: Basic Map Matching | abcdef | no | When I match I should get - | trace | OSM IDs | - | 12 | 1,2,3,4,5,6 | - | 21 | 6,5,4,3,2,1 | - - Scenario: Testbot - return annotations based on parameter - Given the query options - | annotations | duration,weight | - - Given the node map - """ - a - b - | - c - """ - - And the nodes - | node | id | - | a | 1 | - | b | 2 | - | c | 3 | - - And the ways - | nodes | oneway | - | abc | no | - - When I match I should get - | trace | annotation | - | ac | 2:2,2:2 | - | ca | 2:2,2:2,0:0 | + | trace | a:nodes | + | 12 | 1:2:3:4:5:6 | + | 21 | 6:5:4:3:2:1 | diff --git a/features/testbot/weight.feature b/features/testbot/weight.feature index 43dd2d792..14b24c5cc 100644 --- a/features/testbot/weight.feature +++ b/features/testbot/weight.feature @@ -10,7 +10,7 @@ Feature: Weight tests Scenario: Weight details Given the query options - | annotations | true | + | annotations | weight | Given the node map """ @@ -29,8 +29,8 @@ Feature: Weight tests | cde | When I route I should get - | waypoints | route | annotation | - | s,t | abc,cde,cde | 1.1:10.008843:1.1:2:0,2:20.017686:2:3:0,2:20.020734:2:4:0,1:10.010367:1:5:0 | + | waypoints | route | a:weight | + | s,t | abc,cde,cde | 1.1:2:2:1 | When I route I should get | waypoints | route | times | weight_name | weights | @@ -39,7 +39,7 @@ Feature: Weight tests # FIXME include/engine/guidance/assemble_geometry.hpp:95 Scenario: Start and target on the same and adjacent edge Given the query options - | annotations | true | + | annotations | distance,duration,weight,nodes | Given the node map """ @@ -53,11 +53,11 @@ Feature: Weight tests | abc | When I route I should get - | waypoints | route | distances | weights | times | annotation | - | s,t | abc,abc | 20m,0m | 2.1,0 | 2.1s,0s | 3:20.017685:3:1:0 | - | t,s | abc,abc | 20m,0m | 2.1,0 | 2.1s,0s | 3.1:20.017685:3.1:2:0 | - | s,e | abc,abc | 40m,0m | 4.1,0 | 4.1s,0s | 3.1:30.026527:3.1:1:0,1:10.008842:1:2:0 | - | e,s | abc,abc | 40m,0m | 4.1,0 | 4.1s,0s | 1:10.008842:1:3:0,3.1:30.026527:3.1:2:0 | + | waypoints | route | distances | weights | times | a:distance | a:duration | a:weight | + | s,t | abc,abc | 20m,0m | 2.1,0 | 2.1s,0s | 20.017685 | 3 | 3 | + | t,s | abc,abc | 20m,0m | 2.1,0 | 2.1s,0s | 20.017685 | 3.1 | 3.1 | + | s,e | abc,abc | 40m,0m | 4.1,0 | 4.1s,0s | 30.026527:10.008842 | 3.1:1 | 3.1:1 | + | e,s | abc,abc | 40m,0m | 4.1,0 | 4.1s,0s | 10.008842:30.026527 | 1:3.1 | 1:3.1 | Scenario: Step weights -- way_function: fail if no weight or weight_per_meter property diff --git a/unit_tests/server/parameters_parser.cpp b/unit_tests/server/parameters_parser.cpp index 195c5d7ad..b5d90bbb1 100644 --- a/unit_tests/server/parameters_parser.cpp +++ b/unit_tests/server/parameters_parser.cpp @@ -132,9 +132,7 @@ BOOST_AUTO_TEST_CASE(valid_route_urls) CHECK_EQUAL_RANGE(reference_2.radiuses, result_2->radiuses); CHECK_EQUAL_RANGE(reference_2.coordinates, result_2->coordinates); CHECK_EQUAL_RANGE(reference_2.hints, result_2->hints); - BOOST_CHECK_EQUAL( - static_cast(result_2->annotations_type & RouteParameters::AnnotationsType::All), - true); + BOOST_CHECK_EQUAL(result_2->annotations_type == RouteParameters::AnnotationsType::All, true); RouteParameters reference_3{false, false, @@ -376,9 +374,7 @@ BOOST_AUTO_TEST_CASE(valid_route_urls) "1,2;3,4?overview=simplified&annotations=duration,weight,nodes,datasources,distance"); BOOST_CHECK(result_17); BOOST_CHECK_EQUAL(reference_17.geometries, result_17->geometries); - BOOST_CHECK_EQUAL( - static_cast(result_2->annotations_type & RouteParameters::AnnotationsType::All), - true); + BOOST_CHECK_EQUAL(result_2->annotations_type == RouteParameters::AnnotationsType::All, true); BOOST_CHECK_EQUAL(result_17->annotations, true); }