From fa0d4c923f83602e3bd974473a4a6fc1475a82f5 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Sat, 30 Jun 2018 15:04:35 +0200 Subject: [PATCH 1/4] datasource tests: also check value types in REQUIRE_ATTRIBUTES --- test/unit/datasource/ds_test_util.hpp | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/test/unit/datasource/ds_test_util.hpp b/test/unit/datasource/ds_test_util.hpp index 46868d4e5..bf377ca83 100644 --- a/test/unit/datasource/ds_test_util.hpp +++ b/test/unit/datasource/ds_test_util.hpp @@ -107,13 +107,20 @@ inline std::size_t count_features(mapnik::featureset_ptr features) { using attr = std::tuple; -#define REQUIRE_ATTRIBUTES(feature, attrs) \ - REQUIRE(bool(feature)); \ - for (auto const &kv : attrs) { \ - REQUIRE(feature->has_key(std::get<0>(kv))); \ - CHECK(feature->get(std::get<0>(kv)) == std::get<1>(kv)); \ - } \ - +#define REQUIRE_ATTRIBUTES(feature, ...) \ + do { \ + auto const& _feat = (feature); /* evaluate feature only once */ \ + REQUIRE(_feat != nullptr); \ + for (auto const& kv : __VA_ARGS__) { \ + auto& key = std::get<0>(kv); \ + auto& val = std::get<1>(kv); \ + CAPTURE(key); \ + CHECKED_IF(_feat->has_key(key)) { \ + CHECK(_feat->get(key) == val); \ + CHECK(_feat->get(key).which() == val.which()); \ + } \ + } \ + } while (0) inline void require_attributes(mapnik::feature_ptr feature, std::initializer_list const &attrs) { From 93c379820e84b2bea06b151f13276f4d1be05bcd Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Sat, 30 Jun 2018 15:09:49 +0200 Subject: [PATCH 2/4] datasource tests: use macro REQUIRE_ATTRIBUTES instead of function - failing checks report location inside the function, not where it's called from; using macro reports proper location --- test/unit/datasource/csv.cpp | 52 +++++++++++++-------------- test/unit/datasource/ds_test_util.hpp | 5 --- 2 files changed, 26 insertions(+), 31 deletions(-) diff --git a/test/unit/datasource/csv.cpp b/test/unit/datasource/csv.cpp index 6142185ca..634978c45 100644 --- a/test/unit/datasource/csv.cpp +++ b/test/unit/datasource/csv.cpp @@ -241,7 +241,7 @@ TEST_CASE("csv") { auto features = ds->features(query); auto feature = features->next(); - require_attributes(feature, { + REQUIRE_ATTRIBUTES(feature, { attr { lon_name, mapnik::value_integer(0) }, attr { "lat", mapnik::value_integer(0) } }); @@ -298,8 +298,8 @@ TEST_CASE("csv") { , attr { "geo_longitude", mapnik::value_integer(-70) } , attr { "geo_latitude", mapnik::value_integer(40) } }; - require_attributes(feature, expected_attr); - require_attributes(feature2, expected_attr); + REQUIRE_ATTRIBUTES(feature, expected_attr); + REQUIRE_ATTRIBUTES(feature2, expected_attr); if (mapnik::util::exists(filepath + ".index")) { mapnik::util::remove(filepath + ".index"); @@ -367,7 +367,7 @@ TEST_CASE("csv") { auto featureset = all_features(ds); auto feature = featureset->next(); - require_attributes(feature, { + REQUIRE_ATTRIBUTES(feature, { attr { "x", mapnik::value_integer(0) } , attr { "empty_column", mapnik::value_unicode_string("") } , attr { "text", mapnik::value_unicode_string("a b") } @@ -416,15 +416,15 @@ TEST_CASE("csv") { require_field_types(fields, {mapnik::Integer, mapnik::Integer, mapnik::String}); auto featureset = all_features(ds); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 0} , attr{"y", 0} , attr{"name", mapnik::value_unicode_string("a/a") } }); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 1} , attr{"y", 4} , attr{"name", mapnik::value_unicode_string("b/b") } }); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 10} , attr{"y", 2.5} , attr{"name", mapnik::value_unicode_string("c/c") } }); @@ -531,7 +531,7 @@ TEST_CASE("csv") { auto fields = ds->get_descriptor().get_descriptors(); require_field_names(fields, {"x", "y", "1990", "1991", "1992"}); auto feature = all_features(ds)->next(); - require_attributes(feature, { + REQUIRE_ATTRIBUTES(feature, { attr{"x", 0} , attr{"y", 0} , attr{"1990", 1} @@ -575,15 +575,15 @@ TEST_CASE("csv") { require_field_names(fields, {"x", "y", "label"}); auto featureset = all_features(ds); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 0}, attr{"y", 0}, attr{"label", ustring("0,0") } }); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 5}, attr{"y", 5}, attr{"label", ustring("5,5") } }); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 0}, attr{"y", 5}, attr{"label", ustring("0,5") } }); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 5}, attr{"y", 0}, attr{"label", ustring("5,0") } }); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 2.5}, attr{"y", 2.5}, attr{"label", ustring("2.5,2.5") } }); if (mapnik::util::exists(filename + ".index")) { @@ -615,7 +615,7 @@ TEST_CASE("csv") { auto ds = get_csv_ds(filename); auto fields = ds->get_descriptor().get_descriptors(); require_field_names(fields, {"x", "y", "z"}); - require_attributes(all_features(ds)->next(), { + REQUIRE_ATTRIBUTES(all_features(ds)->next(), { attr{"x", 1}, attr{"y", 10}, attr{"z", 9999.9999} }); if (mapnik::util::exists(filename + ".index")) { @@ -653,7 +653,7 @@ TEST_CASE("csv") { auto ds = get_csv_ds(filename); auto fields = ds->get_descriptor().get_descriptors(); require_field_names(fields, {"x", "y", "line"}); - require_attributes(all_features(ds)->next(), { + REQUIRE_ATTRIBUTES(all_features(ds)->next(), { attr{"x", 0}, attr{"y", 0} , attr{"line", ustring("many\n lines\n of text\n with unix newlines")} }); if (mapnik::util::exists(filename + ".index")) @@ -684,7 +684,7 @@ TEST_CASE("csv") { auto ds = get_csv_ds(filename); auto fields = ds->get_descriptor().get_descriptors(); require_field_names(fields, {"x", "y", "z"}); - require_attributes(all_features(ds)->next(), { + REQUIRE_ATTRIBUTES(all_features(ds)->next(), { attr{"x", -122}, attr{"y", 48}, attr{"z", 0} }); if (mapnik::util::exists(filename + ".index")) { @@ -719,7 +719,7 @@ TEST_CASE("csv") { auto ds = get_csv_ds(filename); auto fields = ds->get_descriptor().get_descriptors(); require_field_names(fields, {"x", "y", "z"}); - require_attributes(all_features(ds)->next(), { + REQUIRE_ATTRIBUTES(all_features(ds)->next(), { attr{"x", 0}, attr{"y", 0}, attr{"z", ustring("hello")} }); if (mapnik::util::exists(filename + ".index")) { @@ -754,9 +754,9 @@ TEST_CASE("csv") { require_field_types(fields, {mapnik::Integer, mapnik::Integer, mapnik::String, mapnik::Boolean}); auto featureset = all_features(ds); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 0}, attr{"y", 0}, attr{"null", ustring("null")}, attr{"boolean", true}}); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 0}, attr{"y", 0}, attr{"null", ustring("")}, attr{"boolean", false}}); if (mapnik::util::exists(filename + ".index")) @@ -829,11 +829,11 @@ TEST_CASE("csv") { require_field_types(fields, {mapnik::Integer, mapnik::Integer, mapnik::String}); auto featureset = all_features(ds); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 0}, attr{"y", 0}, attr{"fips", ustring("001")}}); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 0}, attr{"y", 0}, attr{"fips", ustring("003")}}); - require_attributes(featureset->next(), { + REQUIRE_ATTRIBUTES(featureset->next(), { attr{"x", 0}, attr{"y", 0}, attr{"fips", ustring("005")}}); if (mapnik::util::exists(filename + ".index")) { @@ -990,7 +990,7 @@ TEST_CASE("csv") { auto fields = ds->get_descriptor().get_descriptors(); require_field_names(fields, {"x", "y", "name"}); require_field_types(fields, {mapnik::Integer, mapnik::Integer, mapnik::String}); - require_attributes(all_features(ds)->next(), { + REQUIRE_ATTRIBUTES(all_features(ds)->next(), { attr{"x", 0}, attr{"y", 0}, attr{"name", ustring("data_name")} }); REQUIRE(count_features(all_features(ds)) == r.second); CHECK(ds->get_geometry_type() == mapnik::datasource_geometry_t::Point); @@ -1007,13 +1007,13 @@ TEST_CASE("csv") { auto fs = all_features(ds); auto feature = fs->next(); - require_attributes(feature, { + REQUIRE_ATTRIBUTES(feature, { attr{"x", 0}, attr{"y", 0}, attr{"bigint", 2147483648} }); feature = fs->next(); - require_attributes(feature, { + REQUIRE_ATTRIBUTES(feature, { attr{"x", 0}, attr{"y", 0}, attr{"bigint", 9223372036854775807ll} }); - require_attributes(feature, { + REQUIRE_ATTRIBUTES(feature, { attr{"x", 0}, attr{"y", 0}, attr{"bigint", 0x7FFFFFFFFFFFFFFFll} }); } // END SECTION #pragma GCC diagnostic pop diff --git a/test/unit/datasource/ds_test_util.hpp b/test/unit/datasource/ds_test_util.hpp index bf377ca83..babec4651 100644 --- a/test/unit/datasource/ds_test_util.hpp +++ b/test/unit/datasource/ds_test_util.hpp @@ -122,11 +122,6 @@ using attr = std::tuple; } \ } while (0) -inline void require_attributes(mapnik::feature_ptr feature, - std::initializer_list const &attrs) { - REQUIRE_ATTRIBUTES(feature, attrs); -} - namespace detail { struct feature_count { template From 482cd025851b486a6520dd226deb3e47422cdffa Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Sat, 30 Jun 2018 15:12:27 +0200 Subject: [PATCH 3/4] datasource tests: fix failing csv test - it was indeed the test that was wrong; the coordinates in nypd.csv are specified with a decimal point, thus should be `value_double` --- test/unit/datasource/csv.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/datasource/csv.cpp b/test/unit/datasource/csv.cpp index 634978c45..2cf074171 100644 --- a/test/unit/datasource/csv.cpp +++ b/test/unit/datasource/csv.cpp @@ -295,8 +295,8 @@ TEST_CASE("csv") { , attr { "Phone", mapnik::value_unicode_string("(212) 334-0711") } , attr { "Address", mapnik::value_unicode_string("19 Elizabeth Street") } , attr { "Precinct", mapnik::value_unicode_string("5th Precinct") } - , attr { "geo_longitude", mapnik::value_integer(-70) } - , attr { "geo_latitude", mapnik::value_integer(40) } + , attr { "geo_longitude", mapnik::value_double(-70.0) } + , attr { "geo_latitude", mapnik::value_double(40.0) } }; REQUIRE_ATTRIBUTES(feature, expected_attr); REQUIRE_ATTRIBUTES(feature2, expected_attr); From de14f920197f4ed5c8c6b37dd543b0cc1dee6a7f Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Sat, 30 Jun 2018 15:26:17 +0200 Subject: [PATCH 4/4] datasource tests: typos --- test/unit/datasource/geojson.cpp | 2 +- test/unit/datasource/topojson.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/datasource/geojson.cpp b/test/unit/datasource/geojson.cpp index c442590be..5b891f53d 100644 --- a/test/unit/datasource/geojson.cpp +++ b/test/unit/datasource/geojson.cpp @@ -834,7 +834,7 @@ TEST_CASE("geojson") { std::initializer_list attrs = { attr{"name", tr.transcode("Test")}, attr{"NOM_FR", tr.transcode("Québec")}, - attr{"boolean", mapnik::value_bool("true")}, + attr{"boolean", mapnik::value_bool(true)}, attr{"description", tr.transcode("Test: \u005C")}, attr{"double", mapnik::value_double(1.1)}, attr{"int", mapnik::value_integer(1)}, diff --git a/test/unit/datasource/topojson.cpp b/test/unit/datasource/topojson.cpp index 7bf155e33..1c1088aa0 100644 --- a/test/unit/datasource/topojson.cpp +++ b/test/unit/datasource/topojson.cpp @@ -92,7 +92,7 @@ TEST_CASE("topojson") std::initializer_list attrs = { attr{"name", tr.transcode("Test")}, attr{"NOM_FR", tr.transcode("Québec")}, - attr{"boolean", mapnik::value_bool("true")}, + attr{"boolean", mapnik::value_bool(true)}, attr{"description", tr.transcode("Test: \u005C")}, attr{"double", mapnik::value_double(1.1)}, attr{"int", mapnik::value_integer(1)},