Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Conflict with signinfo's temporary linguistic node sequence file caused test failures. #4625

Merged
merged 14 commits into from Mar 12, 2024
Merged

Conflict with signinfo's temporary linguistic node sequence file caused test failures. #4625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2b19544
updated path location so that we don't share the same as utrecht.cc test
gknisely Mar 11, 2024
ad2e534
updated
gknisely Mar 11, 2024
ae02884
more cleanup
gknisely Mar 11, 2024
c0adc5b
try cout
gknisely Mar 11, 2024
6c77dcf
revert back.
gknisely Mar 11, 2024
3361d47
testing
gknisely Mar 11, 2024
1ff0ca7
clean up
gknisely Mar 11, 2024
b907568
updated
gknisely Mar 12, 2024
ef62d7e
reverted
gknisely Mar 12, 2024
6555fc9
this was not the issue. can share dir
gknisely Mar 12, 2024
0b0308f
was using same test file.
gknisely Mar 12, 2024
80b46fd
reverted back.
gknisely Mar 12, 2024
043610e
Update CHANGELOG.md
gknisely Mar 12, 2024
66ecf2d
Update graphparser.cc
gknisely Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -96,6 +96,7 @@
* FIXED: Fixed roundoff issue in Tiles Row and Col methods [#4585](https://github.com/valhalla/valhalla/pull/4585)
* ADDED: isochrone proper polygon support & pbf output for isochrone [#4575](https://github.com/valhalla/valhalla/pull/4575)
* ADDED: return isotile grid as geotiff [#4594](https://github.com/valhalla/valhalla/pull/4594)
* FIXED: Conflict with signinfo's temporary linguistic node sequence file caused test failures. [#4625](https://github.com/valhalla/valhalla/pull/4625)

## Release Date: 2023-05-11 Valhalla 3.4.0
* **Removed**
Expand Down
200 changes: 63 additions & 137 deletions test/graphparser.cc
View file
Open in desktop
Expand Up @@ -28,6 +28,16 @@

const std::string config_file = "test/test_config_gp";

std::string ways_file = "test_ways.bin";
std::string way_nodes_file = "test_way_nodes.bin";
std::string nodes_file = "test_nodes.bin";
std::string edges_file = "test_edges.bin";
std::string access_file = "test_access.bin";
std::string from_restriction_file = "test_from_complex_restrictions.bin";
std::string to_restriction_file = "test_to_complex_restrictions.bin";
std::string bss_nodes_file = "test_bss_nodes.bin";
std::string linguistic_node_file = "test_linguistic_node.bin";

const auto node_predicate = [](const OSMWayNode& a, const OSMWayNode& b) {
return a.node.osmid_ < b.node.osmid_;
};
Expand All @@ -48,18 +58,55 @@
return *found;
}

void DoConfig() {
std::ofstream file;
try {
file.open(config_file, std::ios_base::trunc);
file << "{ \
\"mjolnir\": { \
\"id_table_size\": 1000, \
\"tile_dir\": \"test/data/parser_tiles\" \
} \
}";
} catch (...) {}

Check warning on line 71 in test/graphparser.cc

View check run for this annotation

Codecov / codecov/patch

test/graphparser.cc#L71 

Added line #L71 was not covered by tests
file.close();
}

// must do clean up here vs TearDown() as we are building data
// in the same directory multiple times
void CleanUp() {
if (filesystem::exists(ways_file))
filesystem::remove(ways_file);

if (filesystem::exists(way_nodes_file))
filesystem::remove(way_nodes_file);

if (filesystem::exists(nodes_file))
filesystem::remove(nodes_file);

if (filesystem::exists(edges_file))
filesystem::remove(edges_file);

if (filesystem::exists(access_file))
filesystem::remove(access_file);

if (filesystem::exists(from_restriction_file))
filesystem::remove(from_restriction_file);

if (filesystem::exists(to_restriction_file))
filesystem::remove(to_restriction_file);

if (filesystem::exists(bss_nodes_file))
filesystem::remove(bss_nodes_file);

if (filesystem::exists(linguistic_node_file))
filesystem::remove(linguistic_node_file);
}

void BollardsGatesAndAccess(const std::string& config_file) {
boost::property_tree::ptree conf;
rapidjson::read_json(config_file, conf);

std::string ways_file = "test_ways.bin";
std::string way_nodes_file = "test_way_nodes.bin";
std::string access_file = "test_access.bin";
std::string from_restriction_file = "test_from_complex_restrictions.bin";
std::string to_restriction_file = "test_to_complex_restrictions.bin";
std::string bss_nodes_file = "test_bss_nodes.bin";
std::string linguistic_node_file = "test_linguistic_node.bin";

auto osmdata =
PBFGraphParser::ParseWays(conf.get_child("mjolnir"),
{VALHALLA_SOURCE_DIR "test/data/liechtenstein-latest.osm.pbf"},
Expand Down Expand Up @@ -206,27 +253,13 @@
EXPECT_TRUE((bike_network & kMcn) && (bike_network & kRcn) && way_75786176.bike_network() == 0)
<< "rcn and mtb not marked on way 75786176.";

filesystem::remove(ways_file);
filesystem::remove(way_nodes_file);
filesystem::remove(access_file);
filesystem::remove(from_restriction_file);
filesystem::remove(to_restriction_file);
filesystem::remove(bss_nodes_file);
filesystem::remove(linguistic_node_file);
CleanUp();
}

void RemovableBollards(const std::string& config_file) {
boost::property_tree::ptree conf;
rapidjson::read_json(config_file, conf);

std::string ways_file = "test_ways.bin";
std::string way_nodes_file = "test_way_nodes.bin";
std::string access_file = "test_access.bin";
std::string from_restriction_file = "test_from_complex_restrictions.bin";
std::string to_restriction_file = "test_to_complex_restrictions.bin";
std::string bss_nodes_file = "test_bss_nodes.bin";
std::string linguistic_node_file = "test_linguistic_node.bin";

auto osmdata = PBFGraphParser::ParseWays(conf.get_child("mjolnir"),
{VALHALLA_SOURCE_DIR "test/data/rome.osm.pbf"}, ways_file,
way_nodes_file, access_file);
Expand All @@ -250,27 +283,13 @@
kEmergencyAccess | kPedestrianAccess | kWheelchairAccess |
kBicycleAccess | kMopedAccess | kMotorcycleAccess);

filesystem::remove(ways_file);
filesystem::remove(way_nodes_file);
filesystem::remove(access_file);
filesystem::remove(from_restriction_file);
filesystem::remove(to_restriction_file);
filesystem::remove(bss_nodes_file);
filesystem::remove(linguistic_node_file);
CleanUp();
}

void Exits(const std::string& config_file) {
boost::property_tree::ptree conf;
rapidjson::read_json(config_file, conf);

std::string ways_file = "test_ways.bin";
std::string way_nodes_file = "test_way_nodes.bin";
std::string access_file = "test_access.bin";
std::string from_restriction_file = "test_from_complex_restrictions.bin";
std::string to_restriction_file = "test_to_complex_restrictions.bin";
std::string bss_nodes_file = "test_bss_nodes.bin";
std::string linguistic_node_file = "test_linguistic_node.bin";

auto osmdata = PBFGraphParser::ParseWays(conf.get_child("mjolnir"),
{VALHALLA_SOURCE_DIR "test/data/harrisburg.osm.pbf"},
ways_file, way_nodes_file, access_file);
Expand Down Expand Up @@ -302,27 +321,13 @@
EXPECT_EQ(osmdata.node_names.name(node.exit_to_index()), "PA441")
<< "node exit_to not set correctly .";

filesystem::remove(ways_file);
filesystem::remove(way_nodes_file);
filesystem::remove(access_file);
filesystem::remove(from_restriction_file);
filesystem::remove(to_restriction_file);
filesystem::remove(bss_nodes_file);
filesystem::remove(linguistic_node_file);
CleanUp();
}

void Baltimore(const std::string& config_file) {
boost::property_tree::ptree conf;
rapidjson::read_json(config_file, conf);

std::string ways_file = "test_ways.bin";
std::string way_nodes_file = "test_way_nodes.bin";
std::string access_file = "test_access.bin";
std::string from_restriction_file = "test_from_complex_restrictions.bin";
std::string to_restriction_file = "test_to_complex_restrictions.bin";
std::string bss_nodes_file = "test_bss_nodes.bin";
std::string linguistic_node_file = "test_linguistic_node.bin";

auto osmdata = PBFGraphParser::ParseWays(conf.get_child("mjolnir"),
{VALHALLA_SOURCE_DIR "test/data/baltimore.osm.pbf"},
ways_file, way_nodes_file, access_file);
Expand Down Expand Up @@ -430,27 +435,13 @@
FAIL() << "98040438 restriction test failed.";
}

filesystem::remove(ways_file);
filesystem::remove(way_nodes_file);
filesystem::remove(access_file);
filesystem::remove(from_restriction_file);
filesystem::remove(to_restriction_file);
filesystem::remove(bss_nodes_file);
filesystem::remove(linguistic_node_file);
CleanUp();
}

void Bike(const std::string& config_file) {
boost::property_tree::ptree conf;
rapidjson::read_json(config_file, conf);

std::string ways_file = "test_ways.bin";
std::string way_nodes_file = "test_way_nodes.bin";
std::string access_file = "test_access.bin";
std::string from_restriction_file = "test_from_complex_restrictions.bin";
std::string to_restriction_file = "test_to_complex_restrictions.bin";
std::string bss_nodes_file = "test_bss_nodes.bin";
std::string linguistic_node_file = "test_linguistic_node.bin";

auto osmdata = PBFGraphParser::ParseWays(conf.get_child("mjolnir"),
{VALHALLA_SOURCE_DIR "test/data/bike.osm.pbf"}, ways_file,
way_nodes_file, access_file);
Expand Down Expand Up @@ -528,26 +519,13 @@
EXPECT_TRUE(way_156539491.moped_forward());
EXPECT_TRUE(way_156539491.bike_backward());

filesystem::remove(ways_file);
filesystem::remove(way_nodes_file);
filesystem::remove(access_file);
filesystem::remove(from_restriction_file);
filesystem::remove(to_restriction_file);
filesystem::remove(linguistic_node_file);
CleanUp();
}

void Bus(const std::string& config_file) {
boost::property_tree::ptree conf;
rapidjson::read_json(config_file, conf);

std::string ways_file = "test_ways.bin";
std::string way_nodes_file = "test_way_nodes.bin";
std::string access_file = "test_access.bin";
std::string from_restriction_file = "test_from_complex_restrictions.bin";
std::string to_restriction_file = "test_to_complex_restrictions.bin";
std::string bss_nodes_file = "test_bss_nodes.bin";
std::string linguistic_node_file = "test_linguistic_node.bin";

auto osmdata = PBFGraphParser::ParseWays(conf.get_child("mjolnir"),
{VALHALLA_SOURCE_DIR "test/data/bus.osm.pbf"}, ways_file,
way_nodes_file, access_file);
Expand Down Expand Up @@ -610,27 +588,13 @@
EXPECT_FALSE(way_225895737.bus_backward());
EXPECT_FALSE(way_225895737.bike_backward());

filesystem::remove(ways_file);
filesystem::remove(way_nodes_file);
filesystem::remove(access_file);
filesystem::remove(from_restriction_file);
filesystem::remove(to_restriction_file);
filesystem::remove(bss_nodes_file);
filesystem::remove(linguistic_node_file);
CleanUp();
}

void BicycleTrafficSignals(const std::string& config_file) {
boost::property_tree::ptree conf;
rapidjson::read_json(config_file, conf);

std::string ways_file = "test_ways.bin";
std::string way_nodes_file = "test_way_nodes.bin";
std::string access_file = "test_access.bin";
std::string from_restriction_file = "test_from_complex_restrictions.bin";
std::string to_restriction_file = "test_to_complex_restrictions.bin";
std::string bss_nodes_file = "test_bss_nodes.bin";
std::string linguistic_node_file = "test_linguistic_node.bin";

auto osmdata = PBFGraphParser::ParseWays(conf.get_child("mjolnir"),
{VALHALLA_SOURCE_DIR "test/data/nyc.osm.pbf"}, ways_file,
way_nodes_file, access_file);
Expand Down Expand Up @@ -661,27 +625,7 @@
<< "Bike rental at a shop not marked as intersection."
*/

filesystem::remove(ways_file);
filesystem::remove(way_nodes_file);
filesystem::remove(access_file);
filesystem::remove(from_restriction_file);
filesystem::remove(to_restriction_file);
filesystem::remove(bss_nodes_file);
filesystem::remove(linguistic_node_file);
}

void DoConfig() {
std::ofstream file;
try {
file.open(config_file, std::ios_base::trunc);
file << "{ \
\"mjolnir\": { \
\"id_table_size\": 1000, \
\"tile_dir\": \"test/data/parser_tiles\" \
gknisely marked this conversation as resolved.
Show resolved Hide resolved
} \
}";
} catch (...) {}
file.close();
CleanUp();
}

