From 429eb316d01e19b4cc64ef959c131a75df3079d3 Mon Sep 17 00:00:00 2001 From: Emil Tin Date: Sat, 18 Feb 2012 14:41:15 +0100 Subject: [PATCH 1/6] update tests to use new query delimter ? instead of & --- features/support/route.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/support/route.rb b/features/support/route.rb index f74ecd6f28b..a66818f60a6 100644 --- a/features/support/route.rb +++ b/features/support/route.rb @@ -1,7 +1,7 @@ require 'net/http' def request_route a,b - @query = "http://localhost:5000/viaroute&start=#{a}&dest=#{b}&output=json&geomformat=cmp" + @query = "http://localhost:5000/viaroute?start=#{a}&dest=#{b}&output=json&geomformat=cmp" #log @query uri = URI.parse @query Net::HTTP.get_response uri From 695fecddebd45e7b1e1e3ed362cffe9295fca4a9 Mon Sep 17 00:00:00 2001 From: Emil Tin Date: Sat, 18 Feb 2012 14:49:02 +0100 Subject: [PATCH 2/6] in cucumber tests, always show failed rows right below expected ones --- features/step_definitions/routing.rb | 4 +- features/support/cucumber.rb | 76 ++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 features/support/cucumber.rb diff --git a/features/step_definitions/routing.rb b/features/step_definitions/routing.rb index b5ba1431842..fa47a2f7d1b 100644 --- a/features/step_definitions/routing.rb +++ b/features/step_definitions/routing.rb @@ -178,7 +178,7 @@ def computed_route actual << got end end - table.diff! actual + table.routing_diff! actual end When /^I route I should get$/ do |table| @@ -217,5 +217,5 @@ def computed_route actual << got end end - table.diff! actual + table.routing_diff! actual end diff --git a/features/support/cucumber.rb b/features/support/cucumber.rb new file mode 100644 index 00000000000..78465bed892 --- /dev/null +++ b/features/support/cucumber.rb @@ -0,0 +1,76 @@ +#monkey patch cucumber table class to reorder output. +#we always want failed rows to be shown right below the expected row. + +class Cucumber::Ast::Table + def routing_diff!(other_table, options={}) + options = {:missing_row => true, :surplus_row => true, :missing_col => true, :surplus_col => false}.merge(options) + + other_table = ensure_table(other_table) + other_table.convert_headers! + other_table.convert_columns! + ensure_green! + + convert_headers! + convert_columns! + + original_width = cell_matrix[0].length + other_table_cell_matrix = pad!(other_table.cell_matrix) + padded_width = cell_matrix[0].length + + missing_col = cell_matrix[0].detect{|cell| cell.status == :undefined} + surplus_col = padded_width > original_width + + require_diff_lcs + cell_matrix.extend(Diff::LCS) + changes = cell_matrix.diff(other_table_cell_matrix).flatten + + inserted = 0 + missing = 0 + + row_indices = Array.new(other_table_cell_matrix.length) {|n| n} + + last_change = nil + missing_row_pos = nil + insert_row_pos = nil + + changes.each do |change| + if(change.action == '-') + missing_row_pos = change.position + inserted + cell_matrix[missing_row_pos].each{|cell| cell.status = :undefined} + row_indices.insert(missing_row_pos, nil) + missing += 1 + else # '+' + #change index so we interleave instead + insert_row_pos = change.position + inserted + 1 + #insert_row_pos = change.position + missing #original + + inserted_row = change.element + inserted_row.each{|cell| cell.status = :comment} + cell_matrix.insert(insert_row_pos, inserted_row) + row_indices[insert_row_pos] = nil + inspect_rows(cell_matrix[missing_row_pos], inserted_row) if last_change && last_change.action == '-' + inserted += 1 + end + last_change = change + end + + other_table_cell_matrix.each_with_index do |other_row, i| + row_index = row_indices.index(i) + row = cell_matrix[row_index] if row_index + if row + (original_width..padded_width).each do |col_index| + surplus_cell = other_row[col_index] + row[col_index].value = surplus_cell.value if row[col_index] + end + end + end + + clear_cache! + should_raise = + missing_row_pos && options[:missing_row] || + insert_row_pos && options[:surplus_row] || + missing_col && options[:missing_col] || + surplus_col && options[:surplus_col] + raise Different.new(self) if should_raise + end +end \ No newline at end of file From 169988b3c4a521c587e240e07b6d9e8f12c53038 Mon Sep 17 00:00:00 2001 From: Emil Tin Date: Sat, 18 Feb 2012 15:28:50 +0100 Subject: [PATCH 3/6] gitignore the sandbox/ folder --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index e499a3282b8..d75581b8c59 100644 --- a/.gitignore +++ b/.gitignore @@ -75,3 +75,7 @@ win/bin-debug/ /osrm-routed /osrm-prepare /nohup.out + +# Sandbox folder # +################### +sandbox/ From 801490b9fc979b450f1470173b1135101057715d Mon Sep 17 00:00:00 2001 From: Emil Tin Date: Sat, 18 Feb 2012 15:52:45 +0100 Subject: [PATCH 4/6] perform a sanity check on which ways are used during routability tests --- features/step_definitions/routing.rb | 35 ++++++++++++++++------------ features/support/data.rb | 2 +- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/features/step_definitions/routing.rb b/features/step_definitions/routing.rb index b5ba1431842..5ca00cc060d 100644 --- a/features/step_definitions/routing.rb +++ b/features/step_definitions/routing.rb @@ -157,24 +157,29 @@ def computed_route table.hashes.each_with_index do |row,i| got = row.dup attempts = [] - if table.headers.include? 'forw' - response = request_route("#{ORIGIN[1]},#{ORIGIN[0]+(1+WAY_SPACING*i)*ZOOM}","#{ORIGIN[1]},#{ORIGIN[0]+(2+WAY_SPACING*i)*ZOOM}") - got['forw'] = route_status response - if got['forw'] != row['forw'] + row['way'] = "w#{i}" + got['way'] = "w#{i}" + ['forw','backw'].each do |direction| + if table.headers.include? direction + if direction == 'forw' + response = request_route("#{ORIGIN[1]},#{ORIGIN[0]+(1+WAY_SPACING*i)*@zoom}","#{ORIGIN[1]},#{ORIGIN[0]+(2+WAY_SPACING*i)*@zoom}") + elsif direction == 'backw' + response = request_route("#{ORIGIN[1]},#{ORIGIN[0]+(2+WAY_SPACING*i)*@zoom}","#{ORIGIN[1]},#{ORIGIN[0]+(1+WAY_SPACING*i)*@zoom}") + end + got[direction] = route_status response json = JSON.parse(response.body) - attempts << { :attempt => 'Forward', :query => @query, :response => response } + if got[direction] == 'x' + route = way_list json['route_instructions'] + if route != "w#{i}" + got[direction] = "#{route}?" + end + end + if got[direction] != row[direction] + attempts << { :attempt => direction, :query => @query, :response => response } + end end end - if table.headers.include? 'backw' - response = request_route("#{ORIGIN[1]},#{ORIGIN[0]+(2+WAY_SPACING*i)*ZOOM}","#{ORIGIN[1]},#{ORIGIN[0]+(1+WAY_SPACING*i)*ZOOM}") - got['backw'] = route_status response - if got['backw'] != row['backw'] - attempts << { :attempt => 'Backward', :query => @query, :response => response } - end - end - if got != row - log_fail row,got,attempts - end + log_fail row,got,attempts if got != row actual << got end end diff --git a/features/support/data.rb b/features/support/data.rb index 140d2ec72f7..63fcec13e21 100644 --- a/features/support/data.rb +++ b/features/support/data.rb @@ -56,7 +56,7 @@ def build_ways_from_table table tags = row.dup tags.delete 'forw' tags.delete 'backw' - tags['name'] = "abcd#{ri}" + tags['name'] = "w#{ri}" tags.reject! { |k,v| v=='' } way << tags osm_db << way From 9eef17506d0bd6c91d498bad28fd2b023a495f6a Mon Sep 17 00:00:00 2001 From: Emil Tin Date: Sat, 18 Feb 2012 16:46:57 +0100 Subject: [PATCH 5/6] add a speedprofile for walking, and a test for basic way accessability --- features/access.feature | 29 +++++++++++++++++++++++++++-- speedprofiles/foot.ini | 28 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 speedprofiles/foot.ini diff --git a/features/access.feature b/features/access.feature index bcdf7ada5b7..5151aa1e1d2 100644 --- a/features/access.feature +++ b/features/access.feature @@ -1,6 +1,6 @@ @routing @access -Feature: Oneway streets - Basic accessability of various way types. +Feature: Default speedprofiles + Basic accessability of various way types depending on speedprofile. Scenario: Basic access for cars Given the speedprofile "car" @@ -53,3 +53,28 @@ Feature: Oneway streets | cycleway | x | | bridleway | | + Scenario: Basic access for walking + Given the speedprofile "foot" + Then routability should be + | highway | forw | + | motorway | | + | motorway_link | | + | trunk | | + | trunk_link | x | + | primary | x | + | secondary | x | + | tertiary | x | + | residential | x | + | service | x | + | unclassified | x | + | living_street | x | + | road | x | + | track | x | + | path | x | + | footway | x | + | pedestrian | x | + | steps | x | + | pier | x | + | cycleway | x | + | bridleway | | + diff --git a/speedprofiles/foot.ini b/speedprofiles/foot.ini new file mode 100644 index 00000000000..656c82ad5b7 --- /dev/null +++ b/speedprofiles/foot.ini @@ -0,0 +1,28 @@ +[foot] + accessTag = foot + defaultSpeed = 5 + obeyOneways = no + useRestrictions = no + obeyBollards = no + + primary = 5 + primary_link = 5 + secondary = 5 + secondary_link = 5 + tertiary = 5 + residential = 5 + unclassified = 5 + living_street = 5 + road = 5 + service = 5 + track = 5 + path = 5 + cycleway = 5 + footway = 5 + pedestrian = 5 + pier = 5 + steps = 2 + + ferry = 5 + + excludeFromGrid = ferry From ef9c3c84319641d5621f4055cfb2e37cf5def149 Mon Sep 17 00:00:00 2001 From: Emil Tin Date: Sat, 18 Feb 2012 17:50:04 +0100 Subject: [PATCH 6/6] added tests for distance calculations (some are failing) --- features/distance.feature | 279 +++++++++++++++++++++++++++ features/snap.feature | 22 +++ features/step_definitions/routing.rb | 9 +- 3 files changed, 307 insertions(+), 3 deletions(-) create mode 100644 features/distance.feature diff --git a/features/distance.feature b/features/distance.feature new file mode 100644 index 00000000000..393244aae94 --- /dev/null +++ b/features/distance.feature @@ -0,0 +1,279 @@ +@routing @distance +Feature: Distance calculation + + Scenario: Distance of a winding south-north path + Given a grid size of 10 meters + Given the nodes + | a | b | + | d | c | + | e | f | + | h | g | + + And the ways + | nodes | + | abcdefg | + + When I route I should get + | from | to | route | distance | + | a | b | abcdefg | 10 | + | a | c | abcdefg | 20 | + | a | d | abcdefg | 30 | + | a | e | abcdefg | 40 | + | a | f | abcdefg | 50 | + | a | g | abcdefg | 60 | + | a | h | abcdefg | 70 | + + Scenario: Distance of a winding east-west path + Given a grid size of 10 meters + Given the nodes + | a | d | e | h | + | b | c | f | g | + + And the ways + | nodes | + | abcdefg | + + When I route I should get + | from | to | route | distance | + | a | b | abcdefg | 10 | + | a | c | abcdefg | 20 | + | a | d | abcdefg | 30 | + | a | e | abcdefg | 40 | + | a | f | abcdefg | 50 | + | a | g | abcdefg | 60 | + | a | h | abcdefg | 70 | + + Scenario: Distances when traversing part of a way + Given a grid size of 100 meters + Given the nodes + | a | 0 | 1 | 2 | + | 9 | | | 3 | + | 8 | | | 4 | + | 7 | 6 | 5 | b | + + And the ways + | nodes | + | ab | + + When I route I should get + | from | to | route | distance | + | a | 0 | ab | 70 | + | a | 1 | ab | 140 | + | a | 2 | ab | 210 | + | a | 3 | ab | 280 | + | a | 4 | ab | 350 | + | a | b | ab | 420 | + | a | 5 | ab | 350 | + | a | 6 | ab | 280 | + | a | 7 | ab | 210 | + | a | 8 | ab | 140 | + | a | 9 | ab | 70 | + | b | 5 | ab | 70 | + | b | 6 | ab | 140 | + | b | 7 | ab | 210 | + | b | 8 | ab | 280 | + | b | 9 | ab | 350 | + | b | a | ab | 420 | + | b | 0 | ab | 350 | + | b | 1 | ab | 280 | + | b | 2 | ab | 210 | + | b | 3 | ab | 140 | + | b | 4 | ab | 70 | + + Scenario: Geometric distances + Given a grid size of 1000 meters + Given the nodes + | v | w | y | a | b | c | d | + | u | | | | | | e | + | t | | | | | | f | + | s | | | x | | | g | + | r | | | | | | h | + | q | | | | | | i | + | p | o | n | m | l | k | j | + + And the ways + | nodes | + | xa | + | xb | + | xc | + | xd | + | xe | + | xf | + | xg | + | xh | + | xi | + | xj | + | xk | + | xl | + | xm | + | xn | + | xo | + | xp | + | xq | + | xr | + | xs | + | xt | + | xu | + | xv | + | xw | + | xy | + + When I route I should get + | from | to | route | distance | + | x | a | xa | 3000 | + | x | b | xb | 3160 | + | x | c | xc | 3600 | + | x | d | xd | 4240 | + | x | e | xe | 3600 | + | x | f | xf | 3160 | + | x | g | xg | 3000 | + | x | h | xh | 3160 | + | x | i | xi | 3600 | + | x | j | xj | 4240 | + | x | k | xk | 3600 | + | x | l | xl | 3160 | + | x | m | xm | 3000 | + | x | n | xn | 3160 | + | x | o | xo | 3600 | + | x | p | xp | 4240 | + | x | q | xq | 3600 | + | x | r | xr | 3160 | + | x | s | xs | 3000 | + | x | t | xt | 3160 | + | x | u | xu | 3600 | + | x | v | xv | 4240 | + | x | w | xw | 3600 | + | x | y | xy | 3160 | + + Scenario: 1m distances + Given a grid size of 1 meters + Given the nodes + | a | b | + | | c | + + And the ways + | nodes | + | abc | + + When I route I should get + | from | to | route | distance | + | a | b | abc | 1 | + | b | a | abc | 1 | + | b | c | abc | 1 | + | c | b | abc | 1 | + | a | c | abc | 2 | + | c | a | abc | 2 | + + Scenario: 10m distances + Given a grid size of 10 meters + Given the nodes + | a | b | + | | c | + + And the ways + | nodes | + | abc | + + When I route I should get + | from | to | route | distance | + | a | b | abc | 10 | + | b | a | abc | 10 | + | b | c | abc | 10 | + | c | b | abc | 10 | + | a | c | abc | 20 | + | c | a | abc | 20 | + + Scenario: 100m distances + Given a grid size of 100 meters + Given the nodes + | a | b | + | | c | + + And the ways + | nodes | + | abc | + + When I route I should get + | from | to | route | distance | + | a | b | abc | 100 | + | b | a | abc | 100 | + | b | c | abc | 100 | + | c | b | abc | 100 | + | a | c | abc | 200 | + | c | a | abc | 200 | + + Scenario: 1km distance + Given a grid size of 1000 meters + Given the nodes + | a | b | + | | c | + + And the ways + | nodes | + | abc | + + When I route I should get + | from | to | route | distance | + | a | b | abc | 1000 | + | b | a | abc | 1000 | + | b | c | abc | 1000 | + | c | b | abc | 1000 | + | a | c | abc | 2000 | + | c | a | abc | 2000 | + + Scenario: 10km distances + Given a grid size of 10000 meters + Given the nodes + | a | b | + | | c | + + And the ways + | nodes | + | abc | + + When I route I should get + | from | to | route | distance | + | a | b | abc | 10000 | + | b | a | abc | 10000 | + | b | c | abc | 10000 | + | c | b | abc | 10000 | + | a | c | abc | 20000 | + | c | a | abc | 20000 | + + Scenario: 100km distances + Given a grid size of 100000 meters + Given the nodes + | a | b | + | | c | + + And the ways + | nodes | + | abc | + + When I route I should get + | from | to | route | distance | + | a | b | abc | 100000 | + | b | a | abc | 100000 | + | b | c | abc | 100000 | + | c | b | abc | 100000 | + | a | c | abc | 200000 | + | c | a | abc | 200000 | + + Scenario: 1000km distances + Given a grid size of 1000000 meters + Given the nodes + | a | b | + | | c | + + And the ways + | nodes | + | abc | + + When I route I should get + | from | to | route | distance | + | a | b | abc | 1000000 | + | b | a | abc | 1000000 | + | b | c | abc | 1000000 | + | c | b | abc | 1000000 | + | a | c | abc | 2000000 | + | c | a | abc | 2000000 | \ No newline at end of file diff --git a/features/snap.feature b/features/snap.feature index 8ebe4f24daf..00b665f174f 100644 --- a/features/snap.feature +++ b/features/snap.feature @@ -84,3 +84,25 @@ Feature: Snap start/end point to the nearest way | b | j | jkla | | b | k | jkla | | b | l | jkla | + + Scenario: Snap to correct way at large scales + Given a grid size of 1000 meters + Given the nodes + | | | | a | + | x | | | b | + | | | | c | + + And the ways + | nodes | + | xa | + | xb | + | xc | + + When I route I should get + | from | to | route | + | x | a | xa | + | x | b | xb | + | x | c | xc | + | a | x | xa | + | b | x | xb | + | c | x | xc | diff --git a/features/step_definitions/routing.rb b/features/step_definitions/routing.rb index b5ba1431842..e35a8e89458 100644 --- a/features/step_definitions/routing.rb +++ b/features/step_definitions/routing.rb @@ -209,9 +209,12 @@ def computed_route if table.headers.include? 'route' got['route'] = (instructions || '').strip end - - if got['route'] != row['route'] || got['start'] != row['start'] || got['end'] != row['end'] - failed = { :attempt => 'Backward', :query => @query, :response => response } + if table.headers.include? 'distance' + got['distance'] = instructions ? json['route_summary']['total_distance'].to_s : nil + end + + if row != got + failed = { :attempt => 'route', :query => @query, :response => response } log_fail row,got,[failed] end actual << got