geopython / pygeoapi

pygeoapi is a Python server implementation of the OGC API suite of standards. The project emerged as part of the next generation OGC API efforts in 2018 and provides the capability for organizations to deploy a RESTful OGC API endpoint using OpenAPI, GeoJSON, and HTML. pygeoapi is open source and released under an MIT license.
https://pygeoapi.io
MIT License
483 stars 258 forks source link

fix Travis builds #406

Closed tomkralidis closed 4 years ago

tomkralidis commented 4 years ago

Description Travis builds are consistently failing against OGR ESRI JSON tests.

Steps to Reproduce Trigger a Travis build (new PR or close/reopen an existing PR)

Expected behavior Travis builds should pass.

Screenshots/Tracebacks https://travis-ci.org/github/geopython/pygeoapi/jobs/674647541#L6911

Environment

Additional context Add any other context about the problem here.

tomkralidis commented 4 years ago

cc @justb4

justb4 commented 4 years ago

analysis: the failing assertions in the two failing tests assume specific object/feature id's.

tomkralidis commented 4 years ago

Should we remove these specific tests or are there IDs (or layers) which are more static?

justb4 commented 4 years ago

Well for one thing the id's have changed, so 78232831 should now be 78320986 and 78220641 should be 78320245. That would certainly fix the tests for now.

Better is to fix the tests: get-by-id certainly cannot be skipped. Strategy: get some features, e.g. first 10 and then pick one to get its id and use that id.

The other failing test: we can skip the statements.

Ok, I can do fixes, though do not have GDAL3 on my local system, but via Docker will help, building/testing that anyways.

francbartoli commented 4 years ago

@justb4 monkey fix for now. I will add your strategy in a different PR which will add support of CSV through the OGR provider (via GDAL virtual layer). This would allow to read csv files from cloud storage for instance. I'm interested to get it from GitHub for the covid-19 demo

tomkralidis commented 4 years ago

ok sounds good to me.

justb4 commented 4 years ago

I was quite far, main issue is that I don't have GDAL v3 yet on my system (homebrew) and had to test via Dockerfile, had little time anyway, was on low bandwidth so builds took a long time.

But below is my WIP as diffed, no more specific id's: (right is my version, left current master). Maybe it is easy for @francbartoli to integrate that code? But test first please.

image

Code:

def test_get_agol(config_ArcGIS_ESRIJSON):
    """Testing query for a specific object"""

    p = OGRProvider(config_ArcGIS_ESRIJSON)
    # Get single feature to have an id
    feature_collection = p.query(startindex=0, limit=1, resulttype='results')
    features = feature_collection.get('features', None)
    assert len(features) == 1
    result = p.get(features[0]['id'])
    assert result['properties']['fulladdr'] is not None

Ok, may get the fulladdr as well from the p.query result and match that value on p.get() result.

and image

Think one can never rely on fids/objectids (think of datasets refreshed each night) and a certain order, so the test is good enough IMO.

def test_query_with_startindex(config_ArcGIS_ESRIJSON):
    """Testing query for a valid JSON object with geometry"""

    p = OGRProvider(config_ArcGIS_ESRIJSON)
    feature_collection = p.query(startindex=10, limit=10, resulttype='results')
    assert feature_collection.get('type', None) == 'FeatureCollection'
    features = feature_collection.get('features', None)
    assert len(features) == 10
    hits = feature_collection.get('numberMatched', None)
    assert hits is None
    feature = features[0]
    properties = feature.get('properties', None)
    assert properties is not None
    assert properties['fulladdr'] is not None
    geometry = feature.get('geometry', None)
    assert geometry is not None
tomkralidis commented 4 years ago

+1 @justb4

@francbartoli here's a diff that may help for the first test:

diff --git a/tests/test_ogr_esrijson_provider.py b/tests/test_ogr_esrijson_provider.py
index d7323fe..84372c9 100644
--- a/tests/test_ogr_esrijson_provider.py
+++ b/tests/test_ogr_esrijson_provider.py
@@ -78,9 +78,15 @@ def test_get_agol(config_ArcGIS_ESRIJSON):
     """Testing query for a specific object"""

     p = OGRProvider(config_ArcGIS_ESRIJSON)
-    result = p.get('78232831')
-    assert result['id'] == 78232831
-    assert '2605' in result['properties']['fulladdr']
+    results = p.query()
+
+    result8 = results['features'][8]
+
+    result = p.get(result8['id'])
+
+    assert result['id'] == result8['id']
+
+    assert result['properties']['fulladdr'] == result8['properties']['fulladdr']

 def test_query_hits_agol(config_ArcGIS_ESRIJSON):
@@ -109,7 +115,6 @@ def test_query_bbox_hits_agol(config_ArcGIS_ESRIJSON):
     assert len(features) == 0
     hits = feature_collection.get('numberMatched', None)
     assert hits is not None
-    print('hits={}'.format(hits))
     assert hits > 1
francbartoli commented 4 years ago

@justb4 sure, I'm going to take this on