Ignore features ordering in test #1675
Conversation
| @@ -410,7 +414,7 @@ jobs: | |||
| tests/test.sh | |||
| - name: Compare test output results (Linux) | |||
| if: matrix.target == 'x86_64-unknown-linux-gnu' | |||
| run: diff --brief --recursive --new-file tests/output tests/expected | |||
| run: diff --brief --recursive --new-file --exclude='*.pbf' tests/output tests/expected | |||
There was a problem hiding this comment.
Features order changed. So no need to diff the *.pbf
|
Or maybe It should be splited to two PRs: |
| @@ -95,7 +95,10 @@ test_pbf() { | |||
|
|
|||
| if [[ $OSTYPE == linux* ]]; then | |||
| ./tests/fixtures/vtzero-check "$FILENAME" | |||
| ./tests/fixtures/vtzero-show "$FILENAME" > "$FILENAME.txt" | |||
| # see https://gdal.org/en/stable/programs/ogrmerge.html#ogrmerge | |||
| ogrmerge.py -o "$FILENAME.geojson" "$FILENAME" -single -src_layer_field_name "source_mvt_layer" -src_layer_field_content "{LAYER_NAME}" -f "GeoJSON" -overwrite_ds | |||
There was a problem hiding this comment.
for users which don't have the right dependency installed this would look like the tests not being able to run.
For the other dependencys, we can reasonably assume that they are already insstalled ^^
As a fresh dev, a missing python file might look like a bug and lead to me skipping testing..
=> Could you add a check at the start of the test that bails if ogrmerge.py is not installed and advises the dev to do so?
| @@ -28,7 +28,7 @@ jobs: | |||
| PGPASSWORD: postgres | |||
| services: | |||
| postgres: | |||
| image: postgis/postgis:14-3.3 | |||
| image: postgis/postgis:15-3.4 | |||
There was a problem hiding this comment.
To be on the same page after merging:
Would there something blocking us from updating to 3.5 and isssuing warnings to <3.5?
From what I have seen, the 3.5 results seem more correct..
(lets do this in a sepaate PR, if desired)
There was a problem hiding this comment.
The feature order changed in v3.4 and MBTiles content changed(related issue here) in v3.5.
I planned to fix them one by one ( for simplicity) rather than directly upgrading to v3.5 in one single step.
But you inspired me, on second thought, I think we could upgrade to 3.5 directly since we have aligned the PostGIS versions in both docker-compose.yml and ci.yml, so the results of local and ci should be same.
There was a problem hiding this comment.
For possibly excessive caution and make this PR dedicated, let's address this in a separate PR? ^^
tests/test.sh
Outdated
| # Verify the tools used in the tests are available | ||
| # todo add more verification for other tools like jq file curl sqlite3... | ||
| if ! command -v ogrmerge.py > /dev/null; then | ||
| echo "gdal-bin is required for testing" | ||
| echo "For Ubuntu, you could install it with sudo apt update && sudo apt install gdal-bin -y." | ||
| echo "see more at https://gdal.org/en/stable/download.html#binaries" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Yeah you are right. And maybe we should add more. @CommanderStorm
Try to fix #1651
*.pbfas the features ordering has changedogrmerge of gdalto generate the description text of*.pbffilesfunction_Mixed_Name.sqlforjqsortingtest.sh