roelderickx / ogr2osm

A tool for converting ogr-readable files like shapefiles into .pbf or .osm data
https://pypi.org/project/ogr2osm/
MIT License
59 stars 14 forks source link

Tests fail #27

Closed SlowMo24 closed 1 year ago

SlowMo24 commented 1 year ago

If run the tests in the current master and two of them fail. I ran the test as described in my PR: #26

!
--- test/basic_usage.t
+++ test/basic_usage.t.err
@@ -20,7 +20,7 @@
                           connection string such as: "PG:dbname=pdx_bldgs
                           user=emma host=localhost" (including the quotes)

-  optional arguments:
+  options:
     -h, --help            show this help message and exit
     --version             show program's version number and exit
     -t TRANSLATION, --translation TRANSLATION
..!
..!
--- test/pbf_output_no_protobuf.t
+++ test/pbf_output_no_protobuf.t.err
@@ -24,6 +24,355 @@
   Writing nodes
   Writing ways
   Writing relations
-  Writing file footer
   $ xmllint --format basic_geometries.osm | diff -uNr - $TESTDIR/basic_geometries.xml
+  warning: failed to load external entity "basic_geometries.osm"
+  --- -\t2022-12-02 11:33:54.278496308 +0000 (esc)
+  +++ /home/moritz/Schreibtisch/ogr2osm/test/basic_geometries.xml\t2022-12-02 09:49:54.164045091 +0000 (esc)
+  @@ -0,0 +1,345 @@
+  +<?xml version="1.0" encoding="UTF-8"?>
+  +<osm version="0.6" generator="ogr2osm 1.1.2" upload="false">
+  +  <node visible="true" id="-1" lat="33.4838" lon="-23.094721">
...
+  +</osm>
+  [1]
roelderickx commented 1 year ago

Hi Moritz,

First of all thanks for your report.

If run the tests in the current master and two of them fail. I ran the test as described in my PR: #26

basic_usage.t -> I guess the help message has changed

!
--- test/basic_usage.t
+++ test/basic_usage.t.err
@@ -20,7 +20,7 @@
                           connection string such as: "PG:dbname=pdx_bldgs
                           user=emma host=localhost" (including the quotes)

-  optional arguments:
+  options:
     -h, --help            show this help message and exit
     --version             show program's version number and exit
     -t TRANSLATION, --translation TRANSLATION
..!

I see. It is probably due to the version of argparse, on my machine it says options but in the docker image it is optional arguments. Since the docker image mimics the github workflow environment we have to keep optional arguments for now.

pbf_output_noProtobuf.t -> this test requires the pbf library not to be installed while pbf_output.t requires it to be installed. So one of them will always fail.

..!
--- test/pbf_output_no_protobuf.t
+++ test/pbf_output_no_protobuf.t.err
@@ -24,6 +24,355 @@
   Writing nodes
   Writing ways
   Writing relations
-  Writing file footer
   $ xmllint --format basic_geometries.osm | diff -uNr - $TESTDIR/basic_geometries.xml
+  warning: failed to load external entity "basic_geometries.osm"
+  --- -\t2022-12-02 11:33:54.278496308 +0000 (esc)
+  +++ /home/moritz/Schreibtisch/ogr2osm/test/basic_geometries.xml\t2022-12-02 09:49:54.164045091 +0000 (esc)
+  @@ -0,0 +1,345 @@
+  +<?xml version="1.0" encoding="UTF-8"?>
+  +<osm version="0.6" generator="ogr2osm 1.1.2" upload="false">
+  +  <node visible="true" id="-1" lat="33.4838" lon="-23.094721">
...
+  +</osm>
+  [1]

This is correct behaviour. What I want to test is that the optional requirements really are optional, most people do not need PBF output. The github workflow runs this test before installing the PBF requirements to verify the fallback to OSM output, which you can distinguish by looking at the Writing file footer message. In your case the PBF libraries are clearly installed, the message in question is absent and a PBF file is generated while the script expects an OSM file.

The most important tests are osm_output.t and pbf_output.t, if these fail then the functionality has been broken somehow.

SlowMo24 commented 1 year ago

Hey, thanks, that explains it. I adapted the documentation in the PR in hope that any future new contributor will have all necessary info.

roelderickx commented 1 year ago

This is idd a better way to explain how to set up the test environment. If it changes at some point we don't need to modify it here again.