treee111 / wahooMapsCreator

Create maps for Wahoo device based on latest OSM maps
247 stars 25 forks source link

Fix unittests #220

Closed alfh closed 10 months ago

alfh commented 10 months ago

Thank you for your contribution! 🙌

To get it merged faster, please use this template and replace as many "{...}" as possible and kindly review the checklist below.

This PR…

Update unit tests for tags, to match new tags that are kept. This fixes the test_constants.

Will try to get the test_generated_files to also pass OK, so will update this PR.

Considerations and implementations

Updating the assertions on what tags are present, since the config for tags to keep has changed

{...}

How to test

To avoid that the

land-polygons-split-4326
file is downloaded each time, assume it is present in test/resources. I have not added the file to git, since it is quite big, do you think we should add it ? Also will look if I'm able to find a version from 2021, to match the other static test resouces.

The test_generated_files still fails, need to dig more into that.

Do you know the last version when the test_generated_files where running OK ?

Pull Request Checklist

alfh commented 10 months ago

When looking at the generated files for Lichtenstein, I see this difference using osmium diff for the land.osm file Summary: left=5915 right=0 same=146355 different=0

When I revert the newly added tags from tags-to-keep.json, I see this Summary: left=0 right=0 same=146355 different=0

But still there are differences, I will look further osmium diff -c for land1 shows n22951459325 v1=======================================================] 100% n22951459326 v1 n22951459327 v1 n22951459328 v1 +n22951459329 v1 n22951459330 v1 -n22951459331 v1 n22951459332 v1 n22951459333 v1 +n22951459334 v1 n22951459335 v1 n22951459336 v1 n22951459337 v1 -n22951459338 v1 n22951459339 v1 n22951459340 v1 +n22951459341 v1 n22951459342 v1 -n22951459343 v1 n22951459344 v1 n22951459345 v1 n22951459346 v1 n22951459347 v1 n22951459349 v1 n22951459350 v1 n22951459351 v1 n22951459352 v1 n22951459353 v1 n22951459354 v1 -w22951459329 v1 +w22951459331 v1 -w22951459334 v1 +w22951459338 v1 -w22951459341 v1 +w22951459343 v1 w22951459348 v1 *w22951459355 v1

osmium diff -c for the split-liechtenstein.osm.pbf split-liechtenstein-names.osm.pbf shows no differences, which is good.

alfh commented 10 months ago

For the Lichtenstein, there are differences in the shapefile, land.shp, as well. Using the qgis UI, I see that the differences is only in the sequence of the 6 polygons, the actual polygons have the same coordinates.

Using the ogr2ogr that generates the file, I see that the changes is not due to version differences, I tried with gdal 3.4.0, 3.6.x and 3.7.2, they all generated files with the same sha256sum.

ogr2ogr -progress -overwrite -skipfailures -spat 8.337500 46.940182 9.943750 48.089922 /home/alf/wahooMapsCreatorData/_tiles/134/89/land.shp /home/alf/wahooMapsCreatorData/_download/land-polygons-split-4326/land_polygons.shp

Then I used the "waybackmachine" to find the land-polygon-split zip file from 2021, the only version available was from 2021/05/17. Using that zip file, and the land_polygons.shp file, for running the unittests, actually make the unit test for Lichtenstein to pass.

So for Lichtenstein, the changes seems to only be related to the data in land_polygons files having changed due to OSM data changes, I assume.

alfh commented 10 months ago

@treee111 Is the file size of the land-polygon-split zip file / directory the reason why it is not checked into git ? Or is it license considerations ? But since OSM data files are already checked in, I assume not.

For Malta, I am still investigating the differences, there it did not help with an old land-polygon file.

treee111 commented 10 months ago

Hi @alfh, thanks for looking into the unittests! I think you found out a lot about how processing and the tool itself works - nice! Sorry that I have been so late to reply!

I also had a look into that yesterday and found out the other things letting the unittests break. I listed them here: #222. Feel free to have a look at it and ask if something is not understandable. For me on macOS, unittests run OK now The last thing toDo is to update the static files for Windows to have the unittests run there successful as well.

As I have been the only one running and checking the unittests till now, I have not documented the "howto" and haven't stored the land-poligons online, because the file is too big.

Here is the land-poligons-file as .zip uploaded: https://1drv.ms/u/s!AnpNcYd7Zz7TnXorg_zZuvsbMGsJ?e=fL6zvM Please extract and move to that location: grafik

I will also update the CONTRIBUTION.md to include how to setup Unittests and will let you review it.

treee111 commented 10 months ago

closed in favor of #222. See comments here and in #222.