From ac5aa927db1b7f073c23bc0ef37d339862cfec3a Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Wed, 30 Sep 2020 18:32:18 -0700 Subject: [PATCH] Test turn generation via goldenfiles instead. Revert the "import from a raw string" stuff from the previous commit. Add tests of a few interesting intersections. The results right now aren't ideal, but this sets things up for fast iteraton. --- abstutil/src/lib.rs | 4 ++ book/src/dev/testing.md | 6 ++ convert_osm/src/lib.rs | 8 +-- convert_osm/src/reader.rs | 20 ++----- data/regen.sh | 4 ++ importer/src/berlin.rs | 5 +- importer/src/krakow.rs | 5 +- importer/src/london.rs | 5 +- importer/src/main.rs | 2 +- importer/src/seattle.rs | 5 +- importer/src/tel_aviv.rs | 5 +- importer/src/xian.rs | 2 +- .../goldenfiles/left_turn_and_bike_lane.txt | 49 ++++++++++++++++ .../goldenfiles/multiple_left_turn_lanes.txt | 52 +++++++++++++++++ .../left_turn_and_bike_lane.osm} | 53 ++++++------------ map_tests/input/multiple_left_turn_lanes.osm | 52 +++++++++++++++++ map_tests/src/lib.rs | 36 ------------ map_tests/src/main.rs | 56 +++++++++++++++++++ 18 files changed, 253 insertions(+), 116 deletions(-) create mode 100644 map_tests/goldenfiles/left_turn_and_bike_lane.txt create mode 100644 map_tests/goldenfiles/multiple_left_turn_lanes.txt rename map_tests/{src/turns.rs => input/left_turn_and_bike_lane.osm} (51%) create mode 100644 map_tests/input/multiple_left_turn_lanes.osm delete mode 100644 map_tests/src/lib.rs create mode 100644 map_tests/src/main.rs diff --git a/abstutil/src/lib.rs b/abstutil/src/lib.rs index dde3e106ed..9ae3fe84e9 100644 --- a/abstutil/src/lib.rs +++ b/abstutil/src/lib.rs @@ -68,6 +68,8 @@ lazy_static::lazy_static! { "data".to_string() } else if file_exists("../data/".to_string()) { "../data".to_string() + } else if file_exists("../../data/".to_string()) { + "../../data".to_string() } else { panic!("Can't find the data/ directory"); } @@ -87,6 +89,8 @@ lazy_static::lazy_static! { "data".to_string() } else if file_exists("../data/".to_string()) { "../data".to_string() + } else if file_exists("../../data/".to_string()) { + "../../data".to_string() } else { panic!("Can't find the data/ directory"); } diff --git a/book/src/dev/testing.md b/book/src/dev/testing.md index d0f52d7f87..e756c44ea3 100644 --- a/book/src/dev/testing.md +++ b/book/src/dev/testing.md @@ -42,6 +42,12 @@ Additionally, this script does a few more tests: - `--check_proposals` makes sure the edits shipped with the game still load properly +## map_tests + +The `map_tests` crate runs the full importer against really simple `.osm` +files. To iterate rapidly on interpreting turn restrictions, for example, it +produces goldenfiles describing all turns in the tiny map. + ## Old tests Once upon a time, I made a little test harness that would run the simulation diff --git a/convert_osm/src/lib.rs b/convert_osm/src/lib.rs index 2416b553e8..36a6d76d28 100644 --- a/convert_osm/src/lib.rs +++ b/convert_osm/src/lib.rs @@ -12,14 +12,8 @@ use geom::{Distance, FindClosest, GPSBounds, LonLat, Pt2D, Ring}; use map_model::raw::RawMap; use map_model::{osm, MapConfig, NamePerLanguage}; -pub enum Input { - Path(String), - Contents(String), -} - pub struct Options { - pub osm_input: Input, - + pub osm_input: String, pub city_name: String, pub name: String, diff --git a/convert_osm/src/reader.rs b/convert_osm/src/reader.rs index 64d41656bb..34c585dd64 100644 --- a/convert_osm/src/reader.rs +++ b/convert_osm/src/reader.rs @@ -1,4 +1,3 @@ -use crate::Input; use abstutil::{prettyprint_usize, slurp_file, Tags, Timer}; use geom::{GPSBounds, LonLat, Pt2D}; use map_model::osm::{NodeID, OsmID, RelationID, WayID}; @@ -40,22 +39,15 @@ pub struct Relation { } pub fn read( - input: &Input, + path: &str, input_gps_bounds: &GPSBounds, timer: &mut Timer, ) -> Result> { - let bytes: Vec; - let tree = match input { - Input::Path(path) => { - timer.start(format!("read {}", path)); - bytes = slurp_file(path)?; - let raw_string = std::str::from_utf8(&bytes)?; - let tree = roxmltree::Document::parse(raw_string)?; - timer.stop(format!("read {}", path)); - tree - } - Input::Contents(contents) => roxmltree::Document::parse(contents)?, - }; + timer.start(format!("read {}", path)); + let bytes = slurp_file(path)?; + let raw_string = std::str::from_utf8(&bytes)?; + let tree = roxmltree::Document::parse(raw_string)?; + timer.stop(format!("read {}", path)); let mut doc = Document { gps_bounds: input_gps_bounds.clone(), diff --git a/data/regen.sh b/data/regen.sh index 10056eb84d..ce5e69e643 100755 --- a/data/regen.sh +++ b/data/regen.sh @@ -14,3 +14,7 @@ rm -fv data/system/maps/huge_seattle.bin data/input/raw_maps/huge_seattle.bin da cargo run --release --bin game -- --prebake cargo run --release --bin game -- --smoketest cargo run --release --bin game -- --check_proposals + +# This is cheap enough to do before every single commit, but since we don't +# have presubmit tests, it's fine to just run it here. +cargo run --bin map_tests diff --git a/importer/src/berlin.rs b/importer/src/berlin.rs index ffb3abbe43..00455d473e 100644 --- a/importer/src/berlin.rs +++ b/importer/src/berlin.rs @@ -58,10 +58,7 @@ pub fn osm_to_raw(name: &str, timer: &mut Timer, config: &ImporterConfiguration) let map = convert_osm::convert( convert_osm::Options { - osm_input: convert_osm::Input::Path(abstutil::path(format!( - "input/berlin/osm/{}.osm", - name - ))), + osm_input: abstutil::path(format!("input/berlin/osm/{}.osm", name)), city_name: "berlin".to_string(), name: name.to_string(), diff --git a/importer/src/krakow.rs b/importer/src/krakow.rs index 1509af4a38..590f934ef4 100644 --- a/importer/src/krakow.rs +++ b/importer/src/krakow.rs @@ -20,10 +20,7 @@ pub fn osm_to_raw(name: &str, timer: &mut abstutil::Timer, config: &ImporterConf let map = convert_osm::convert( convert_osm::Options { - osm_input: convert_osm::Input::Path(abstutil::path(format!( - "input/krakow/osm/{}.osm", - name - ))), + osm_input: abstutil::path(format!("input/krakow/osm/{}.osm", name)), city_name: "krakow".to_string(), name: name.to_string(), diff --git a/importer/src/london.rs b/importer/src/london.rs index 6514bdddf8..dd5c002c23 100644 --- a/importer/src/london.rs +++ b/importer/src/london.rs @@ -20,10 +20,7 @@ pub fn osm_to_raw(name: &str, timer: &mut abstutil::Timer, config: &ImporterConf let map = convert_osm::convert( convert_osm::Options { - osm_input: convert_osm::Input::Path(abstutil::path(format!( - "input/london/osm/{}.osm", - name - ))), + osm_input: abstutil::path(format!("input/london/osm/{}.osm", name)), city_name: "london".to_string(), name: name.to_string(), diff --git a/importer/src/main.rs b/importer/src/main.rs index 8d95aedc80..bd80bb8801 100644 --- a/importer/src/main.rs +++ b/importer/src/main.rs @@ -203,7 +203,7 @@ fn oneshot(osm_path: String, clip: Option, drive_on_right: bool, build_c let name = abstutil::basename(&osm_path); let raw = convert_osm::convert( convert_osm::Options { - osm_input: convert_osm::Input::Path(osm_path), + osm_input: osm_path, city_name: "oneshot".to_string(), name: name.clone(), diff --git a/importer/src/seattle.rs b/importer/src/seattle.rs index e84aa11e17..695c3c0b5d 100644 --- a/importer/src/seattle.rs +++ b/importer/src/seattle.rs @@ -67,10 +67,7 @@ pub fn osm_to_raw(name: &str, timer: &mut abstutil::Timer, config: &ImporterConf let map = convert_osm::convert( convert_osm::Options { - osm_input: convert_osm::Input::Path(abstutil::path(format!( - "input/seattle/osm/{}.osm", - name - ))), + osm_input: abstutil::path(format!("input/seattle/osm/{}.osm", name)), city_name: "seattle".to_string(), name: name.to_string(), diff --git a/importer/src/tel_aviv.rs b/importer/src/tel_aviv.rs index a89d35481d..5af060363c 100644 --- a/importer/src/tel_aviv.rs +++ b/importer/src/tel_aviv.rs @@ -20,10 +20,7 @@ pub fn osm_to_raw(name: &str, timer: &mut abstutil::Timer, config: &ImporterConf let map = convert_osm::convert( convert_osm::Options { - osm_input: convert_osm::Input::Path(abstutil::path(format!( - "input/tel_aviv/{}.osm", - name - ))), + osm_input: abstutil::path(format!("input/tel_aviv/osm/{}.osm", name)), city_name: "tel_aviv".to_string(), name: name.to_string(), diff --git a/importer/src/xian.rs b/importer/src/xian.rs index 4e31bfbae9..61c0d41426 100644 --- a/importer/src/xian.rs +++ b/importer/src/xian.rs @@ -20,7 +20,7 @@ pub fn osm_to_raw(name: &str, timer: &mut abstutil::Timer, config: &ImporterConf let map = convert_osm::convert( convert_osm::Options { - osm_input: convert_osm::Input::Path(abstutil::path(format!("input/xian/{}.osm", name))), + osm_input: abstutil::path(format!("input/xian/osm/{}.osm", name)), city_name: "xian".to_string(), name: name.to_string(), diff --git a/map_tests/goldenfiles/left_turn_and_bike_lane.txt b/map_tests/goldenfiles/left_turn_and_bike_lane.txt new file mode 100644 index 0000000000..b5b876c0e5 --- /dev/null +++ b/map_tests/goldenfiles/left_turn_and_bike_lane.txt @@ -0,0 +1,49 @@ +TurnID(Lane #0, Lane #5, Intersection #0) is a Crosswalk +TurnID(Lane #0, Lane #22, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #1, Lane #10, Intersection #0) is a Straight +TurnID(Lane #1, Lane #11, Intersection #0) is a Straight +TurnID(Lane #1, Lane #20, Intersection #0) is a Right +TurnID(Lane #1, Lane #21, Intersection #0) is a Right +TurnID(Lane #2, Lane #10, Intersection #0) is a Straight +TurnID(Lane #2, Lane #11, Intersection #0) is a Straight +TurnID(Lane #2, Lane #15, Intersection #0) is a Left +TurnID(Lane #2, Lane #21, Intersection #0) is a Right +TurnID(Lane #5, Lane #0, Intersection #0) is a Crosswalk +TurnID(Lane #5, Lane #13, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #6, Lane #12, Intersection #0) is a Crosswalk +TurnID(Lane #6, Lane #16, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #7, Lane #3, Intersection #0) is a Straight +TurnID(Lane #7, Lane #4, Intersection #0) is a Straight +TurnID(Lane #7, Lane #15, Intersection #0) is a Right +TurnID(Lane #7, Lane #21, Intersection #0) is a Left +TurnID(Lane #8, Lane #3, Intersection #0) is a Straight +TurnID(Lane #8, Lane #4, Intersection #0) is a Straight +TurnID(Lane #8, Lane #21, Intersection #0) is a Left +TurnID(Lane #9, Lane #20, Intersection #0) is a Left +TurnID(Lane #9, Lane #21, Intersection #0) is a Left +TurnID(Lane #12, Lane #6, Intersection #0) is a Crosswalk +TurnID(Lane #12, Lane #17, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #13, Lane #5, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #13, Lane #16, Intersection #0) is a Crosswalk +TurnID(Lane #14, Lane #3, Intersection #0) is a Right +TurnID(Lane #14, Lane #4, Intersection #0) is a Right +TurnID(Lane #14, Lane #10, Intersection #0) is a Left +TurnID(Lane #14, Lane #11, Intersection #0) is a Left +TurnID(Lane #14, Lane #20, Intersection #0) is a Straight +TurnID(Lane #14, Lane #21, Intersection #0) is a Straight +TurnID(Lane #16, Lane #6, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #16, Lane #13, Intersection #0) is a Crosswalk +TurnID(Lane #17, Lane #12, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #17, Lane #22, Intersection #0) is a Crosswalk +TurnID(Lane #18, Lane #3, Intersection #0) is a Left +TurnID(Lane #18, Lane #4, Intersection #0) is a Left +TurnID(Lane #18, Lane #10, Intersection #0) is a Right +TurnID(Lane #18, Lane #11, Intersection #0) is a Right +TurnID(Lane #18, Lane #15, Intersection #0) is a Straight +TurnID(Lane #19, Lane #3, Intersection #0) is a Left +TurnID(Lane #19, Lane #4, Intersection #0) is a Left +TurnID(Lane #19, Lane #10, Intersection #0) is a Right +TurnID(Lane #19, Lane #11, Intersection #0) is a Right +TurnID(Lane #19, Lane #15, Intersection #0) is a Straight +TurnID(Lane #22, Lane #0, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #22, Lane #17, Intersection #0) is a Crosswalk diff --git a/map_tests/goldenfiles/multiple_left_turn_lanes.txt b/map_tests/goldenfiles/multiple_left_turn_lanes.txt new file mode 100644 index 0000000000..0c3bac54e1 --- /dev/null +++ b/map_tests/goldenfiles/multiple_left_turn_lanes.txt @@ -0,0 +1,52 @@ +TurnID(Lane #0, Lane #4, Intersection #0) is a Crosswalk +TurnID(Lane #0, Lane #20, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #4, Lane #0, Intersection #0) is a Crosswalk +TurnID(Lane #4, Lane #15, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #5, Lane #9, Intersection #0) is a Crosswalk +TurnID(Lane #5, Lane #16, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #6, Lane #17, Intersection #0) is a Left +TurnID(Lane #6, Lane #18, Intersection #0) is a Left +TurnID(Lane #6, Lane #19, Intersection #0) is a Left +TurnID(Lane #7, Lane #1, Intersection #0) is a Straight +TurnID(Lane #7, Lane #2, Intersection #0) is a Straight +TurnID(Lane #7, Lane #3, Intersection #0) is a Straight +TurnID(Lane #7, Lane #17, Intersection #0) is a Left +TurnID(Lane #7, Lane #18, Intersection #0) is a Left +TurnID(Lane #7, Lane #19, Intersection #0) is a Left +TurnID(Lane #8, Lane #1, Intersection #0) is a Straight +TurnID(Lane #8, Lane #2, Intersection #0) is a Straight +TurnID(Lane #8, Lane #3, Intersection #0) is a Straight +TurnID(Lane #8, Lane #17, Intersection #0) is a Left +TurnID(Lane #8, Lane #18, Intersection #0) is a Left +TurnID(Lane #8, Lane #19, Intersection #0) is a Left +TurnID(Lane #9, Lane #5, Intersection #0) is a Crosswalk +TurnID(Lane #9, Lane #10, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #10, Lane #9, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #10, Lane #15, Intersection #0) is a Crosswalk +TurnID(Lane #11, Lane #1, Intersection #0) is a Right +TurnID(Lane #11, Lane #2, Intersection #0) is a Right +TurnID(Lane #11, Lane #3, Intersection #0) is a Right +TurnID(Lane #11, Lane #17, Intersection #0) is a Straight +TurnID(Lane #11, Lane #18, Intersection #0) is a Straight +TurnID(Lane #11, Lane #19, Intersection #0) is a Straight +TurnID(Lane #12, Lane #1, Intersection #0) is a Right +TurnID(Lane #12, Lane #2, Intersection #0) is a Right +TurnID(Lane #12, Lane #3, Intersection #0) is a Right +TurnID(Lane #12, Lane #17, Intersection #0) is a Straight +TurnID(Lane #12, Lane #18, Intersection #0) is a Straight +TurnID(Lane #12, Lane #19, Intersection #0) is a Straight +TurnID(Lane #13, Lane #1, Intersection #0) is a Right +TurnID(Lane #13, Lane #2, Intersection #0) is a Right +TurnID(Lane #13, Lane #3, Intersection #0) is a Right +TurnID(Lane #13, Lane #17, Intersection #0) is a Straight +TurnID(Lane #13, Lane #18, Intersection #0) is a Straight +TurnID(Lane #13, Lane #19, Intersection #0) is a Straight +TurnID(Lane #14, Lane #1, Intersection #0) is a Right +TurnID(Lane #14, Lane #2, Intersection #0) is a Right +TurnID(Lane #14, Lane #3, Intersection #0) is a Right +TurnID(Lane #15, Lane #4, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #15, Lane #10, Intersection #0) is a Crosswalk +TurnID(Lane #16, Lane #5, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #16, Lane #20, Intersection #0) is a Crosswalk +TurnID(Lane #20, Lane #0, Intersection #0) is a SharedSidewalkCorner +TurnID(Lane #20, Lane #16, Intersection #0) is a Crosswalk diff --git a/map_tests/src/turns.rs b/map_tests/input/left_turn_and_bike_lane.osm similarity index 51% rename from map_tests/src/turns.rs rename to map_tests/input/left_turn_and_bike_lane.osm index 04a1730500..1a9602165c 100644 --- a/map_tests/src/turns.rs +++ b/map_tests/input/left_turn_and_bike_lane.osm @@ -1,32 +1,6 @@ -use crate::import_map; -use map_model::{LaneID, TurnType}; - -// If this test gets fully fleshed out, there would be many more of these. This one means the -// southern road, inbound to the intersection, the left lane of it. -const S_IN_LEFT: LaneID = LaneID(2); - -#[test] -fn test_left_turn_lane() { - let map = import_map(four_way()); - - // Assert the hardcoded ID is reasonable - assert_eq!("south", map.get_parent(S_IN_LEFT).get_name(None)); - assert!(map.get_l(S_IN_LEFT).is_driving()); - - // TODO This is quite a weak assertion. I want to express that there's only the left turn from - // this lane, and S_IN_RIGHT has the two straight turns and the right turn. But it'll be so - // verbose. - assert_eq!( - TurnType::Left, - map.get_turns_from_lane(S_IN_LEFT)[0].turn_type - ); -} - -// A map with 4 roads (north, south, east, west) and one intersection. The north/south roads have 4 -// lanes, the east/west just 2. The south road has a left turn lane. -fn four_way() -> String { - format!( - r#" + + + @@ -38,34 +12,39 @@ fn four_way() -> String { - - + + - + + + + + - + + - + + + - "# - ) -} + diff --git a/map_tests/input/multiple_left_turn_lanes.osm b/map_tests/input/multiple_left_turn_lanes.osm new file mode 100644 index 0000000000..3c75daba6a --- /dev/null +++ b/map_tests/input/multiple_left_turn_lanes.osm @@ -0,0 +1,52 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/map_tests/src/lib.rs b/map_tests/src/lib.rs deleted file mode 100644 index e012c5354e..0000000000 --- a/map_tests/src/lib.rs +++ /dev/null @@ -1,36 +0,0 @@ -// This crate contains tests for the map importing pipeline. They're closer to integration tests -// than unit tests, mostly because of how tedious and brittle it would be to specify the full input -// to individual pieces of the pipeline. - -#[cfg(test)] -mod turns; - -// Run the contents of a .osm through the full map importer with default options. -#[cfg(test)] -fn import_map(raw_osm: String) -> map_model::Map { - let mut timer = abstutil::Timer::new("convert synthetic map"); - let raw = convert_osm::convert( - convert_osm::Options { - osm_input: convert_osm::Input::Contents(raw_osm), - city_name: "oneshot".to_string(), - name: "test_map".to_string(), - clip: None, - map_config: map_model::MapConfig { - driving_side: map_model::DrivingSide::Right, - bikes_can_use_bus_lanes: true, - }, - onstreet_parking: convert_osm::OnstreetParking::JustOSM, - public_offstreet_parking: convert_osm::PublicOffstreetParking::None, - private_offstreet_parking: convert_osm::PrivateOffstreetParking::FixedPerBldg(0), - elevation: None, - include_railroads: true, - }, - &mut timer, - ); - let map = map_model::Map::create_from_raw(raw, true, &mut timer); - // Useful to debug the result - if false { - map.save(); - } - map -} diff --git a/map_tests/src/main.rs b/map_tests/src/main.rs new file mode 100644 index 0000000000..8dcef08401 --- /dev/null +++ b/map_tests/src/main.rs @@ -0,0 +1,56 @@ +use map_model::Map; +use std::fs::File; +use std::io::Write; + +// Test the map pipeline by importing simple, handcrafted .osm files, then emitting goldenfiles +// that summarize part of the generated map. Keep the goldenfiles under version control to notice +// when they change. The goldenfiles (and changes to them) themselves aren't easy to understand, +// but the test maps are. +fn main() -> Result<(), std::io::Error> { + // TODO It's kind of a hack to reference the crate's directory relative to the data dir. + for path in abstutil::list_dir(std::path::Path::new(&abstutil::path("../map_tests/input"))) { + let map = import_map(path); + // Enable to debug the result wih the normal GUI + if false { + map.save(); + } + println!("Producing goldenfiles for {}", map.get_name()); + dump_turn_goldenfile(&map)?; + } + Ok(()) +} + +// Run the contents of a .osm through the full map importer with default options. +fn import_map(path: String) -> Map { + let mut timer = abstutil::Timer::new("convert synthetic map"); + let raw = convert_osm::convert( + convert_osm::Options { + name: abstutil::basename(&path), + osm_input: path, + city_name: "oneshot".to_string(), + clip: None, + map_config: map_model::MapConfig { + driving_side: map_model::DrivingSide::Right, + bikes_can_use_bus_lanes: true, + }, + onstreet_parking: convert_osm::OnstreetParking::JustOSM, + public_offstreet_parking: convert_osm::PublicOffstreetParking::None, + private_offstreet_parking: convert_osm::PrivateOffstreetParking::FixedPerBldg(0), + elevation: None, + include_railroads: true, + }, + &mut timer, + ); + let map = Map::create_from_raw(raw, true, &mut timer); + map +} + +// Verify what turns are generated by writing (from lane, to lane, turn type). +fn dump_turn_goldenfile(map: &Map) -> Result<(), std::io::Error> { + let path = abstutil::path(format!("../map_tests/goldenfiles/{}.txt", map.get_name())); + let mut f = File::create(path)?; + for (_, t) in map.all_turns() { + writeln!(f, "{} is a {:?}", t.id, t.turn_type)?; + } + Ok(()) +}