From be37f3a5659d785668aec7ec50901bca3021e137 Mon Sep 17 00:00:00 2001 From: Brian Sperlongano Date: Thu, 28 Oct 2021 07:38:46 -0400 Subject: [PATCH] Framework for SQL-based unit tests for import and updates (#1249) This PR adds the ability to create SQL tests that ensure that OSM data is properly imported and updated in the OpenMapTiles data schema. The tests work by injecting test OSM data and updates into the database and checking to ensure that the data is properly loaded into the database using standard SQL statements. With this framework in place, developers can now write small tests to inject known data into the database and ensure that imports and updates are working correctly. In addition to the framework, basic tests are provided for four layers. These initial tests are in no way comprehensive, but they provide a structure and framework for developers to add their own tests or expand the existing ones to cover more cases. Usage: `make clean && make sql-test` ## How it works The SQL tests consist of the following parts: 1. **Test import data**, located in `tests/import`. This test data is in the [OSM XML](https://wiki.openstreetmap.org/wiki/OSM_XML) format and contains the data that should be initially injected into the database. The files are numbered in order to ensure that each test data file contains OSM id numbers that are distinct from the other files. For example, the file starting with `100` will use node ids from 100000-199999, way ids from 1000-1999, and relation ids from 100-199. 1. **Test update data**, located in `tests/update`. This test data is in the [osmChange XML](https://wiki.openstreetmap.org/wiki/OsmChange) format, and contains the data that will be used to update the test import data (in order to verify that the update process is working correctly. These files are also numbered using the same scheme as the test import data. 1. **Import SQL test script**, located at `tests/test-post-import.sql`. This script is executed after the test import data has been injected, and runs SQL-based checks to ensure that the import data was properly imported. If there are failures in the tests, an entry will be added to the table `omt_test_failures`, with one record per error that occurs during the import process. A test failure will also fail the build. To inspect the test failure messages, run `make psql` and issue the comment `SELECT * FROM omt_test_failures`. 1. **Update SQL test script**, located at `tests/test-post-update.sql`. This script performs the same function as the import test script, except that it occurs after the test update data has been applied to the database. Note that script will only run if the import script passes all tests. --- .github/workflows/sql-tests.yml | 22 +++++++ CONTRIBUTING.md | 4 ++ Makefile | 47 +++++++++++++- TESTING.md | 18 ++++++ tests/changes.repl.json | 4 ++ tests/changes.state.txt | 3 + tests/import/100_import-large-park.osm | 43 +++++++++++++ tests/import/200_import-aerodrome.osm | 29 +++++++++ tests/import/300_import-landcover.osm | 15 +++++ tests/import/400_import-boundary.osm | 47 ++++++++++++++ tests/last.state.txt | 3 + tests/test-post-import.sql | 88 ++++++++++++++++++++++++++ tests/test-post-update.sql | 67 ++++++++++++++++++++ tests/update/100_update-large-park.osc | 26 ++++++++ tests/update/200_update-aerodrome.osc | 21 ++++++ tests/update/300_update-landcover.osc | 17 +++++ tests/update/400_update-boundary.osc | 29 +++++++++ 17 files changed, 482 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/sql-tests.yml create mode 100644 TESTING.md create mode 100644 tests/changes.repl.json create mode 100644 tests/changes.state.txt create mode 100644 tests/import/100_import-large-park.osm create mode 100644 tests/import/200_import-aerodrome.osm create mode 100644 tests/import/300_import-landcover.osm create mode 100644 tests/import/400_import-boundary.osm create mode 100644 tests/last.state.txt create mode 100644 tests/test-post-import.sql create mode 100644 tests/test-post-update.sql create mode 100644 tests/update/100_update-large-park.osc create mode 100644 tests/update/200_update-aerodrome.osc create mode 100644 tests/update/300_update-landcover.osc create mode 100644 tests/update/400_update-boundary.osc diff --git a/.github/workflows/sql-tests.yml b/.github/workflows/sql-tests.yml new file mode 100644 index 00000000..b301f055 --- /dev/null +++ b/.github/workflows/sql-tests.yml @@ -0,0 +1,22 @@ +# Workflow to run unit tests on OMT`s new Pull Requests and commits pushed into OMT repo + +name: OpenMapTiles SQL Test CI + +on: + push: + branches: [ master, master-tools ] + pull_request: + +jobs: + + unit_tests: + name: Run unit test + runs-on: ubuntu-latest + steps: + + - name: Checkout the changes + uses: actions/checkout@v2 + + - name: Run unit tests + run: | + make clean && make test-sql diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 535f3c16..0f7d7e6e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -41,3 +41,7 @@ When you modify import data rules in `mapping.yaml` or `*.sql`, please update: 5. check if OMT styles are affected by the PR and if there is a need for style updates When you are making PR that adds new spatial features to OpenMapTiles schema, please make also PR for at least one of our GL styles to show it on the map. Visual check is crucial. + +# Testing + +It is recommended that you create a [unit test](TESTING.md) when modifying the behavior of the SQL layer. This will ensure that your changes are working as expected when importing or updating OSM data into an OpenMapTiles database. diff --git a/Makefile b/Makefile index e6272228..f431430c 100644 --- a/Makefile +++ b/Makefile @@ -184,6 +184,7 @@ Hints for developers: make generate-qa # statistics for a given layer's field make generate-tiles-pg # generate vector tiles based on .env settings using PostGIS ST_MVT() make generate-tiles # generate vector tiles based on .env settings using Mapnik (obsolete) + make test-sql # run unit tests on the OpenMapTiles SQL schema cat .env # list PG database and MIN_ZOOM and MAX_ZOOM information cat quickstart.log # transcript of the last ./quickstart.sh run make help # help about available commands @@ -276,9 +277,14 @@ ifeq (,$(wildcard build/sql/run_last.sql)) endif .PHONY: clean -clean: +clean: clean-test-data rm -rf build +clean-test-data: + rm -rf data/changes.state.txt + rm -rf data/last.state.txt + rm -rf data/changes.repl.json + .PHONY: destroy-db # TODO: Use https://stackoverflow.com/a/27852388/177275 destroy-db: DC_PROJECT := $(shell echo $(DC_PROJECT) | tr A-Z a-z) @@ -589,3 +595,42 @@ debug: ## Use this target when developing Makefile itself to verify loaded envi @echo BBOX = $(BBOX) , $$BBOX @echo MIN_ZOOM = $(MIN_ZOOM) , $$MIN_ZOOM @echo MAX_ZOOM = $(MAX_ZOOM) , $$MAX_ZOOM + +build/import-tests.osm.pbf: init-dirs + $(DOCKER_COMPOSE) $(DC_CONFIG_CACHE) run $(DC_OPTS_CACHE) openmaptiles-tools sh -c 'osmconvert tests/import/*.osm -o=build/import-tests.osm.pbf' + +data/changes.state.txt: + cp -f tests/changes.state.txt data/ + +data/last.state.txt: + cp -f tests/last.state.txt data/ + +data/changes.repl.json: + cp -f tests/changes.repl.json data/ + +data/changes.osc.gz: init-dirs + @echo " UPDATE unit test data..." + $(DOCKER_COMPOSE) $(DC_CONFIG_CACHE) run $(DC_OPTS_CACHE) openmaptiles-tools sh -c 'osmconvert tests/update/*.osc --merge-versions -o=data/changes.osc && gzip -f data/changes.osc' + +test-sql: clean refresh-docker-images destroy-db start-db-nowait build/import-tests.osm.pbf data/changes.state.txt data/last.state.txt data/changes.repl.json build/mapping.yaml data/changes.osc.gz + $(eval area := changes) + + @echo "Load IMPORT test data" + sed -ir "s/^[#]*\s*MAX_ZOOM=.*/MAX_ZOOM=14/" .env + sed -ir "s/^[#]*\s*DIFF_MODE=.*/DIFF_MODE=false/" .env + $(DOCKER_COMPOSE) $(DC_CONFIG_CACHE) run $(DC_OPTS_CACHE) openmaptiles-tools sh -c 'pgwait && import-osm build/import-tests.osm.pbf' + + @echo "Apply OpenMapTiles SQL schema to test data @ Zoom 14..." + $(DOCKER_COMPOSE) run $(DC_OPTS) openmaptiles-tools sh -c 'pgwait && import-sql' | \ + awk -v s=": WARNING:" '1{print; fflush()} $$0~s{print "\n*** WARNING detected, aborting"; exit(1)}' | \ + awk '1{print; fflush()} $$0~".*ERROR" {txt=$$0} END{ if(txt){print "\n*** ERROR detected, aborting:"; print txt; exit(1)} }' + + @echo "Test SQL output for Import Test Data" + $(DOCKER_COMPOSE) $(DC_CONFIG_CACHE) run $(DC_OPTS_CACHE) openmaptiles-tools sh -c 'pgwait && psql.sh < tests/test-post-import.sql' + + @echo "Run UPDATE process on test data..." + sed -ir "s/^[#]*\s*DIFF_MODE=.*/DIFF_MODE=true/" .env + $(DOCKER_COMPOSE) $(DC_CONFIG_CACHE) run $(DC_OPTS_CACHE) openmaptiles-tools sh -c 'pgwait && import-diff' + + @echo "Test SQL output for Update Test Data" + $(DOCKER_COMPOSE) $(DC_CONFIG_CACHE) run $(DC_OPTS_CACHE) openmaptiles-tools sh -c 'pgwait && psql.sh < tests/test-post-update.sql' diff --git a/TESTING.md b/TESTING.md new file mode 100644 index 00000000..7248e523 --- /dev/null +++ b/TESTING.md @@ -0,0 +1,18 @@ + +# OpenMapTiles SQL Testing + +The OpenMapTiles SQL tests ensure that OSM data is properly imported and updated in the OpenMapTiles data schema. The tests work by injecting test OSM data into the database and checking to ensure that the data is properly reflected in the SQL output. + +Usage: + +`make clean && make sql-test` + +## How it works + +The SQL tests consist of the following parts: + + 1. **Test import data**, located in `tests/import`. This test data is in the [OSM XML](https://wiki.openstreetmap.org/wiki/OSM_XML) format and contains the data that should be initially injected into the database. The files are numbered in order to ensure that each test data file OSM id numbers that are unique from the other files. For example, the file starting with `100` will use node ids from 100000-199999, way ids from 1000-1999, and relation ids from 100-199. + 2. **Test update data**, located in `tests/update`. This test data is in the [osmChange XML](https://wiki.openstreetmap.org/wiki/OsmChange) format, and contains the data that will be used to update the test import data (in order to verify that the update process is working correctly. These files are also numbered using the same scheme as the test import data. + 3. **Import SQL test script**, located at `tests/test-post-import.sql`. This script is executed after the test import data has been injected, and runs SQL-based checks to ensure that the import data was properly imported. If there are failures in the tests, an entry will be added to the table `omt_test_failures`, with one record per error that occurs during the import process. A test failure will also fail the build. To inspect the test failure messages, run `make psql` and issue the comment `SELECT * FROM omt_test_failures`. + 4. **Update SQL test script**, located at `tests/test-post-update.sql`. This script performs the same function as the import test script, except that it occurs after the test update data has been applied to the database. Note that script will only run if the import script passes all tests. + diff --git a/tests/changes.repl.json b/tests/changes.repl.json new file mode 100644 index 00000000..7805b994 --- /dev/null +++ b/tests/changes.repl.json @@ -0,0 +1,4 @@ +{ + "replication_interval": "24h", + "replication_url": "dummy" +} diff --git a/tests/changes.state.txt b/tests/changes.state.txt new file mode 100644 index 00000000..630648ae --- /dev/null +++ b/tests/changes.state.txt @@ -0,0 +1,3 @@ +#Sat Sep 25 23:23:00 UTC 2021 +sequenceNumber=4730693 +timestamp=2021-09-25T23\:22\:58Z diff --git a/tests/import/100_import-large-park.osm b/tests/import/100_import-large-park.osm new file mode 100644 index 00000000..79740b4a --- /dev/null +++ b/tests/import/100_import-large-park.osm @@ -0,0 +1,43 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/import/200_import-aerodrome.osm b/tests/import/200_import-aerodrome.osm new file mode 100644 index 00000000..0667a36e --- /dev/null +++ b/tests/import/200_import-aerodrome.osm @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/import/300_import-landcover.osm b/tests/import/300_import-landcover.osm new file mode 100644 index 00000000..19cf6db0 --- /dev/null +++ b/tests/import/300_import-landcover.osm @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + diff --git a/tests/import/400_import-boundary.osm b/tests/import/400_import-boundary.osm new file mode 100644 index 00000000..fda6bee1 --- /dev/null +++ b/tests/import/400_import-boundary.osm @@ -0,0 +1,47 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/last.state.txt b/tests/last.state.txt new file mode 100644 index 00000000..a2a0c28a --- /dev/null +++ b/tests/last.state.txt @@ -0,0 +1,3 @@ +#Sat Sep 25 23:23:00 UTC 2021 +sequenceNumber=4730692 +timestamp=2021-09-25T23\:21\:58Z diff --git a/tests/test-post-import.sql b/tests/test-post-import.sql new file mode 100644 index 00000000..11d4a303 --- /dev/null +++ b/tests/test-post-import.sql @@ -0,0 +1,88 @@ +-- Store test results + +DROP TABLE IF EXISTS omt_test_failures; +CREATE TABLE omt_test_failures( + test_id integer, + test_type varchar(6), + error_message text +); + +-- Checks to ensure that test data was imported correctly +DO $$ + +DECLARE + cnt integer; + +BEGIN + + -- Test 100 + SELECT COUNT(*) INTO cnt FROM osm_park_polygon; + IF cnt <> 3 THEN + INSERT INTO omt_test_failures VALUES(100, 'import', 'osm_park_polygon expected 3, got ' || cnt); + END IF; + + SELECT COUNT(*) INTO cnt FROM osm_park_polygon_gen_z5; + IF cnt <> 3 THEN + INSERT INTO omt_test_failures VALUES(100, 'import', 'osm_park_polygon_gen_z5 expected 3, got ' || cnt); + END IF; + + SELECT COUNT(*) INTO cnt FROM osm_park_polygon_gen_z5 WHERE leisure='nature_reserve'; + IF cnt <> 1 THEN + INSERT INTO omt_test_failures VALUES(100, 'import', 'osm_park_polygon_gen_z5 nature_reserve expected 1, got ' || cnt); + END IF; + + SELECT COUNT(*) INTO cnt FROM osm_park_polygon_gen_z5 WHERE boundary='protected_area'; + IF cnt <> 1 THEN + INSERT INTO omt_test_failures VALUES(100, 'import', 'osm_park_polygon_gen_z5 protected_area expected 1, got ' || cnt); + END IF; + + SELECT COUNT(*) INTO cnt FROM osm_park_polygon_gen_z5 WHERE boundary='national_park'; + IF cnt <> 1 THEN + INSERT INTO omt_test_failures VALUES(100, 'import', 'osm_park_polygon_gen_z5 national_park expected 1, got ' || cnt); + END IF; + + -- Test 200 + SELECT COUNT(*) INTO cnt FROM osm_aerodrome_label_point; + IF cnt <> 3 THEN + INSERT INTO omt_test_failures VALUES(200, 'import', 'osm_aerodrome_label expected 3, got ' || cnt); + END IF; + + SELECT COUNT(*) INTO cnt FROM osm_aerodrome_label_point WHERE ele=123; + IF cnt <> 1 THEN + INSERT INTO omt_test_failures VALUES(200, 'import', 'osm_aerodrome_label ele=123 expected 1, got ' || cnt); + END IF; + + -- Test 300 + SELECT COUNT(*) INTO cnt FROM osm_landcover_polygon WHERE mapping_key='natural' AND subclass='wood'; + IF cnt <> 1 THEN + INSERT INTO omt_test_failures VALUES(300, 'import', 'osm_landcover_polygon natural=wood expected 1, got ' || cnt); + END IF; + + -- Test 400 + SELECT COUNT(DISTINCT relation_id) INTO cnt FROM osm_border_linestring WHERE admin_level=8; + IF cnt <> 1 THEN + INSERT INTO omt_test_failures VALUES(400, 'update', 'osm_border_linestring city count expected 1, got ' || cnt); + END IF; + + SELECT COUNT(DISTINCT relation_id) INTO cnt FROM osm_border_linestring WHERE admin_level=2; + IF cnt <> 1 THEN + INSERT INTO omt_test_failures VALUES(400, 'update', 'osm_border_linestring country count expected 1, got ' || cnt); + END IF; + +END; + +$$ +LANGUAGE plpgsql; + +DO $$ + +DECLARE + cnt integer; +BEGIN + SELECT COUNT(*) INTO cnt FROM omt_test_failures; + IF cnt > 0 THEN + RAISE '% unit test(s) failed on imports. Details can be found in table omt_test_failures.', cnt USING ERRCODE = '0Z000'; + END IF; +END; + +$$; diff --git a/tests/test-post-update.sql b/tests/test-post-update.sql new file mode 100644 index 00000000..b5e4e1c2 --- /dev/null +++ b/tests/test-post-update.sql @@ -0,0 +1,67 @@ +-- Checks to ensure that test data was imported correctly +DO $$ + +DECLARE + cnt integer; + +BEGIN + + -- Clear prior results + DELETE FROM omt_test_failures WHERE test_type='update'; + + -- Test 100: Verify re-tag of national_park to protected_area worked + SELECT COUNT(*) INTO cnt FROM osm_park_polygon_gen_z5 WHERE boundary='national_park'; + IF cnt <> 0 THEN + INSERT INTO omt_test_failures VALUES(100, 'update', 'osm_park_polygon_gen_z5 national_park expected 0, got ' || cnt); + END IF; + + SELECT COUNT(*) INTO cnt FROM osm_park_polygon_gen_z5 WHERE boundary='protected_area'; + IF cnt <> 2 THEN + INSERT INTO omt_test_failures VALUES(100, 'update', 'osm_park_polygon_gen_z5 protected_area expected 2, got ' || cnt); + END IF; + + -- Test 200: Verify aerodrome deleted and modified + SELECT COUNT(*) INTO cnt FROM osm_aerodrome_label_point; + IF cnt <> 2 THEN + INSERT INTO omt_test_failures VALUES(200, 'update', 'osm_aerodrome_label_point expected 2, got ' || cnt); + END IF; + + SELECT COUNT(*) INTO cnt FROM osm_aerodrome_label_point WHERE icao='KOMT' AND ele='124' AND name='OpenMapTiles International Airport'; + IF cnt <> 1 THEN + INSERT INTO omt_test_failures VALUES(200, 'update', 'osm_aerodrome_label_point failed to update attributes'); + END IF; + + -- Test 300: Verify landuse modified + SELECT COUNT(*) INTO cnt FROM osm_landcover_polygon WHERE mapping_key='natural' AND subclass='scrub'; + IF cnt <> 1 THEN + INSERT INTO omt_test_failures VALUES(300, 'import', 'osm_landcover_polygon natural=scrub expected 1, got ' || cnt); + END IF; + + SELECT COUNT(*) INTO cnt FROM osm_landcover_polygon WHERE mapping_key='natural' AND subclass='wood'; + IF cnt <> 0 THEN + INSERT INTO omt_test_failures VALUES(300, 'import', 'osm_landcover_polygon natural=wood expected 0, got ' || cnt); + END IF; + + -- Test 400: Verify new city added + SELECT COUNT(DISTINCT relation_id) INTO cnt FROM osm_border_linestring WHERE admin_level=8; + IF cnt <> 2 THEN + INSERT INTO omt_test_failures VALUES(400, 'update', 'osm_border_linestring city count expected 2, got ' || cnt); + END IF; + +END; + +$$; + + +DO $$ + +DECLARE + cnt integer; +BEGIN + SELECT COUNT(*) INTO cnt FROM omt_test_failures; + IF cnt > 0 THEN + RAISE '% unit test(s) failed on updates. Details can be found in table omt_test_failures.', cnt USING ERRCODE = '0Z000'; + END IF; +END; + +$$; diff --git a/tests/update/100_update-large-park.osc b/tests/update/100_update-large-park.osc new file mode 100644 index 00000000..ca67695e --- /dev/null +++ b/tests/update/100_update-large-park.osc @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/update/200_update-aerodrome.osc b/tests/update/200_update-aerodrome.osc new file mode 100644 index 00000000..c6fc222b --- /dev/null +++ b/tests/update/200_update-aerodrome.osc @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + diff --git a/tests/update/300_update-landcover.osc b/tests/update/300_update-landcover.osc new file mode 100644 index 00000000..4f3fe151 --- /dev/null +++ b/tests/update/300_update-landcover.osc @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + diff --git a/tests/update/400_update-boundary.osc b/tests/update/400_update-boundary.osc new file mode 100644 index 00000000..9d7e99ca --- /dev/null +++ b/tests/update/400_update-boundary.osc @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + + + + + + + + + + +