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.

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

Integration test error fixes #2016

Merged
merged 7 commits into from
Dec 1, 2021
Merged

Integration test error fixes #2016

merged 7 commits into from
Dec 1, 2021

Conversation

jeffdefacto
Copy link
Contributor

Updated integration tests to fix errors

@jeffdefacto jeffdefacto changed the title Initial commit of fixes to tests Integration test error fixes Nov 25, 2021
@nvkelso
Copy link
Member

nvkelso commented Nov 29, 2021

These are the still failing tests:

FAIL: test_cycleway_with_international_route (integration-test.1170-very-early-paths-and-bike-routes.VeryEarlyPathsAndBikeRoutes)
FAIL: test_path_with_international_route (integration-test.1170-very-early-paths-and-bike-routes.VeryEarlyPathsAndBikeRoutes)
FAIL: test_path_with_national_route (integration-test.1170-very-early-paths-and-bike-routes.VeryEarlyPathsAndBikeRoutes)
FAIL: test_full_lists_disappear_by_zoom_12 (integration-test.1194-bus-route-refs.BusRouteRefs)
FAIL: test_driveway_nwn (integration-test.596-add-hiking-routes.AddHikingRoutes)
FAIL: test_minor_road_nwn (integration-test.596-add-hiking-routes.AddHikingRoutes)
FAIL: test_service_nwn (integration-test.596-add-hiking-routes.AddHikingRoutes)
FAIL: test_drop_sea_polygon_but_keep_label (integration-test.951-remove-sea.RemoveSea)

Additionally, Jeff just merged the latest changes into his branch and this one is failing now. It's already a fixture and just a min zoom mismatch so its possibly from a code change elsewhere.

FAIL: test_tidal_marsh (integration-test.1800-limit-marshes.MarshTest)

@jeffdefacto
Copy link
Contributor Author

@nvkelso
Copy link
Member

nvkelso commented Nov 30, 2021

FAIL: test_drop_sea_polygon_but_keep_label (integration-test.951-remove-sea.RemoveSea)

https://github.com/tilezen/vector-datasource/blob/error_fixes/integration-test/951-remove-sea.py

For this one it's geometry is very large. Consider making a box_area dsl fixture with a similar area, most the key,value pairs and predictable location for the label placement.

"name":		"Αιγαίον Πέλαγος / Ege Denizi"
"name:el":	"Αιγαίο Πέλαγος"
"name:en":	"Aegean Sea"
"name:tr":	"Ege Denizi"
"place":	"sea"
"wikidata":	"Q34575"

Comparable such test:

  • def test_adirondack(self):
    import dsl
    z, x, y = (8, 75, 93)
    self.generate_fixtures(
    # https://www.openstreetmap.org/relation/1695394
    dsl.way(1695394, dsl.box_area(z, x, y, 45506933692), {
    'boundary': 'national_park',
    'name': 'Adirondack Park',
    'source': 'openstreetmap.org',
    'type': 'multipolygon',
    'wikidata': 'Q357550',
    'wikipedia': 'en:Adirondack Park',
    }),
    )
    self.assert_has_feature(
    z, x, y, 'pois', {
    'id': 1695394,
    'kind': 'park',
    'min_zoom': 8,
    })

@nvkelso
Copy link
Member

nvkelso commented Nov 30, 2021

For these 3 tests in the single 596-add-hiking-routes.py file:

FAIL: test_driveway_nwn (integration-test.596-add-hiking-routes.AddHikingRoutes)
FAIL: test_minor_road_nwn (integration-test.596-add-hiking-routes.AddHikingRoutes)
FAIL: test_service_nwn (integration-test.596-add-hiking-routes.AddHikingRoutes)

The zoom range is controlled via this logic:

