mapbox / mapnik-vector-tile

Mapnik implemention of Mapbox Vector Tile specification
BSD 3-Clause "New" or "Revised" License
553 stars 117 forks source link

Tests for Invalid or Bad Geometries #153

Closed flippmoke closed 9 years ago

flippmoke commented 9 years ago

We need to have an extensive set of geometries to test against for failures in the mapnik-vector-tile code, the following type of geometries need tests added if they do not exist. The resulting vector tile for each of these geometry types should be validated by checking:

List of geometries to test:

/cc @springmeyer @jakepruitt

flippmoke commented 9 years ago

A list of good tests for each set of data provided:

Each test should test the process of creating a vector tile from the geometry at a set of different clippings. Each clipping is a different bounding box area defined for the tile -- therefore the tile coordinates for each are expected to be different as the view transform will be different. Here is a good set of extents to test for each set of data provided

Each of these bounding boxes should be tested against the following configurations:

For each of these vector tiles created it should be tested against an existing geojson file that is in the tile's coordinate system (integer coordinates). Use https://github.com/mapbox/mapnik-vector-tile/blob/master/src/vector_tile_geometry_decoder.hpp#L542 to decode the geometry from tiles. If they do not match it should throw an error.

Additionally, each tile created should be tested against an expected state of is_valid with reason and is_simple. https://github.com/mapnik/mapnik/blob/master/include/mapnik/geometry_is_valid.hpp#L273 The reason for is_valid failing can be returned in a string or enum (for general type).

Attempt to make updating as simple as possible in a method similar to how it is done with visual tests in mapnik core. For example setting environmental variable update would update the tests? UPDATE=1 make test. This part is not critical and could be added later but is very nice when needing to update the tests due to expected changes.

/cc @jakepruitt

jakepruitt commented 9 years ago

@flippmoke I would love to hear your feedback on the current state of this now that you've had the chance to use it for a few weeks. In terms of the future of these tests, I think we have a few additions that would really help:

  1. Visual comparison of conflicts, either with png rendering a-la mapnik-core visual tests or with custom-made geojson viewer/comparison. I was hoping github would be able to handle this better, but it's a bit janky.
  2. Change from bounding-box specified regions to z/x/y vector-tile specified regions. This would probably improve the length of output file names.

Let me know what the priority is on this and what we can do moving forward! cc/ @springmeyer

flippmoke commented 9 years ago

The only hard part is updating the tests is tedious and takes time to look over all the results -- visual tests would be good but we need some way to note that vertices have changed and visualize winding order... this shouldn't replace the results being geojson IMO. I was thinking perhaps a script to do this in browser might be good and useful in other places -- maybe it would be part of the library data?