TEST(GraphParser, TestBollardsGatesAndAccess) {
Expand Down Expand Up @@ -726,16 +670,6 @@

conf.put("mjolnir.import_bike_share_stations", true);

std::string ways_file = "test_ways.bin";
std::string way_nodes_file = "test_way_nodes.bin";
std::string nodes_file = "test_nodes.bin";
std::string edges_file = "test_edges.bin";
std::string access_file = "test_access.bin";
std::string from_restriction_file = "test_from_complex_restrictions.bin";
std::string to_restriction_file = "test_to_complex_restrictions.bin";
std::string bss_nodes_file = "test_bss_nodes.bin";
std::string linguistic_node_file = "test_linguistic_node.bin";

auto osmdata = PBFGraphParser::ParseWays(conf.get_child("mjolnir"),
{VALHALLA_SOURCE_DIR "test/data/rome.osm.pbf"}, ways_file,
way_nodes_file, access_file);
Expand Down Expand Up @@ -825,15 +759,7 @@
kPedestrianAccess);
check_edge_attribute(local_tile->directededge(edge_idx_2 + count_2 - 2), kPedestrianAccess,
kBicycleAccess);

filesystem::remove(ways_file);
filesystem::remove(way_nodes_file);
filesystem::remove(bss_nodes_file);
filesystem::remove(access_file);
filesystem::remove(from_restriction_file);
filesystem::remove(to_restriction_file);
filesystem::remove(bss_nodes_file);
filesystem::remove(linguistic_node_file);
CleanUp();
}

} // namespace
Expand Down
2 changes: 1 addition & 1 deletion test/signinfo.cc
View file
Open in desktop
Expand Up @@ -30,7 +30,7 @@ TEST(Signinfo, ExitToTest) {
std::vector<SignInfo> signs;
std::vector<std::string> linguistics;
std::vector<std::pair<std::string, bool>> default_languages;
const std::string linguistic_node_file = "test_linguistic_node.bin";
const std::string linguistic_node_file = "test_sign_linguistic_node.bin";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was the cause of the problem. In the multi-threaded environment the test_linguistic_node.bin was getting deleted for the graphparser.cc test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this test also builds to the same directory as graphparser.cc? if so, shouldn't we change that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most are all running in the same directory. This test is testing specifically signs and is not building data. This is a temporary file as it is required for CreateSignInfoList We are not testing linguistic node signage here as it is done in the gurka tests. The real solution is to migrate the old tests to gurka.

sequence<OSMNodeLinguistic> linguistic_node(linguistic_node_file, true);

bool has_guide = GraphBuilder::CreateSignInfoList(exit_node, way, pronunciationMap, langMap,
Expand Down