CREATE OR REPLACE FUNCTION mz_calculate_path_major_route(osm_id BIGINT, dummy INTEGER)
RETURNS SMALLINT AS $$
BEGIN
RETURN (
SELECT
MIN(
CASE WHEN hstore(tags)->'network' IN ('icn', 'ncn') THEN 8
WHEN hstore(tags)->'network' IN ('iwn', 'nwn') THEN 9
WHEN hstore(tags)->'network' IN ('rcn') THEN 11
WHEN hstore(tags)->'network' IN ('rwn') THEN 11
WHEN hstore(tags)->'network' IN ('lcn') THEN 12
WHEN hstore(tags)->'network' IN ('lwn') THEN 12
ELSE NULL
END

National walking networks (nwn) should come in at zoom 9 regardless of their kind, so a test at zoom 11 should certainly return a feature. So are these testing the wrong tile?

Suggest turning test_driveway_nwn into simple tile_diagnal and relation dsl features inside a given tile, then testing again. (Versus figuring out what tile to test for for that geometry.)

For test_minor_road_nwn it looks like you've already done this trick, but you're setting the way's ID inconsistently, I'll comment in the PR.

For test_service_nwn it's not immediately obvious to me why that test is failing.

@nvkelso
Copy link
Member

nvkelso commented Nov 30, 2021

FAIL: test_full_lists_disappear_by_zoom_12 (integration-test.1194-bus-route-refs.BusRouteRefs)

https://github.com/tilezen/vector-datasource/blob/error_fixes/integration-test/1194-bus-route-refs.py#L164

Given the previous tests in the file pass, I suspect that tile isn't the right one. Convert to tile_diagnal for that tile and try again.

@nvkelso
Copy link
Member

nvkelso commented Nov 30, 2021

For:

FAIL: test_cycleway_with_international_route (integration-test.1170-very-early-paths-and-bike-routes.VeryEarlyPathsAndBikeRoutes)

Try a tile_diagnal for the way at 13, 4211, 2670 tile coordinates, and that zoom 8 coord (which is valid for icn) looks right to me (is the parent of that zoom 13 tile). Everything else smells right.

FAIL: test_path_with_international_route (integration-test.1170-very-early-paths-and-bike-routes.VeryEarlyPathsAndBikeRoutes)

Try a tile_diagnal for the way at 15, 17003, 11507 tile coordinates, and that zoom 9 coord (which is valid for iwn) looks right to me (is the parent of that zoom 15 tile). Everything else smells right.

FAIL: test_path_with_national_route (integration-test.1170-very-early-paths-and-bike-routes.VeryEarlyPathsAndBikeRoutes)

You're already doing a tile_diagnal (though at zoom 9, which is not wrong but novel). Not sure about this one.

@nvkelso
Copy link
Member

nvkelso commented Nov 30, 2021

FAIL: test_tidal_marsh (integration-test.1800-limit-marshes.MarshTest)

The logic for marshes in the landuse layer is in:

Which is using tier2_min_zoom:

Since we're setting the area to 89099, so it should indeed be zoom 13 not 14.

The test wasn't updated as part of #1996, because finding the failing test was hard (thanks for fixing this). Just change 14 to 13 in the test and you should be good to go.

@nvkelso
Copy link
Member

nvkelso commented Nov 30, 2021

FAIL: test_tourism (integration-test.440-zoos-and-other-attractions-tourism.ZoosAndOtherAttractionsTourism)

High Hampton Resort

self._run_test(14, 4410, 6484, -12775348, 'resort')

Logic:

Which means the point label for that polygon feature in the pois layer should appear sometime between zoom 14 and zoom 17 based on the area. It seems reasonable that it be at zoom 14, in either the tile you're testing or the tile just north.

But to calc the actual min_zoom you need to know the area and adjust.

Reviewing the prior state:

Disneyland Resort

self._run_test(14, 2825, 6555, -4795362, 'resort')

That was slightly bigger so could be a different zoom on your new feature.

Digging into this more, it looks like tourism=yes on the Disneyland Resort, and it has migrated to leisure=resort, and it's noted on that page that tourism=resort is no longer valid OSM tagging, and TagInfo reports that very few features have that tag anymore (features were migrated to new tags).

So I think the old Disneyland Resort test is actually good (restore it), but change the logic in:

And related in:

So it reflects current OSM tagging best practices.

'http://www.openstreetmap.org/relation/1544944',
], clip=self.tile_bbox(11, 491, 762))
self.generate_fixtures(
dsl.way(16000421, wkt_loads('LineString (-93.68584719999999777 41.74244529999999997, -93.6859489999999937 41.74249919999999747, -93.68607550000000117 41.74251509999999854, -93.68619499999999789 41.74251499999999737, -93.68646250000000464 41.74245650000000296, -93.68669930000000079 41.74234510000000142, -93.68697509999999795 41.7422196999999997, -93.6872615000000053 41.74211700000000036, -93.68774890000000255 41.7419809000000015)'), {
Copy link
Member

Choose a reason for hiding this comment

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

Try a simple tile_diagnal and relation dsl features inside a given tile, then testing again. (Versus figuring out what tile to test for for that geometry.)

Copy link
Member

Choose a reason for hiding this comment

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

Inside tile 11, 491, 762 (what you're testing below) or a child tile.

'http://www.openstreetmap.org/relation/2980504'])

self.generate_fixtures(
dsl.way(225516711, wkt_loads('LineString (-122.44117830000000424 37.79163929999999993, -122.44752560000000585 37.79082819999999998)'), {
Copy link
Member

Choose a reason for hiding this comment

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

Consider a tile_diagnal for this feature as well (rather than searching for right parent tile).

import dsl
from shapely.wkt import loads as wkt_loads
self.generate_fixtures(
dsl.way(35568189, wkt_loads('LineString (5.20611110000000021 53.0203684000000024, 5.20225570000000026 53.01814749999999776, 5.20133840000000003 53.01767960000000102, 5.20109159999999981 53.01755539999999911, 5.2000295000000003 53.01723590000000286, 5.19848179999999971 53.01643560000000122, 5.19116390000000028 53.01260760000000261, 5.17956600000000034 53.00652889999999928, 5.15636820000000018 52.99436250000000115, 5.14245929999999962 52.98704740000000157, 5.13548730000000031 52.98336119999999738, 5.12635620000000003 52.97859449999999981, 5.12550750000000033 52.97814499999999782, 5.12357559999999967 52.97711340000000035, 5.11780650000000037 52.9740553000000034, 5.11234319999999975 52.97123249999999928, 5.11137360000000029 52.97082439999999792, 5.11007820000000024 52.97023440000000249, 5.10912040000000012 52.96974740000000281, 5.10839659999999984 52.96936840000000046, 5.1071093000000003 52.96868380000000087, 5.10643549999999991 52.96833000000000169, 5.10612449999999995 52.96815589999999929, 5.10582579999999986 52.96797099999999858, 5.1038036 52.96672759999999869, 5.10248640000000009 52.9660340000000005, 5.09509550000000022 52.9621532999999971, 5.08968869999999995 52.95931259999999696, 5.07287130000000008 52.95045970000000324, 5.06300279999999958 52.94526559999999904, 5.06257420000000025 52.94504040000000344, 5.06130849999999999 52.94437419999999861, 5.05951370000000011 52.94342830000000077, 5.05829210000000007 52.94278479999999831, 5.05619499999999977 52.94168160000000256, 5.05398130000000023 52.94051449999999903, 5.05171740000000025 52.93933280000000252, 5.05093960000000042 52.93901509999999888, 5.05027440000000016 52.93871359999999981, 5.04894219999999994 52.9380398999999997)'), {
Copy link
Member

Choose a reason for hiding this comment

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

Try a tile_diagnal for the way at 13, 4211, 2670 tile coordinates.

import dsl
from shapely.wkt import loads as wkt_loads
self.generate_fixtures(
dsl.way(285975282, wkt_loads('LineString (6.81894570000000044 47.14029759999999669, 6.81851650000000031 47.1398889999999966, 6.81791570000000036 47.1393051000000014, 6.81695009999999968 47.13809369999999888, 6.81658529999999985 47.13749519999999649, 6.8161132999999996 47.13699890000000181, 6.81561970000000006 47.13641510000000068, 6.81482580000000038 47.13607929999999868, 6.81381730000000019 47.13587499999999864, 6.8127658999999996 47.13588959999999872, 6.81134970000000006 47.13583119999999838, 6.81029820000000008 47.13587499999999864, 6.80961160000000021 47.13584579999999846, 6.80901080000000025 47.1359187999999989, 6.80849689999999974 47.13581949999999665, 6.80768039999999974 47.1355829999999969, 6.80735850000000031 47.13532029999999651, 6.80708070000000021 47.13485610000000037, 6.8068660999999997 47.13430139999999824, 6.80675879999999989 47.13235989999999731, 6.80665150000000008 47.13177600000000211, 6.80645840000000035 47.13138190000000094, 6.80617840000000029 47.13121840000000162, 6.80469889999999999 47.13091469999999816, 6.80328260000000018 47.13079799999999864, 6.80210250000000016 47.13056439999999725, 6.80059940000000029 47.13007970000000313, 6.79980540000000033 47.12968560000000195, 6.7990984000000001 47.12946949999999902, 6.7982604999999996 47.12921839999999918, 6.79748909999999995 47.12930889999999806, 6.79701700000000031 47.12927969999999789, 6.79637329999999995 47.12910449999999685, 6.7960718 47.12891179999999736, 6.79574989999999968 47.12878039999999658, 6.79524569999999972 47.12887529999999714, 6.79487229999999975 47.128579000000002, 6.79409979999999969 47.12815559999999948, 6.79306989999999988 47.12755700000000303)'), {
Copy link
Member

Choose a reason for hiding this comment

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

Try a tile_diagnal for the way at 15, 17003, 11507 tile coordinates

@jeffdefacto
Copy link
Contributor Author

@nvkelso I made the couple of corrections and converted the requested tests to tile_diagonal fixtures. Still no luck clearing out the errors though.

FAIL: test_tourism (integration-test.440-zoos-and-other-attractions-tourism.ZoosAndOtherAttractionsTourism)

Both Disney World and Disneyland are failing this check currently. Disney World contains both leisure=resort and tourism=theme_park. I'm not sure if that would cause an issue when assigning kind. Disneyland had some wonky geometry and tagging that I thought could be causing issues but still fails even after I cleared that up.

@nvkelso
Copy link
Member

nvkelso commented Nov 30, 2021

Down to just 8 failures (down from 60)!

FAIL: test_cycleway_with_international_route (integration-test.1170-very-early-paths-and-bike-routes.VeryEarlyPathsAndBikeRoutes)
AssertionError: Did not find feature including properties {'kind': 'path', 'is_bicycle_related': True, 'bicycle_network': 'icn'} (because layer 'roads' was empty)

======================================================================
FAIL: test_path_with_international_route (integration-test.1170-very-early-paths-and-bike-routes.VeryEarlyPathsAndBikeRoutes)
AssertionError: Did not find feature including properties {'kind': 'path', 'walking_network': 'iwn'} (because layer 'roads' was empty)

======================================================================
FAIL: test_path_with_national_route (integration-test.1170-very-early-paths-and-bike-routes.VeryEarlyPathsAndBikeRoutes)
AssertionError: Did not find feature including properties {'kind': 'path', 'walking_network': 'nwn'} (because layer 'roads' was empty)

======================================================================
FAIL: test_full_lists_disappear_by_zoom_12 (integration-test.1194-bus-route-refs.BusRouteRefs)
AssertionError: Did not find feature including properties {'bus_network': <type 'NoneType'>, 'bus_shield_text': None} (because layer 'roads' was empty)

======================================================================
FAIL: test_tourism (integration-test.440-zoos-and-other-attractions-tourism.ZoosAndOtherAttractionsTourism)
AssertionError: Did not find feature including properties {'kind': 'theme_park', 'id': -1228099} (because layer 'pois' was empty)

======================================================================
FAIL: test_driveway_nwn (integration-test.596-add-hiking-routes.AddHikingRoutes)
AssertionError: Did not find feature including properties {'kind_detail': 'service', 'kind': 'minor_road', 'walking_network': 'nwn', 'service': 'driveway'} (because layer 'roads' was empty)

======================================================================
FAIL: test_minor_road_nwn (integration-test.596-add-hiking-routes.AddHikingRoutes)
AssertionError: Did not find feature including properties {'kind_detail': 'residential', 'kind': 'minor_road', 'walking_network': 'nwn'} (because layer 'roads' was empty)

======================================================================
FAIL: test_service_nwn (integration-test.596-add-hiking-routes.AddHikingRoutes)
AssertionError: Did not find feature including properties {'kind_detail': 'service', 'kind': 'minor_road', 'walking_network': 'nwn'} (because layer 'roads' was empty)

@nvkelso
Copy link
Member

nvkelso commented Nov 30, 2021

FAIL: test_tourism (integration-test.440-zoos-and-other-attractions-tourism.ZoosAndOtherAttractionsTourism)
Both Disney World and Disneyland are failing this check currently. Disney World contains both leisure=resort and tourism=theme_park. I'm not sure if that would cause an issue when assigning kind. Disneyland had some wonky geometry and tagging that I thought could be causing issues but still fails even after I cleared that up.

I'll have a closer look at this one. Because of the contention between the tags, the order of the YAML filters matters, too.

@nvkelso
Copy link
Member

nvkelso commented Nov 30, 2021

Let's consider reviewing the fixes in this PR and then making a new PR for any followup to keep things moving.

@jeffdefacto jeffdefacto marked this pull request as ready for review November 30, 2021 22:40
@jeffdefacto
Copy link
Contributor Author

Sounds good. Marked for review now.

Copy link
Contributor

@peitili peitili left a comment

Choose a reason for hiding this comment

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

thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants