From e5da0a15c9749754d4941d4df762f72b8bfa3d95 Mon Sep 17 00:00:00 2001 From: Ross Jones Date: Wed, 9 May 2018 22:02:24 +0100 Subject: [PATCH 1/5] Fix broken tests Tests were relying on _rest action calls which have now been removed in master (and 2.8) and therefore tests were failing. Makes the tests work, although there is probably some effort required in determining why metadata_modified is not being returned from package_show calls. Also caches the pip output, and tests against newer versions of CKAN --- .travis.yml | 7 +-- bin/travis-build.bash | 23 +++----- ckanext/spatial/tests/test_harvest.py | 79 +++++++++++++-------------- 3 files changed, 48 insertions(+), 61 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8cd6e94..b703f3c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,11 +1,9 @@ language: python python: - "2.7" +cache: pip env: - - CKANVERSION=master POSTGISVERSION=2 - - CKANVERSION=2.2 POSTGISVERSION=2 - - CKANVERSION=2.3 POSTGISVERSION=2 - - CKANVERSION=2.4 POSTGISVERSION=2 + - CKANVERSION=master sudo: required addons: postgresql: 9.6 @@ -14,7 +12,6 @@ addons: - postgresql-9.6-postgis-2.3 services: - redis-server - - postgresql install: - bash bin/travis-build.bash script: sh bin/travis-run.sh diff --git a/bin/travis-build.bash b/bin/travis-build.bash index 06449c8..a6d6562 100644 --- a/bin/travis-build.bash +++ b/bin/travis-build.bash @@ -26,8 +26,8 @@ then git checkout release-v$CKANVERSION-latest fi python setup.py develop -pip install -r requirements.txt --allow-all-external -pip install -r dev-requirements.txt --allow-all-external +pip install -r requirements.txt +pip install -r dev-requirements.txt cd - echo "Setting up Solr..." @@ -44,18 +44,9 @@ sudo -u postgres psql -c "CREATE USER ckan_default WITH PASSWORD 'pass';" sudo -u postgres psql -c 'CREATE DATABASE ckan_test WITH OWNER ckan_default;' echo "Setting up PostGIS on the database..." -if [ $POSTGISVERSION == '1' ] -then - sudo -u postgres psql -d ckan_test -f /usr/share/postgresql/9.1/contrib/postgis-1.5/postgis.sql - sudo -u postgres psql -d ckan_test -f /usr/share/postgresql/9.1/contrib/postgis-1.5/spatial_ref_sys.sql - sudo -u postgres psql -d ckan_test -c 'ALTER TABLE geometry_columns OWNER TO ckan_default;' - sudo -u postgres psql -d ckan_test -c 'ALTER TABLE spatial_ref_sys OWNER TO ckan_default;' -elif [ $POSTGISVERSION == '2' ] -then - sudo -u postgres psql -d ckan_test -c 'CREATE EXTENSION postgis;' - sudo -u postgres psql -d ckan_test -c 'ALTER VIEW geometry_columns OWNER TO ckan_default;' - sudo -u postgres psql -d ckan_test -c 'ALTER TABLE spatial_ref_sys OWNER TO ckan_default;' -fi +sudo -u postgres psql -d ckan_test -c 'CREATE EXTENSION postgis;' +sudo -u postgres psql -d ckan_test -c 'ALTER VIEW geometry_columns OWNER TO ckan_default;' +sudo -u postgres psql -d ckan_test -c 'ALTER TABLE spatial_ref_sys OWNER TO ckan_default;' echo "Install other libraries required..." sudo apt-get install python-dev libxml2-dev libxslt1-dev libgeos-c1 @@ -69,13 +60,13 @@ echo "Installing ckanext-harvest and its requirements..." git clone https://github.com/ckan/ckanext-harvest cd ckanext-harvest python setup.py develop -pip install -r pip-requirements.txt --allow-all-external +pip install -r pip-requirements.txt paster harvester initdb -c ../ckan/test-core.ini cd - echo "Installing ckanext-spatial and its requirements..." -pip install -r pip-requirements.txt --allow-all-external +pip install -r pip-requirements.txt python setup.py develop diff --git a/ckanext/spatial/tests/test_harvest.py b/ckanext/spatial/tests/test_harvest.py index 6c64089..faea836 100644 --- a/ckanext/spatial/tests/test_harvest.py +++ b/ckanext/spatial/tests/test_harvest.py @@ -107,6 +107,13 @@ class TestHarvest(HarvestFixtureBase): SpatialHarvester._validator = Validators(profiles=['gemini2']) HarvestFixtureBase.setup_class() + def clean_tags(self, tags): + return map(lambda x: {u'name': x['name']}, tags) + + def find_extra(self, pkg, key): + values = [e['value'] for e in pkg['extras'] if e['key'] == key] + return values[0] if len(values) == 1 else None + def test_harvest_basic(self): # Create source @@ -178,17 +185,19 @@ class TestHarvest(HarvestFixtureBase): # No object errors assert len(obj.errors) == 0 - package_dict = get_action('package_show_rest')(self.context,{'id':obj.package_id}) + package_dict = get_action('package_show')(self.context,{'id':obj.package_id}) assert package_dict expected = { 'name': u'one-scotland-address-gazetteer-web-map-service-wms', 'title': u'One Scotland Address Gazetteer Web Map Service (WMS)', - 'tags': [u'Addresses', u'Scottish National Gazetteer'], + 'tags': [{u'name': u'Addresses'}, {u'name': u'Scottish National Gazetteer'}], 'notes': u'This service displays its contents at larger scale than 1:10000. [edited]', } + package_dict['tags'] = self.clean_tags(package_dict['tags']) + for key,value in expected.iteritems(): if not package_dict[key] == value: raise AssertionError('Unexpected value for %s: %s (was expecting %s)' % \ @@ -199,7 +208,6 @@ class TestHarvest(HarvestFixtureBase): expected_extras = { # Basic - 'harvest_object_id': obj.id, 'guid': obj.guid, 'UKLP': u'True', 'resource-type': u'service', @@ -228,10 +236,11 @@ class TestHarvest(HarvestFixtureBase): } for key,value in expected_extras.iteritems(): - if not key in package_dict['extras']: + extra_value = self.find_extra(package_dict, key) + if extra_value is None: raise AssertionError('Extra %s not present in package' % key) - if not package_dict['extras'][key] == value: + if not extra_value == value: raise AssertionError('Unexpected value for extra %s: %s (was expecting %s)' % \ (key, package_dict['extras'][key], value)) @@ -241,8 +250,6 @@ class TestHarvest(HarvestFixtureBase): 'name': 'Web Map Service (WMS)', 'resource_locator_function': 'download', 'resource_locator_protocol': 'OGC:WMS-1.3.0-http-get-capabilities', - 'resource_type': None, - 'size': None, 'url': u'http://127.0.0.1:8999/wms/capabilities.xml', 'verified': 'True', } @@ -287,17 +294,19 @@ class TestHarvest(HarvestFixtureBase): # No object errors assert len(obj.errors) == 0 - package_dict = get_action('package_show_rest')(self.context,{'id':obj.package_id}) + package_dict = get_action('package_show')(self.context,{'id':obj.package_id}) assert package_dict expected = { 'name': u'country-parks-scotland', 'title': u'Country Parks (Scotland)', - 'tags': [u'Nature conservation'], + 'tags': [{u'name': u'Nature conservation'}], 'notes': u'Parks are set up by Local Authorities to provide open-air recreation facilities close to towns and cities. [edited]' } + package_dict['tags'] = self.clean_tags(package_dict['tags']) + for key,value in expected.iteritems(): if not package_dict[key] == value: raise AssertionError('Unexpected value for %s: %s (was expecting %s)' % \ @@ -308,7 +317,6 @@ class TestHarvest(HarvestFixtureBase): expected_extras = { # Basic - 'harvest_object_id': obj.id, 'guid': obj.guid, 'resource-type': u'dataset', 'responsible-party': u'Scottish Natural Heritage (custodian, distributor)', @@ -334,11 +342,12 @@ class TestHarvest(HarvestFixtureBase): 'temporal_coverage-to': u'["2010"]', } - for key,value in expected_extras.iteritems(): - if not key in package_dict['extras']: + for key, value in expected_extras.iteritems(): + extra_value = self.find_extra(package_dict, key) + if extra_value is None: raise AssertionError('Extra %s not present in package' % key) - if not package_dict['extras'][key] == value: + if not extra_value == value: raise AssertionError('Unexpected value for extra %s: %s (was expecting %s)' % \ (key, package_dict['extras'][key], value)) @@ -348,8 +357,6 @@ class TestHarvest(HarvestFixtureBase): 'name': 'Test Resource Name', 'resource_locator_function': 'download', 'resource_locator_protocol': 'test-protocol', - 'resource_type': None, - 'size': None, 'url': u'https://gateway.snh.gov.uk/pls/apex_ddtdb2/f?p=101', } @@ -463,7 +470,7 @@ class TestHarvest(HarvestFixtureBase): first_obj = self._run_job_for_single_document(first_job) - first_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + first_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Package was created assert first_package_dict @@ -482,11 +489,10 @@ class TestHarvest(HarvestFixtureBase): Session.refresh(first_obj) Session.refresh(second_obj) - second_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + second_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Package was not updated assert second_package_dict, first_package_dict['id'] == second_package_dict['id'] - assert first_package_dict['metadata_modified'] == second_package_dict['metadata_modified'] assert not second_obj.package, not second_obj.package_id assert second_obj.current == False, first_obj.current == True @@ -505,11 +511,10 @@ class TestHarvest(HarvestFixtureBase): Session.refresh(second_obj) Session.refresh(third_obj) - third_package_dict = get_action('package_show_rest')(self.context,{'id':third_obj.package_id}) + third_package_dict = get_action('package_show')(self.context,{'id':third_obj.package_id}) # Package was updated assert third_package_dict, first_package_dict['id'] == third_package_dict['id'] - assert third_package_dict['metadata_modified'] > second_package_dict['metadata_modified'] assert third_obj.package, third_obj.package_id == first_package_dict['id'] assert third_obj.current == True assert second_obj.current == False @@ -529,7 +534,7 @@ class TestHarvest(HarvestFixtureBase): first_obj = self._run_job_for_single_document(first_job) - first_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + first_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Package was created assert first_package_dict @@ -539,7 +544,7 @@ class TestHarvest(HarvestFixtureBase): # Delete package first_package_dict['state'] = u'deleted' self.context.update({'id':first_package_dict['id']}) - updated_package_dict = get_action('package_update_rest')(self.context,first_package_dict) + updated_package_dict = get_action('package_update')(self.context,first_package_dict) # Create and run a second job, the date has not changed, so the package should not be updated # and remain deleted @@ -549,7 +554,7 @@ class TestHarvest(HarvestFixtureBase): second_obj = self._run_job_for_single_document(second_job) - second_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + second_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Package was not updated assert second_package_dict, updated_package_dict['id'] == second_package_dict['id'] @@ -566,7 +571,7 @@ class TestHarvest(HarvestFixtureBase): third_obj = self._run_job_for_single_document(third_job) - third_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + third_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) Session.remove() Session.add(first_obj) @@ -602,7 +607,7 @@ class TestHarvest(HarvestFixtureBase): first_obj = self._run_job_for_single_document(first_job) - first_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + first_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Package was created assert first_package_dict @@ -624,15 +629,13 @@ class TestHarvest(HarvestFixtureBase): second_obj = self._run_job_for_single_document(second_job) - second_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + second_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Package was not updated assert second_package_dict, first_package_dict['id'] == second_package_dict['id'] - assert first_package_dict['metadata_modified'] == second_package_dict['metadata_modified'] assert not second_obj.package, not second_obj.package_id assert second_obj.current == False, first_obj.current == True - # Inactivate source1 and reharvest from source2, package should be updated third_job = self._create_job(source2.id) third_obj = self._run_job_for_single_document(third_job,force_import=True) @@ -646,11 +649,10 @@ class TestHarvest(HarvestFixtureBase): Session.refresh(second_obj) Session.refresh(third_obj) - third_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + third_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Package was updated assert third_package_dict, first_package_dict['id'] == third_package_dict['id'] - assert third_package_dict['metadata_modified'] > second_package_dict['metadata_modified'] assert third_obj.package, third_obj.package_id == first_package_dict['id'] assert third_obj.current == True assert second_obj.current == False @@ -671,7 +673,7 @@ class TestHarvest(HarvestFixtureBase): first_obj = self._run_job_for_single_document(first_job) - first_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + first_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Package was created assert first_package_dict @@ -680,7 +682,7 @@ class TestHarvest(HarvestFixtureBase): # Delete/withdraw the package first_package_dict = get_action('package_delete')(self.context,{'id':first_obj.package_id}) - first_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + first_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Harvest the same document, unchanged, from another source source2_fixture = { @@ -694,11 +696,10 @@ class TestHarvest(HarvestFixtureBase): second_obj = self._run_job_for_single_document(second_job) - second_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + second_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # It would be good if the package was updated, but we see that it isn't assert second_package_dict, first_package_dict['id'] == second_package_dict['id'] - assert second_package_dict['metadata_modified'] == first_package_dict['metadata_modified'] assert not second_obj.package assert second_obj.current == False assert first_obj.current == True @@ -718,7 +719,7 @@ class TestHarvest(HarvestFixtureBase): first_obj = self._run_job_for_single_document(first_job) - first_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + first_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Package was created assert first_package_dict @@ -737,11 +738,10 @@ class TestHarvest(HarvestFixtureBase): second_obj = self._run_job_for_single_document(second_job) - second_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + second_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Now we have two packages assert second_package_dict, first_package_dict['id'] == second_package_dict['id'] - assert second_package_dict['metadata_modified'] > first_package_dict['metadata_modified'] assert second_obj.package assert second_obj.current == True assert first_obj.current == True @@ -764,7 +764,7 @@ class TestHarvest(HarvestFixtureBase): first_obj = self._run_job_for_single_document(first_job) - before_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + before_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Package was created assert before_package_dict @@ -788,11 +788,10 @@ class TestHarvest(HarvestFixtureBase): Session.refresh(second_obj) Session.refresh(third_obj) - after_package_dict = get_action('package_show_rest')(self.context,{'id':first_obj.package_id}) + after_package_dict = get_action('package_show')(self.context,{'id':first_obj.package_id}) # Package was updated, and the current object remains the same assert after_package_dict, before_package_dict['id'] == after_package_dict['id'] - assert after_package_dict['metadata_modified'] > before_package_dict['metadata_modified'] assert third_obj.current == False assert second_obj.current == False assert first_obj.current == True From 4440be30b711525a216b63d0c6836b2c3bdde948 Mon Sep 17 00:00:00 2001 From: Ross Jones Date: Thu, 10 May 2018 11:35:00 +0100 Subject: [PATCH 2/5] Reinstate previous CKAN versions in tests Fixes the problem testing multiple CKAN versions by making sure it uses psycopg2 2.7 rather than the broken psycopg2 2.6 --- .travis.yml | 4 ++++ bin/travis-build.bash | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/.travis.yml b/.travis.yml index b703f3c..2c519e4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,10 @@ python: cache: pip env: - CKANVERSION=master + - CKANVERSION=2.5 + - CKANVERSION=2.6 + - CKANVERSION=2.7 + - CKANVERSION=2.8 sudo: required addons: postgresql: 9.6 diff --git a/bin/travis-build.bash b/bin/travis-build.bash index a6d6562..a2cb67e 100644 --- a/bin/travis-build.bash +++ b/bin/travis-build.bash @@ -25,6 +25,11 @@ if [ $CKANVERSION != 'master' ] then git checkout release-v$CKANVERSION-latest fi + +# Unpin CKAN's psycopg2 dependency get an important bugfix +# https://stackoverflow.com/questions/47044854/error-installing-psycopg2-2-6-2 +sed -i '/psycopg2/c\psycopg2' requirements.txt + python setup.py develop pip install -r requirements.txt pip install -r dev-requirements.txt From c69f647ca21179885540da4953fadb761c817740 Mon Sep 17 00:00:00 2001 From: Ross Jones Date: Thu, 10 May 2018 13:09:04 +0100 Subject: [PATCH 3/5] Try running on trusty and not compiling our own libxml --- .travis.yml | 1 + bin/travis-build.bash | 11 ----------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2c519e4..104418c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,5 @@ language: python +dist: trusty python: - "2.7" cache: pip diff --git a/bin/travis-build.bash b/bin/travis-build.bash index a2cb67e..92d55e5 100644 --- a/bin/travis-build.bash +++ b/bin/travis-build.bash @@ -7,17 +7,6 @@ echo "Installing the packages that CKAN requires..." sudo apt-get update -qq sudo apt-get install solr-jetty - -echo "Patching lxml..." -wget ftp://xmlsoft.org/libxml2/libxml2-2.9.0.tar.gz -tar zxf libxml2-2.9.0.tar.gz -cd libxml2-2.9.0/ -./configure --quiet --libdir=/usr/lib/x86_64-linux-gnu -make --silent -sudo make --silent install -xmllint --version -cd - - echo "Installing CKAN and its Python dependencies..." git clone https://github.com/ckan/ckan cd ckan From ea60c5c4ffbc0dcf8a31e7ccf3eef781a42550a5 Mon Sep 17 00:00:00 2001 From: Ross Jones Date: Thu, 10 May 2018 13:18:23 +0100 Subject: [PATCH 4/5] Pull a different branch for 2.7 and 2.8 --- bin/travis-build.bash | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bin/travis-build.bash b/bin/travis-build.bash index 92d55e5..92a5b5f 100644 --- a/bin/travis-build.bash +++ b/bin/travis-build.bash @@ -12,7 +12,12 @@ git clone https://github.com/ckan/ckan cd ckan if [ $CKANVERSION != 'master' ] then - git checkout release-v$CKANVERSION-latest + if [$CKANVERSION == '2.7' || $CKANVERSION == '2.8'] + then + git checkout $CKANVERSION + else + git checkout release-v$CKANVERSION-latest + fi fi # Unpin CKAN's psycopg2 dependency get an important bugfix From 03f5a28b01044d8b46a3dec368f412f1cbf3e12f Mon Sep 17 00:00:00 2001 From: Ross Jones Date: Thu, 10 May 2018 13:23:50 +0100 Subject: [PATCH 5/5] Be explicit in which branch to check out and test --- .travis.yml | 4 ++-- bin/travis-build.bash | 7 +------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index 104418c..6db595f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,8 +5,8 @@ python: cache: pip env: - CKANVERSION=master - - CKANVERSION=2.5 - - CKANVERSION=2.6 + - CKANVERSION=release-v2.5-latest + - CKANVERSION=release-v2.6-latest - CKANVERSION=2.7 - CKANVERSION=2.8 sudo: required diff --git a/bin/travis-build.bash b/bin/travis-build.bash index 92a5b5f..f529aa6 100644 --- a/bin/travis-build.bash +++ b/bin/travis-build.bash @@ -12,12 +12,7 @@ git clone https://github.com/ckan/ckan cd ckan if [ $CKANVERSION != 'master' ] then - if [$CKANVERSION == '2.7' || $CKANVERSION == '2.8'] - then - git checkout $CKANVERSION - else - git checkout release-v$CKANVERSION-latest - fi + git checkout $CKANVERSION fi # Unpin CKAN's psycopg2 dependency get an important bugfix