From af6db9c520459e75184563e30a3e7b2fb2a8f561 Mon Sep 17 00:00:00 2001 From: William Ronchetti Date: Tue, 18 Jul 2023 12:03:19 -0400 Subject: [PATCH 1/7] add more tests --- src/encoded/tests/test_authentication.py | 103 +++++++++++++++++++++ src/encoded/tests/test_authorization.py | 28 ++++++ src/encoded/tests/test_indexing.py | 111 +++++++++++++++++++++++ src/encoded/tests/test_search.py | 10 ++ src/encoded/tests/testing_views.py | 64 ++++++++++++- 5 files changed, 315 insertions(+), 1 deletion(-) create mode 100644 src/encoded/tests/test_authentication.py create mode 100644 src/encoded/tests/test_authorization.py create mode 100644 src/encoded/tests/test_indexing.py create mode 100644 src/encoded/tests/test_search.py diff --git a/src/encoded/tests/test_authentication.py b/src/encoded/tests/test_authentication.py new file mode 100644 index 000000000..3a5f1eabf --- /dev/null +++ b/src/encoded/tests/test_authentication.py @@ -0,0 +1,103 @@ +import pytest +import unittest + +from pyramid.interfaces import IAuthenticationPolicy +from pyramid.security import Authenticated, Everyone +from pyramid.testing import DummyRequest +from zope.interface.verify import verifyClass, verifyObject +from ..authentication import NamespacedAuthenticationPolicy + + +pytestmark = [pytest.mark.setone, pytest.mark.working] + + +class TestNamespacedAuthenticationPolicy(unittest.TestCase): + """ This is a modified version of TestRemoteUserAuthenticationPolicy + """ + def _getTargetClass(self): + return NamespacedAuthenticationPolicy + + def _makeOne(self, namespace='user', + base='pyramid.authentication.RemoteUserAuthenticationPolicy', + *args, **kw): + return self._getTargetClass()(namespace, base, *args, **kw) + + def test_class_implements_IAuthenticationPolicy(self): + klass = self._makeOne().__class__ + verifyClass(IAuthenticationPolicy, klass) + + def test_instance_implements_IAuthenticationPolicy(self): + verifyObject(IAuthenticationPolicy, self._makeOne()) + + def test_unauthenticated_userid_returns_None(self): + request = DummyRequest(environ={}) + policy = self._makeOne() + self.assertEqual(policy.unauthenticated_userid(request), None) + + def test_unauthenticated_userid(self): + request = DummyRequest(environ={'REMOTE_USER':'fred'}) + policy = self._makeOne() + self.assertEqual(policy.unauthenticated_userid(request), 'user.fred') + + def test_authenticated_userid_None(self): + request = DummyRequest(environ={}) + policy = self._makeOne() + self.assertEqual(policy.authenticated_userid(request), None) + + def test_authenticated_userid(self): + request = DummyRequest(environ={'REMOTE_USER':'fred'}) + policy = self._makeOne() + self.assertEqual(policy.authenticated_userid(request), 'user.fred') + + def test_effective_principals_None(self): + request = DummyRequest(environ={}) + policy = self._makeOne() + self.assertEqual(policy.effective_principals(request), [Everyone]) + + def test_effective_principals(self): + request = DummyRequest(environ={'REMOTE_USER':'fred'}) + policy = self._makeOne() + self.assertEqual(policy.effective_principals(request), + [Everyone, Authenticated, 'user.fred']) + + def test_remember(self): + request = DummyRequest(environ={'REMOTE_USER':'fred'}) + policy = self._makeOne() + result = policy.remember(request, 'fred') + self.assertEqual(result, []) + + def test_forget(self): + request = DummyRequest(environ={'REMOTE_USER':'fred'}) + policy = self._makeOne() + result = policy.forget(request) + self.assertEqual(result, []) + + # From TestSessionAuthenticationPolicy + + def test_session_remember(self): + request = DummyRequest() + policy = self._makeOne( + base='pyramid.authentication.SessionAuthenticationPolicy', + prefix='') + result = policy.remember(request, 'user.fred') + self.assertEqual(request.session.get('userid'), 'fred') + self.assertEqual(result, []) + self.assertEqual(policy.unauthenticated_userid(request), 'user.fred') + + def test_session_forget(self): + request = DummyRequest(session={'userid':'fred'}) + policy = self._makeOne( + base='pyramid.authentication.SessionAuthenticationPolicy', + prefix='') + result = policy.forget(request) + self.assertEqual(request.session.get('userid'), None) + self.assertEqual(result, []) + + def test_session_forget_no_identity(self): + request = DummyRequest() + policy = self._makeOne( + base='pyramid.authentication.SessionAuthenticationPolicy', + prefix='') + result = policy.forget(request) + self.assertEqual(request.session.get('userid'), None) + self.assertEqual(result, []) diff --git a/src/encoded/tests/test_authorization.py b/src/encoded/tests/test_authorization.py new file mode 100644 index 000000000..65e0db588 --- /dev/null +++ b/src/encoded/tests/test_authorization.py @@ -0,0 +1,28 @@ +import pytest + + +from .test_search import MockedRequest +from snovault.util import is_admin_request + + +pytestmark = [pytest.mark.working] + + +@pytest.mark.parametrize('mock_request, expected', [ + [MockedRequest(), False], + [MockedRequest(principals_allowed=[ + 'group.admin' + ]), True], + [MockedRequest(principals_allowed=[ + 'group.read-only-admin' + ]), False], # read only admin is not admin technically + [MockedRequest(principals_allowed=[ + 'group.something-else' + ]), False], + [MockedRequest(principals_allowed=[ + 'lots', 'of', 'other', 'perms', 'but', 'not', 'group', 'dot', 'admin' + ]), False] +]) +def test_authorization_is_admin_request(mock_request, expected): + """ Checks that the request tests correctly resolve. """ + assert is_admin_request(mock_request) is expected diff --git a/src/encoded/tests/test_indexing.py b/src/encoded/tests/test_indexing.py new file mode 100644 index 000000000..651d1e768 --- /dev/null +++ b/src/encoded/tests/test_indexing.py @@ -0,0 +1,111 @@ +""" Test full indexing setup + +The fixtures in this module setup a full system with postgresql and +elasticsearch running as subprocesses. +""" +import json +import os +import pkg_resources +import pytest +import re +import time +import transaction +import uuid + +from dcicutils.misc_utils import PRINT +from dcicutils.qa_utils import notice_pytest_fixtures, Eventually +from snovault.tools import index_n_items_for_testing, make_es_count_checker +from snovault import DBSESSION, TYPES +from snovault.elasticsearch import create_mapping, ELASTIC_SEARCH +from snovault.elasticsearch.create_mapping import ( + type_mapping, + create_mapping_by_type, + build_index_record, + compare_against_existing_mapping +) +from snovault.elasticsearch.indexer_utils import get_namespaced_index, compute_invalidation_scope +from snovault.elasticsearch.interfaces import INDEXER_QUEUE +from snovault.storage import Base +from sqlalchemy import MetaData, func, exc +from timeit import default_timer as timer +from unittest import mock +from zope.sqlalchemy import mark_changed +from .. import main +from snovault import loadxl + + +pytestmark = [pytest.mark.working, pytest.mark.indexing] + + +# All compatible postgres versions +POSTGRES_COMPATIBLE_MAJOR_VERSIONS = ['11', '12', '13', '14', '15'] + + +# subset of collections to run test on +TEST_COLLECTIONS = ['file_processed'] + + +def test_postgres_version(session): + """ Tests that the local postgres is running one of the compatible versions """ + (version_info,) = session.query(func.version()).one() + PRINT("version_info=", version_info) + assert isinstance(version_info, str) + assert re.match("PostgreSQL (%s)([.][0-9]+)? " % '|'.join(POSTGRES_COMPATIBLE_MAJOR_VERSIONS), version_info) + + +@pytest.fixture(scope='module') +def es_app(es_app_settings, request): + notice_pytest_fixtures(request) + # for now, don't run with mpindexer. Add `True` to params above to do so + # if request.param: + # # we disable the MPIndexer since the build runs on a small machine + # # snovault should be testing the mpindexer - Will 12/12/2020 + # es_app_settings['mpindexer'] = True + app = main({}, **es_app_settings) + + yield app + + db_session = app.registry[DBSESSION] + # Dispose connections so postgres can tear down. + db_session.bind.pool.dispose() + + +@pytest.fixture +def setup_and_teardown(es_app): + """ + Run create mapping and purge queue before tests and clear out the + DB tables after the test + """ + + # BEFORE THE TEST - run create mapping for tests types and clear queues + create_mapping.run(es_app, collections=TEST_COLLECTIONS, skip_indexing=True) + es_app.registry[INDEXER_QUEUE].clear_queue() + + yield # run the test + + # AFTER THE TEST + session = es_app.registry[DBSESSION] + connection = session.connection().connect() + meta = MetaData(bind=session.connection()) + meta.reflect() + # sqlalchemy 1.4 - use TRUNCATE instead of DELETE + while True: + try: + table_names = ','.join(table.name for table in reversed(Base.metadata.sorted_tables)) + connection.execute(f'TRUNCATE {table_names} RESTART IDENTITY;') + break + except exc.OperationalError as e: + if 'statement timeout' in str(e): + continue + else: + raise + except exc.InternalError as e: + if 'current transaction is aborted' in str(e): + break + else: + raise + session.flush() + mark_changed(session()) # Has it always changed? -kmp 12-Oct-2022 + transaction.commit() + + diff --git a/src/encoded/tests/test_search.py b/src/encoded/tests/test_search.py new file mode 100644 index 000000000..1d563c2a0 --- /dev/null +++ b/src/encoded/tests/test_search.py @@ -0,0 +1,10 @@ +class MockedRequest(object): + """ Test object intended to be used to mock certain aspects of requests. Takes kwargs which + will be passed as named fields to MockedRequest. More arguments could be added if other + use is seen. + """ + def __init__(self, **kwargs): + if 'principals_allowed' not in kwargs: + self.effective_principals = ['system.Everyone'] + else: + self.effective_principals = kwargs['principals_allowed'] # note this is not exactly what the field is diff --git a/src/encoded/tests/testing_views.py b/src/encoded/tests/testing_views.py index 8af84528e..a2b2622c9 100644 --- a/src/encoded/tests/testing_views.py +++ b/src/encoded/tests/testing_views.py @@ -1,2 +1,64 @@ +from pyramid.security import Allow +from snovault import Item, collection + + def includeme(config): - pass \ No newline at end of file + pass + + +@collection( + 'testing-post-put-patch', + acl=[ + (Allow, 'group.submitter', ['add', 'edit', 'view']), + ], +) +class TestingPostPutPatch(Item): + item_type = 'testing_post_put_patch' + schema = { + 'required': ['required'], + 'type': 'object', + 'additionalProperties': False, + 'properties': { + "schema_version": { + "type": "string", + "pattern": "^\\d+(\\.\\d+)*$", + "requestMethod": [], + "default": "1", + }, + "uuid": { + "title": "UUID", + "description": "", + "type": "string", + "format": "uuid", + "permission": "restricted_fields", + "requestMethod": "POST", + }, + 'required': { + 'type': 'string', + }, + 'simple1': { + 'type': 'string', + 'default': 'simple1 default', + }, + 'simple2': { + 'type': 'string', + 'default': 'simple2 default', + }, + 'protected': { + # This should be allowed on PUT so long as value is the same + 'type': 'string', + 'default': 'protected default', + 'permission': 'restricted_fields', + }, + 'protected_link': { + # This should be allowed on PUT so long as the linked uuid is + # the same + 'type': 'string', + 'linkTo': 'TestingLinkTarget', + 'permission': 'restricted_fields', + }, + 'field_no_default': { + 'type': 'string', + }, + } + } From 994d7603cc308e88fe8dbe1bb7103f985f849d2b Mon Sep 17 00:00:00 2001 From: William Ronchetti Date: Wed, 19 Jul 2023 12:19:58 -0400 Subject: [PATCH 2/7] checkpoint a bunch more tests --- src/encoded/__init__.py | 2 +- src/encoded/appdefs.py | 7 + src/encoded/project/loadxl.py | 13 +- src/encoded/schemas/consortium.json | 2 +- src/encoded/schemas/submission_center.json | 2 +- src/encoded/tests/conftest.py | 2 +- src/encoded/tests/test_indexing.py | 178 ++++++++++++++++- src/encoded/tests/test_ingestion_listener.py | 176 +++++++++++++++++ src/encoded/tests/test_renderers.py | 197 +++++++++++++++++++ src/encoded/tests/test_schemas.py | 196 ++++++++++++++++++ src/encoded/tests/test_static.py | 32 +++ src/encoded/tests/testing_views.py | 2 +- src/encoded/tests/verifier.py | 113 +++++++++++ 13 files changed, 911 insertions(+), 11 deletions(-) create mode 100644 src/encoded/tests/test_ingestion_listener.py create mode 100644 src/encoded/tests/test_renderers.py create mode 100644 src/encoded/tests/test_schemas.py create mode 100644 src/encoded/tests/test_static.py create mode 100644 src/encoded/tests/verifier.py diff --git a/src/encoded/__init__.py b/src/encoded/__init__.py index b332451e1..c0551d465 100644 --- a/src/encoded/__init__.py +++ b/src/encoded/__init__.py @@ -84,8 +84,8 @@ def include_snovault(config: Configurator) -> None: config.include('snovault.resource_views') config.include('snovault.settings') config.include('snovault.server_defaults') - # Renderers is giving problems at the moment - Will 6/1/23 config.include('snovault.renderers') + config.include('snovault.ingestion.ingestion_listener') # configure redis server in production.ini if 'redis.server' in config.registry.settings: config.include('snovault.redis') diff --git a/src/encoded/appdefs.py b/src/encoded/appdefs.py index 771c070bb..30948daa2 100644 --- a/src/encoded/appdefs.py +++ b/src/encoded/appdefs.py @@ -1 +1,8 @@ +import os + + APP_VERSION_REGISTRY_KEY = 'snovault.app_version' + + +ENCODED_ROOT_DIR = os.path.dirname(__file__) +PROJECT_DIR = os.path.dirname(os.path.dirname(ENCODED_ROOT_DIR)) # two levels of hierarchy up diff --git a/src/encoded/project/loadxl.py b/src/encoded/project/loadxl.py index 6a5ab7a1c..531b6e3c3 100644 --- a/src/encoded/project/loadxl.py +++ b/src/encoded/project/loadxl.py @@ -3,9 +3,7 @@ class SMaHTProjectLoadxl(SnovaultProjectLoadxl): - def loadxl_order(self): - """ Defines any hard orderings that must happen when reindexing types """ - return [ + order = [ 'AccessKey', 'User', 'Consortium', @@ -29,4 +27,11 @@ def loadxl_order(self): 'FilterSet', 'HiglassViewConfig', 'IngestionSubmission' - ] + ] + + def loadxl_order(self): + """ Defines any hard orderings that must happen when reindexing types """ + return self.order + + +ITEM_INDEX_ORDER = SMaHTProjectLoadxl.order diff --git a/src/encoded/schemas/consortium.json b/src/encoded/schemas/consortium.json index baaf6d59c..f516a22ea 100644 --- a/src/encoded/schemas/consortium.json +++ b/src/encoded/schemas/consortium.json @@ -1,6 +1,6 @@ { "title": "Consortium", - "id": "/profiles/Consortium.json", + "id": "/profiles/consortium.json", "$schema": "http://json-schema.org/draft-04/schema#", "required": [ "name", diff --git a/src/encoded/schemas/submission_center.json b/src/encoded/schemas/submission_center.json index f30c21bcc..1d1ccd1f7 100644 --- a/src/encoded/schemas/submission_center.json +++ b/src/encoded/schemas/submission_center.json @@ -1,6 +1,6 @@ { "title": "SubmissionCenter", - "id": "/profiles/SubmissionCenter.json", + "id": "/profiles/submission_center.json", "$schema": "http://json-schema.org/draft-04/schema#", "required": [ "name", diff --git a/src/encoded/tests/conftest.py b/src/encoded/tests/conftest.py index f8fccac5a..43b00d3c1 100644 --- a/src/encoded/tests/conftest.py +++ b/src/encoded/tests/conftest.py @@ -95,7 +95,7 @@ def app_settings(request, wsgi_server_host_port, conn, DBSession): # noQA - We return settings -INDEXER_NAMESPACE_FOR_TESTING = generate_indexer_namespace_for_testing('cgap') +INDEXER_NAMESPACE_FOR_TESTING = generate_indexer_namespace_for_testing('smaht') @pytest.fixture(scope='session') diff --git a/src/encoded/tests/test_indexing.py b/src/encoded/tests/test_indexing.py index 651d1e768..c73d2f155 100644 --- a/src/encoded/tests/test_indexing.py +++ b/src/encoded/tests/test_indexing.py @@ -31,6 +31,7 @@ from unittest import mock from zope.sqlalchemy import mark_changed from .. import main +from .verifier import verify_item from snovault import loadxl @@ -42,7 +43,7 @@ # subset of collections to run test on -TEST_COLLECTIONS = ['file_processed'] +TEST_COLLECTIONS = ['testing_post_put_patch', 'file_processed'] def test_postgres_version(session): @@ -53,7 +54,7 @@ def test_postgres_version(session): assert re.match("PostgreSQL (%s)([.][0-9]+)? " % '|'.join(POSTGRES_COMPATIBLE_MAJOR_VERSIONS), version_info) -@pytest.fixture(scope='module') +@pytest.fixture(scope='session') def es_app(es_app_settings, request): notice_pytest_fixtures(request) # for now, don't run with mpindexer. Add `True` to params above to do so @@ -109,3 +110,176 @@ def setup_and_teardown(es_app): transaction.commit() +@pytest.fixture +def bam_format(es_testapp): + return es_testapp.post_json('/file_format', { + 'file_format': 'bam', + 'standard_file_extension': 'bam', + 'other_allowed_extensions': ['bam.gz'], + 'valid_item_types': ["FileSubmitted", "FileReference", "FileProcessed"] + }, status=201).json['@graph'][0] + + +def test_indexing_simple(es_app, setup_and_teardown, es_testapp, indexer_testapp): + """ This test (and its variants across other portals) essentially do the same thing - verify that the indexer + is working in response to post requests and that mappings are created successfully for a simple type + """ + notice_pytest_fixtures(setup_and_teardown) # unused here, but has a side-effect + + es = es_app.registry['elasticsearch'] + namespaced_ppp = get_namespaced_index(es_app, 'testing_post_put_patch') + doc_count = es.count(index=namespaced_ppp).get('count') + assert doc_count == 0 + # First post a single item so that subsequent indexing is incremental + es_testapp.post_json('/testing-post-put-patch/', {'required': ''}) + + index_n_items_for_testing(indexer_testapp, 1, max_tries=30) + + res = es_testapp.post_json('/testing-post-put-patch/', {'required': ''}) + uuid = res.json['@graph'][0]['uuid'] + + index_n_items_for_testing(indexer_testapp, 1) + + Eventually.call_assertion(make_es_count_checker(2, es=es, namespaced_index=namespaced_ppp)) + + @Eventually.consistent(tries=25, wait_seconds=1) + def check_for_uuids(): + res = es_testapp.get('/search/?type=TestingPostPutPatch') + uuids = [indv_res['uuid'] for indv_res in res.json['@graph']] + assert res.json['total'] >= 2 + assert uuid in uuids + + namespaced_indexing = get_namespaced_index(es_app, 'indexing') + indexing_doc = es.get(index=namespaced_indexing, id='latest_indexing') + indexing_source = indexing_doc['_source'] + assert 'indexing_count' in indexing_source + assert 'indexing_finished' in indexing_source + assert 'indexing_content' in indexing_source + assert indexing_source['indexing_status'] == 'finished' + assert indexing_source['indexing_count'] > 0 + testing_ppp_mappings = es.indices.get_mapping(index=namespaced_ppp)[namespaced_ppp] + assert 'mappings' in testing_ppp_mappings + testing_ppp_settings = es.indices.get_settings(index=namespaced_ppp)[namespaced_ppp] + assert 'settings' in testing_ppp_settings + # ensure we only have 1 shard for tests + assert testing_ppp_settings['settings']['index']['number_of_shards'] == '1' + + +def test_create_mapping_on_indexing(es_app, setup_and_teardown): + """ + Test overall create_mapping functionality using app. + Do this by checking es directly before and after running mapping. + Delete an index directly, run again to see if it recovers. + """ + notice_pytest_fixtures(setup_and_teardown) + es = es_app.registry[ELASTIC_SEARCH] + item_types = TEST_COLLECTIONS + # check that mappings and settings are in index + for item_type in item_types: + type_mapping(es_app.registry[TYPES], item_type) + namespaced_index = get_namespaced_index(es_app, item_type) + item_index = es.indices.get(index=namespaced_index) + found_index_mapping_emb = item_index[namespaced_index]['mappings']['properties']['embedded'] + found_index_settings = item_index[namespaced_index]['settings'] + assert found_index_mapping_emb + assert found_index_settings + # compare the manually created mapping to the one in ES + full_mapping = create_mapping_by_type(item_type, es_app.registry) + item_record = build_index_record(full_mapping, item_type) + # below is True if the found mapping matches manual one + assert compare_against_existing_mapping(es, namespaced_index, item_type, item_record, True) + + +def test_file_processed_detailed(es_app, setup_and_teardown, es_testapp, indexer_testapp, bam_format): + notice_pytest_fixtures(setup_and_teardown) + # post file_processed + item = { + 'file_format': bam_format.get('@id'), + 'filename': 'test.bam', + 'status': 'uploading' + } + fp_res = es_testapp.post_json('/file_processed', item) + test_fp_atid = fp_res.json['@graph'][0]['@id'] + test_fp_uuid = fp_res.json['@graph'][0]['uuid'] + es_testapp.post_json('/file_processed', item) + index_n_items_for_testing(indexer_testapp, 1) + + # Todo, input a list of accessions / uuids: + verify_item(test_fp_atid, indexer_testapp, es_testapp, es_app.registry) + # While we're here, test that _update of the file properly + # queues the file with given relationship + indexer_queue = es_app.registry[INDEXER_QUEUE] + rel_file = { + 'file_format': bam_format.get('@id') + } + rel_res = es_testapp.post_json('/file_processed', rel_file) + rel_uuid = rel_res.json['@graph'][0]['uuid'] + # now update the original file with the relationship + # ensure rel_file is properly queued + related_files = [{'relationship_type': 'derived from', 'file': rel_uuid}] + es_testapp.patch_json('/' + test_fp_atid, {'related_files': related_files}, status=200) + time.sleep(2) + # may need to make multiple calls to indexer_queue.receive_messages + received = [] + received_batch = None + while received_batch is None or len(received_batch) > 0: + received_batch = indexer_queue.receive_messages() + received.extend(received_batch) + to_replace = [] + to_delete = [] + found_fp_sid = None + found_rel_sid = None + # keep track of the PATCH of the original file and the associated PATCH + # of the related file. Compare uuids + for msg in received: + json_body = json.loads(msg.get('Body', {})) + if json_body['uuid'] == test_fp_uuid and json_body['method'] == 'PATCH': + found_fp_sid = json_body['sid'] + to_delete.append(msg) + elif json_body['uuid'] == rel_uuid and json_body['method'] == 'PATCH': + assert json_body['info'] == f"queued from {test_fp_uuid} _update" + found_rel_sid = json_body['sid'] + to_delete.append(msg) + else: + to_replace.append(msg) + indexer_queue.delete_messages(to_delete) + indexer_queue.replace_messages(to_replace, vis_timeout=0) + assert found_fp_sid is not None and found_rel_sid is not None + assert found_rel_sid > found_fp_sid # sid of related file is greater + + +def test_real_validation_error(es_app, setup_and_teardown, indexer_testapp, es_testapp, bam_format): + """ + Create an item (file-processed) with a validation error and index, + to ensure that validation errors work + """ + notice_pytest_fixtures(setup_and_teardown) + es = es_app.registry[ELASTIC_SEARCH] + fp_body = { + 'schema_version': '3', + 'uuid': str(uuid.uuid4()), + 'file_format': bam_format.get('uuid'), + 'file_classification': 'unprocessed file', # validation error - this enum is not present + 'higlass_uid': 1 # validation error -- higlass_uid should be string + } + res = es_testapp.post_json('/files-processed/?validate=false&upgrade=False', + fp_body, status=201).json + fp_id = res['@graph'][0]['@id'] + val_err_view = es_testapp.get(fp_id + '@@validation-errors', status=200).json + assert val_err_view['@id'] == fp_id + assert val_err_view['validation_errors'] == [] + + # call to /index will throw MissingIndexItemException multiple times, + # since associated file_format are not indexed. + # That's okay if we don't detect that it succeeded, keep trying until it does + index_n_items_for_testing(indexer_testapp, 1) + time.sleep(2) + namespaced_fp = get_namespaced_index(es_app, 'file_processed') + es_res = es.get(index=namespaced_fp, id=res['@graph'][0]['uuid']) + assert len(es_res['_source'].get('validation_errors', [])) == 2 + # check that validation-errors view works + val_err_view = es_testapp.get(fp_id + '@@validation-errors', status=200).json + assert val_err_view['@id'] == fp_id + assert val_err_view['validation_errors'] == es_res['_source']['validation_errors'] + + diff --git a/src/encoded/tests/test_ingestion_listener.py b/src/encoded/tests/test_ingestion_listener.py new file mode 100644 index 000000000..07d4cec01 --- /dev/null +++ b/src/encoded/tests/test_ingestion_listener.py @@ -0,0 +1,176 @@ +import datetime +import json +import pytest +import time + +from dcicutils.misc_utils import ignored +from uuid import uuid4 +from snovault.ingestion.ingestion_listener import ( + IngestionQueueManager, IngestionListener, +) + + +QUEUE_INGESTION_URL = '/queue_ingestion' +INGESTION_STATUS_URL = '/ingestion_status' +QUEUE_CATCH_UP_WAIT_SECONDS = 3 + + +def wait_for_queue_to_catch_up(queue_manager, n, initially=False): + """ + Wait until queue has done the things we told it to do. + + Right now this just sleeps for QUEUE_CATCH_UP_WAIT_SECONDS seconds + in any non-initial situation (i.e., where initially is False), + assuming most operations should complete within that amount of time. + """ + ignored(queue_manager, n) + if not initially: + time.sleep(QUEUE_CATCH_UP_WAIT_SECONDS) + + +def _expect_message_uuids(queue_manager, expected_uuids, max_tries=12): + checklist = set(expected_uuids) + n = len(expected_uuids) + print("Expecting: %s" % expected_uuids) + wait_for_queue_to_catch_up(queue_manager, 0, initially=True) + try_count, expected_seen, strays_seen = 0, [], [] + while True: + print(str(datetime.datetime.now()), "Try", try_count, "...") + if try_count >= max_tries: + print(str(datetime.datetime.now()), "Giving up") + break + _msgs = queue_manager.receive_messages(batch_size=1) # should reduce flakiness + _expected_messages = [] + _stray_messages = [] + for _msg in _msgs: + print("Received: %r" % _msg['Body']) + # Double-check that any message we received was ours. + uuid = json.loads(_msg['Body'])['uuid'] + if uuid in expected_uuids: + _expected_messages.append(_msg) + checklist.remove(uuid) # No longer waiting to see this + else: + _stray_messages.append(_msg) + print("Unexpected message uuid: %r" % uuid) + print(str(datetime.datetime.now()), "Received this try:", len(_msgs), + "expected", _expected_messages, "stray", _stray_messages) + expected_seen.extend(_expected_messages) + strays_seen.extend(_stray_messages) + if not checklist: # Stop if we have nothing more to look for + print(str(datetime.datetime.now()), "Got what we wanted") + break + wait_for_queue_to_catch_up(queue_manager, 0) + try_count += 1 + print(str(datetime.datetime.now()), "Total expected messages seen:", len(expected_seen)) + print(str(datetime.datetime.now()), "Total stray messages seen:", len(strays_seen)) + # The receipt of stray messages does not void our success. Some other test is probably leaving junk around. + # -kmp 12-Aug-2020 + assert len(expected_seen) == n + assert not checklist # empty set + + +class MockedEnv: + + MOCKED_ENV_PREFIX = 'smaht-other' + _last_version = 0 + + @classmethod + def new_name(cls): + cls._last_version = last_version = int(datetime.datetime.now().timestamp() * 1000000) + return "%s%s" % (cls.MOCKED_ENV_PREFIX, last_version) + + +class IngestionQueueManagerForTesting(IngestionQueueManager): + BUCKET_EXTENSION = '-ingestion-queue' # XXX: Missing from upstream class? + + def __init__(self): + + mocked_env = MockedEnv.new_name() + registry = MockedRegistry({ + 'env.name': mocked_env + }) + super().__init__(registry) + + def delete_queue(self): + """ Deletes the queue, rendering this object unusable. You have to promise not to use it after this. """ + self.client.delete_queue(QueueUrl=self.queue_url) + + +@pytest.fixture(scope='function', autouse=True) +def fresh_ingestion_queue_manager_for_testing(): + """ Yield fixture that initializes SQS and clears all messages after the each in this module. """ + queue_manager = IngestionQueueManagerForTesting() + yield queue_manager + queue_manager.delete_queue() # reclaim the AWS resource + + +class MockedRegistry: + """ Very simple mock with the single 'settings' field. """ + def __init__(self, settings): + self.settings = settings + + +def test_ingestion_queue_manager_basic(fresh_ingestion_queue_manager_for_testing): + """ Tests basic things about initializing the queue manager """ + queue_manager = fresh_ingestion_queue_manager_for_testing + # The env name will start with a constant string and have a bunch of digits. + assert queue_manager.env_name.startswith(MockedEnv.MOCKED_ENV_PREFIX) + assert queue_manager.env_name[len(MockedEnv.MOCKED_ENV_PREFIX)].isdigit() + # The queue name will have a suffix attached. + assert queue_manager.queue_name == queue_manager.env_name + queue_manager.BUCKET_EXTENSION + + # This has to be done after the above, so that there's not a gap between fixture creation and testing its name. + # This tests that each call gets a new name. + assert MockedEnv.new_name() != MockedEnv.new_name() + + +def test_ingestion_queue_add_and_receive(fresh_ingestion_queue_manager_for_testing): + """ Tests adding/receiving some uuids """ + queue_manager = fresh_ingestion_queue_manager_for_testing + test_uuids = [str(uuid4()), str(uuid4())] + print(str(datetime.datetime.now()), "test_uuids =", test_uuids) + print(str(datetime.datetime.now()), "queue_manager.queue_name =", queue_manager.queue_name) + queue_manager.add_uuids(test_uuids) + _expect_message_uuids(queue_manager, test_uuids) + + +def test_ingestion_queue_add_via_route(fresh_ingestion_queue_manager_for_testing, workbook, es_testapp): + """ Tests adding uuids to the queue via /queue_ingestion """ + queue_manager = fresh_ingestion_queue_manager_for_testing + test_uuids = [str(uuid4()), str(uuid4())] + print(str(datetime.datetime.now()), "test_uuids =", test_uuids) + print(str(datetime.datetime.now()), "queue_manager.queue_name =", queue_manager.queue_name) + request_body = { + 'uuids': test_uuids, + 'override_name': queue_manager.queue_name + } + response = es_testapp.post_json(QUEUE_INGESTION_URL, request_body).json + print(json.dumps(response, indent=2)) + assert response['notification'] == 'Success' + assert response['number_queued'] == 2 + _expect_message_uuids(queue_manager, test_uuids) + + +def test_ingestion_queue_delete(fresh_ingestion_queue_manager_for_testing, workbook, es_testapp): + """ Tests deleting messages from SQS results in no messages being there. """ + queue_manager = fresh_ingestion_queue_manager_for_testing + request_body = { + 'uuids': [str(uuid4()), str(uuid4())], + 'override_name': queue_manager.queue_name + } + es_testapp.post_json(QUEUE_INGESTION_URL, request_body, status=200) + msgs = queue_manager.receive_messages() + failed = queue_manager.delete_messages(msgs) + assert failed == [] + wait_for_queue_to_catch_up(queue_manager, 0) + + +def test_ingestion_listener_should_remain_online(fresh_ingestion_queue_manager_for_testing): + """ Tests that the 'should_remain_online' method works """ + def _await(): + time.sleep(3) + before = datetime.datetime.utcnow() + end_delta = datetime.timedelta(seconds=2) # this diff should not occur if _await is not executed + IngestionListener.should_remain_online(override=_await) + after = datetime.datetime.utcnow() + assert after > (before + end_delta) diff --git a/src/encoded/tests/test_renderers.py b/src/encoded/tests/test_renderers.py new file mode 100644 index 000000000..7c68a4cf1 --- /dev/null +++ b/src/encoded/tests/test_renderers.py @@ -0,0 +1,197 @@ +import pytest +import urllib.parse + +from dcicutils.lang_utils import n_of +from dcicutils.misc_utils import filtered_warnings +from dcicutils.qa_utils import MockResponse +from pyramid.testing import DummyRequest +from unittest import mock +from snovault import renderers +from snovault.renderers import ( + best_mime_type, should_transform, MIME_TYPES_SUPPORTED, MIME_TYPE_DEFAULT, + MIME_TYPE_JSON, MIME_TYPE_HTML, MIME_TYPE_LD_JSON, MIME_TYPE_TRIAGE_MODE, +) + + +pytestmark = [pytest.mark.setone, pytest.mark.working] + + +class DummyResponse(MockResponse): + + def __init__(self, content_type=None, status_code: int = 200, json=None, content=None, url=None, + params=None): + self.params = {} if params is None else params + self.content_type = content_type + super().__init__(status_code=status_code, json=json, content=content, url=url) + + +def test_mime_variables(): + + # Really these don't need testing but it's useful visually to remind us of their values here. + assert MIME_TYPE_HTML == 'text/html' + assert MIME_TYPE_JSON == 'application/json' + assert MIME_TYPE_LD_JSON == 'application/ld+json' + + # The MIME_TYPES_SUPPORTED is a list whose first element has elevated importance as we've structured things. + # First check that it is a list, and that its contents contain the things we support. That isn't controversial. + assert isinstance(MIME_TYPES_SUPPORTED, list) + assert set(MIME_TYPES_SUPPORTED) == {MIME_TYPE_JSON, MIME_TYPE_HTML, MIME_TYPE_LD_JSON} + # Check that the first element is consistent with the MIME_TYPE_DEFAULT. + # It's an accident of history that this next relationship matters, but at this point check for consistency. + assert MIME_TYPE_DEFAULT == MIME_TYPES_SUPPORTED[0] + # Now we concern ourselves with the actual values... + # TODO: I think it's a bug that JSON is at the head of this list (and so the default) in cgap-portal. + # cgap-portal needs to be made to match what Fourfront does to dig it out of a bug I introduced. + # -kmp 29-Jan-2022 + assert MIME_TYPES_SUPPORTED == [MIME_TYPE_JSON, MIME_TYPE_HTML, MIME_TYPE_LD_JSON] + assert MIME_TYPE_DEFAULT == MIME_TYPE_JSON + + # Regardless of whether we're using legacy mode or modern mode, we should get the same result. + assert MIME_TYPE_TRIAGE_MODE in ['legacy', 'modern'] + + +VARIOUS_MIME_TYPES_TO_TEST = ['*/*', 'text/html', 'application/json', 'application/ld+json', 'text/xml', 'who/cares'] + + +def test_best_mime_type(): + + the_constant_answer = MIME_TYPE_DEFAULT + + with filtered_warnings("ignore", category=DeprecationWarning): + # Suppresses this warning: + # DeprecationWarning: The behavior of .best_match for the Accept classes is currently being maintained + # for backward compatibility, but the method will be deprecated in the future, as its behavior is not + # specified in (and currently does not conform to) RFC 7231. + + for requested_mime_type in VARIOUS_MIME_TYPES_TO_TEST: + req = DummyRequest(headers={'Accept': requested_mime_type}) + assert best_mime_type(req, 'legacy') == the_constant_answer + assert best_mime_type(req, 'modern') == the_constant_answer + req = DummyRequest(headers={}) # The Accept header in the request just isn't being consulted + assert best_mime_type(req, 'modern') == the_constant_answer + assert best_mime_type(req, 'modern') == the_constant_answer + + +TYPICAL_URLS = [ + 'http://whatever/foo', + 'http://whatever/foo/', + 'http://whatever/foo.json', + 'http://whatever/foo.html', +] + +ALLOWED_FRAMES_OR_NONE = ['raw', 'page', 'embedded', 'object', 'bad', None] + +SOME_HTTP_METHODS = ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'HEAD'] + +ALLOWED_FORMATS_OR_NONE = ['json', 'html', None] + + +def test_should_transform(): + + passed = [] + failed = [] + problem_area = set() + + with filtered_warnings("ignore", category=DeprecationWarning): + # Suppresses this warning: + # DeprecationWarning: The behavior of .best_match for the Accept classes is currently being maintained + # for backward compatibility, but the method will be deprecated in the future, as its behavior is not + # specified in (and currently does not conform to) RFC 7231. + + for method in SOME_HTTP_METHODS: + for _format in ALLOWED_FORMATS_OR_NONE: + for requested_mime_type in VARIOUS_MIME_TYPES_TO_TEST: + for response_content_type in VARIOUS_MIME_TYPES_TO_TEST: + for frame in [None] + ALLOWED_FRAMES_OR_NONE: + for url in TYPICAL_URLS: + + params = {} + if frame is not None: + params['frame'] = frame + if _format is not None: + params['format'] = _format + + req = DummyRequest(headers={'Accept': requested_mime_type}, + method=method, + url=url, + params=params) + resp = DummyResponse(content_type=response_content_type, url=url) + + _should_transform = should_transform(req, resp) + + situation = { + 'method': method, + "format": _format, + "encoded_params": urllib.parse.urlencode(params), + "requested": requested_mime_type, + "response_content_type": response_content_type, + "frame": frame, + "url": url, + "params": params, + } + + if req.method not in ('GET', 'HEAD'): + rule_applied = "method not GET or HEAD" + correct = not _should_transform + elif resp.content_type != 'application/json': + # If the response MIME type is not application/json, + # it just can't be transformed at all. + rule_applied = "content_type is not application/json" + correct = not _should_transform + elif params.get("frame", "page") != 'page': + rule_applied = "?frame=xxx is not page" + correct = not _should_transform + elif _format is not None: + rule_applied = "?format=xxx given but not html" + correct = _should_transform is (_format == 'html') + else: + # TODO: kmp thinks this behavior is a bug. It should default to HTML. -kmp 23-Mar-2021 + correct = _should_transform is False # If no cue is given, default to JSON + + + if correct: + # There are a lot of cases, so we don't print stuff here by default, but + # uncomment to see what cases are passing as they pass: + # print(situation) + # print("=should_transform?=>", _should_transform, "(correct)") + passed.append(situation) + + else: + # There are a lot of cases, so we don't print stuff here by default, but + # uncomment to see what cases are failing as they fail: + # print(situation) + # print("=should_transform?=>", _should_transform, "(WRONG)") + failed.append(situation) + problem_area.add(rule_applied) + + if failed: + # Collect all failures in one place: + print("FAILED:") + for failure in failed: + print(" method=%(method)s format=%(format)s requested=%(requested)s" + " response_content_type=%(response_content_type)s frame=%(frame)s" + " url=%(url)s params=%(encoded_params)s" + % failure) + + n_failed = len(failed) + n_passed = len(passed) + assert not n_failed, ( + "%s passed, %s FAILED (%s: %s)" + % (n_passed, n_failed, n_of(problem_area, "problem area"), ", ".join(problem_area)) + ) + print("\n", n_passed, "combinations tried. ALL PASSED") + + +def test_should_transform_without_best_mime_type(): + + # As we call things now, we really don't need the best_mime_type function because it just returns the + # first element of its first argument. That probably should change. Because it should be a function + # of the request and its Accept offerings. Even so, we test for this now not because this makes programs + # right, but so we notice if/when this truth changes. -kmp 23-Mar-2021 + + with mock.patch.object(renderers, "best_mime_type") as mock_best_mime_type: + + # Demonstrate that best_mime_type(...) could be replaced by MIME_TYPES_SUPPORTED[0] + mock_best_mime_type.return_value = MIME_TYPES_SUPPORTED[0] + + test_should_transform() diff --git a/src/encoded/tests/test_schemas.py b/src/encoded/tests/test_schemas.py new file mode 100644 index 000000000..78c789dba --- /dev/null +++ b/src/encoded/tests/test_schemas.py @@ -0,0 +1,196 @@ +import pytest +import re + +from pkg_resources import resource_listdir +from snovault import COLLECTIONS, TYPES +from snovault.schema_utils import load_schema + + +pytestmark = [pytest.mark.setone, pytest.mark.working, pytest.mark.schema, pytest.mark.indexing] + + +SCHEMA_FILES = [ + f for f in resource_listdir('encoded', 'schemas') + if f.endswith('.json') and 'embeds' not in f +] + + +def pluralize(name): + """ This is a special function used to pluralize in a somewhat random way, but kept for legacy reasons... """ + name = name.replace("_", "-") + # deal with a few special cases explicitly + specials = ["file", "quality-metric", "summary-statistic", "workflow-run", "consortium"] + for sp in specials: + if name.startswith(sp) and re.search("-(set|flag|format|type)", name) is None: + return name.replace(sp, sp + "s") + elif name.startswith(sp) and re.search("setting", name): + return name.replace(sp, sp + "s") + elif name == "consortium": # special case for consortium + return name + # otherwise just add 's/es/ies' + if name.endswith("ly"): + return name[:-1] + "ies" + if name.endswith("sis"): + return name[:-2] + "es" + if name.endswith("s"): + return name + "es" + return name + "s" + + +@pytest.fixture(scope='module') +def master_mixins(): + return compute_master_mixins() + + +def compute_master_mixins(): + mixins = load_schema('encoded:schemas/mixins.json') + mixin_keys = [ + 'schema_version', + 'uuid', + 'accession', + 'aliases', + 'status', + 'submitted', + 'modified', + 'attribution', + 'notes', + 'documents', + 'attachment', + 'dbxrefs', + 'alternative_ids', + 'static_embeds', + 'tags', + 'facets_common', + 'supplementary_files' + ] + for key in mixin_keys: + assert mixins[key] + + +def camel_case(name): + return ''.join(x for x in name.title() if not x == '_') + + +@pytest.mark.parametrize('schema', [k for k in SCHEMA_FILES]) +def test_load_schema(schema, master_mixins, registry, testapp): + + abstract = [ + 'analysis.json', + 'file.json', + 'individual.json', + 'quality_metric.json', + 'note.json', + 'workflow_run.json', + 'user_content.json', + 'higlass_view_config.json' + ] + + loaded_schema = load_schema('encoded:schemas/%s' % schema) + assert loaded_schema + + typename = schema.replace('.json', '') + collection_names = [camel_case(typename), pluralize(typename)] + + # TODO: add pattern field checks when applicable + + # check the mixin properties for each schema + if schema != 'mixins.json': + verify_mixins(loaded_schema, master_mixins) + + if schema not in ['namespaces.json', 'mixins.json']: + # check that schema.id is same as /profiles/schema + idtag = loaded_schema['id'] + idtag = idtag.replace('/profiles/', '') + # special case for access_key.json + if schema == 'access_key.json': + idtag = idtag.replace('_admin', '') + assert schema == idtag + + # check for pluralized and camel cased in collection_names + val = None + for name in collection_names: + assert name in registry[COLLECTIONS] + if val is not None: + assert registry[COLLECTIONS][name] == val + else: + val = registry[COLLECTIONS][name] + + if schema not in abstract: + # check schema w/o json extension is in registry[TYPES] + assert typename in registry[TYPES].by_item_type + assert typename in registry[COLLECTIONS] + assert registry[COLLECTIONS][typename] == val + + shared_properties = [ + 'uuid', + 'schema_version', + 'aliases', + 'date_created', + 'submitted_by', + 'last_modified', + 'status' + ] + no_alias_or_attribution = [] + for prop in shared_properties: + if schema == 'access_key.json' and prop not in ['uuid', 'schema_version']: + continue + if schema in no_alias_or_attribution and prop in ['aliases']: + continue + verify_property(loaded_schema, prop) + + +def verify_property(loaded_schema, property): + assert loaded_schema['properties'][property] + + +def verify_mixins(loaded_schema, master_mixins): + """ + test to ensure that we didn't accidently overwrite mixins somehow + """ + for mixin in loaded_schema.get('mixinProperties', []): + # get the mixin name from {'$ref':'mixins.json#/schema_version'} + mixin_file_name, mixin_name = mixin['$ref'].split('/') + if mixin_file_name != "mixins.json": + # skip any mixins not in main mixins.json + continue + mixin_schema = master_mixins[mixin_name] + + # each field in the mixin should be present in the parent schema with same properties + for mixin_field_name, mixin_field in mixin_schema.items(): + schema_field = loaded_schema['properties'][mixin_field_name] + for key in mixin_field.keys(): + assert mixin_field[key] == schema_field[key] + + +def test_mixinProperties(): + """ Tests that a uuid mixin property shows up correctly """ + schema = load_schema('encoded:schemas/consortium.json') + assert schema['properties']['uuid']['type'] == 'string' + + +def test_changelogs(testapp, registry): + for typeinfo in registry[TYPES].by_item_type.values(): + changelog = typeinfo.schema.get('changelog') + if changelog is not None: + res = testapp.get(changelog) + assert res.status_int == 200, changelog + assert res.content_type == 'text/markdown' + + +def test_schema_version_present_on_items(app): + """Test a valid schema version is present on all non-test item + types. + + Expecting positive integer values for non-abstract items, and empty + string for all abstract items. + """ + all_types = app.registry.get(TYPES).by_item_type + for type_name, item_type in all_types.items(): + if type_name.startswith("testing"): + continue + schema_version = item_type.schema_version + if item_type.is_abstract is False: + assert schema_version + assert int(schema_version) >= 1 + else: + assert schema_version == "" diff --git a/src/encoded/tests/test_static.py b/src/encoded/tests/test_static.py new file mode 100644 index 000000000..96d4bc575 --- /dev/null +++ b/src/encoded/tests/test_static.py @@ -0,0 +1,32 @@ +import os +import pytest + +from dcicutils.qa_checkers import ChangeLogChecker, DebuggingArtifactChecker +from .conftest_settings import REPOSITORY_ROOT_DIR + + +@pytest.mark.static +def test_changelog_consistency(): + + class MyAppChangeLogChecker(ChangeLogChecker): + PYPROJECT = os.path.join(REPOSITORY_ROOT_DIR, "pyproject.toml") + CHANGELOG = os.path.join(REPOSITORY_ROOT_DIR, "CHANGELOG.rst") + + MyAppChangeLogChecker.check_version() + + +@pytest.mark.static +def test_utils_debugging_artifacts_pdb(): + checker = DebuggingArtifactChecker(sources_subdir="src/encoded", + skip_files="(tests/data)", + filter_patterns=['pdb']) + checker.check_for_debugging_patterns() + + +@pytest.mark.static +def test_utils_debugging_artifacts_print(): + checker = DebuggingArtifactChecker(sources_subdir="src/encoded", + skip_files="encoded/(commands|tests)/", + filter_patterns=['print'], + if_used='warning') + checker.check_for_debugging_patterns() diff --git a/src/encoded/tests/testing_views.py b/src/encoded/tests/testing_views.py index a2b2622c9..48163dcff 100644 --- a/src/encoded/tests/testing_views.py +++ b/src/encoded/tests/testing_views.py @@ -3,7 +3,7 @@ def includeme(config): - pass + config.scan(__name__) @collection( diff --git a/src/encoded/tests/verifier.py b/src/encoded/tests/verifier.py new file mode 100644 index 000000000..7f188047c --- /dev/null +++ b/src/encoded/tests/verifier.py @@ -0,0 +1,113 @@ +from dcicutils.misc_utils import ignored +from functools import wraps +from snovault import TYPES +from .test_create_mapping import test_create_mapping_correctly_maps_embeds +#from .test_embedding import test_add_default_embeds, test_manual_embeds +#from .test_schemas import compute_master_mixins, test_load_schema + + +def verifier(func): + @wraps(func) + def wrapper(*args, **kwargs): + print("running tests " + func.__name__) + try: + res = func(*args, **kwargs) + except Exception as e: + print("test failed with exception " + str(e.args)) + else: + print("success") + return res + return wrapper + + +@verifier +def verify_get_from_es(item_uuid, indexer_testapp, registry): + ignored(registry) + # get from elasticsearch + es_item = indexer_testapp.get("/" + item_uuid + "/?datastore=elasticsearch").maybe_follow(status=200).json + item_type = es_item['@type'][0] + ensure_basic_data(es_item, item_type) + return es_item, item_type + + +@verifier +def verify_get_by_accession(es_item, item_type, indexer_testapp): + # get by accession + accession = es_item.get('accession') + if accession: # some items don't have acessions + item_by_accession = indexer_testapp.get("/" + accession).follow(status=200).json + ensure_basic_data(item_by_accession, item_type) + + +@verifier +def verify_get_from_db(item_uuid, item_type, indexer_testapp): + # get from database + db_item = indexer_testapp.get("/" + item_uuid + "/?datastore=database").follow(status=200).json + ensure_basic_data(db_item, item_type) + + +@verifier +def verify_profile(item_type, indexer_testapp): + # is this something we actually know about? + profile = indexer_testapp.get("/profiles/" + item_type + ".json").json + assert profile + item_type_camel = profile['id'].strip('.json').split('/')[-1] + return item_type_camel + + +@verifier +def verify_schema(item_type_camel, registry): + # test schema + test_load_schema(item_type_camel + ".json", compute_master_mixins(), registry) + + +@verifier +def verify_can_embed(item_type_camel, es_item, indexer_testapp, registry): + # get the embedds + pyr_item_type = registry[TYPES].by_item_type[item_type_camel] + embeds = pyr_item_type.embedded + + assert embeds == pyr_item_type.factory.embedded + got_embeds = indexer_testapp.get(es_item['@id'] + "@@embedded").json + assert got_embeds + + +@verifier +def verify_indexing(item_uuid, indexer_testapp): + # test indexing this bad by + res = indexer_testapp.get("/" + item_uuid + "/@@index-data") + assert res + + +@verifier +def verify_embeds(registry, item_type): + test_add_default_embeds(registry, item_type) + test_manual_embeds(registry, item_type) + + +@verifier +def verify_mapping(registry, item_type): + test_create_mapping_correctly_maps_embeds(registry, item_type) + + +def verify_item(item_uuid, indexer_testapp, testapp, registry): + ignored(testapp) + es_item, item_type = verify_get_from_es(item_uuid, indexer_testapp, registry) + verify_get_by_accession(es_item, item_type, indexer_testapp) + verify_get_from_db(item_uuid, item_type, indexer_testapp) + item_type_camel = verify_profile(item_type, indexer_testapp) + verify_schema(item_type_camel, registry) + verify_can_embed(item_type_camel, es_item, indexer_testapp, registry) + verify_indexing(item_uuid, indexer_testapp) + verify_embeds(registry, item_type_camel) + verify_mapping(registry, item_type_camel) + + +def ensure_basic_data(item_data, item_type=None): + # ensure we have basic identifing properties + assert item_data + if not item_type: + item_type = item_data['@type'][0] + assert item_data['uuid'] + assert item_data['@id'] + assert item_type in item_data['@type'] From 33fb5bf03b29e394244f1e3f8e6a3791de745294 Mon Sep 17 00:00:00 2001 From: William Ronchetti Date: Wed, 19 Jul 2023 14:32:15 -0400 Subject: [PATCH 3/7] numerous changes to accommodate working search --- src/encoded/tests/test_search.py | 95 ++++++++++++++++++++++++ src/encoded/types/access_key.py | 3 +- src/encoded/types/base.py | 34 +++++---- src/encoded/types/consortium.py | 2 +- src/encoded/types/document.py | 3 +- src/encoded/types/file.py | 4 +- src/encoded/types/file_format.py | 3 +- src/encoded/types/filter_set.py | 3 +- src/encoded/types/higlass_view_config.py | 3 +- src/encoded/types/image.py | 3 +- src/encoded/types/meta_workflow.py | 3 +- src/encoded/types/page.py | 3 +- src/encoded/types/quality_metric.py | 4 +- src/encoded/types/software.py | 3 +- src/encoded/types/static_section.py | 3 +- src/encoded/types/submission_center.py | 2 +- src/encoded/types/tracking_item.py | 3 +- src/encoded/types/user.py | 3 +- src/encoded/types/user_content.py | 3 +- src/encoded/types/workflow.py | 3 +- 20 files changed, 152 insertions(+), 31 deletions(-) diff --git a/src/encoded/tests/test_search.py b/src/encoded/tests/test_search.py index 1d563c2a0..39edf3ed8 100644 --- a/src/encoded/tests/test_search.py +++ b/src/encoded/tests/test_search.py @@ -1,3 +1,24 @@ +import json +import pytest +import webtest + +from datetime import datetime, timedelta +from dcicutils.misc_utils import Retry, ignored, local_attrs +from dcicutils.qa_utils import notice_pytest_fixtures +from pyramid.httpexceptions import HTTPBadRequest +from snovault import TYPES, COLLECTIONS +from snovault.elasticsearch import create_mapping +from snovault.elasticsearch.indexer_utils import get_namespaced_index +from snovault.schema_utils import load_schema +from snovault.util import add_default_embeds +from webtest import AppError +from snovault.search.lucene_builder import LuceneBuilder +from snovault.search.search_utils import find_nested_path + + +pytestmark = [pytest.mark.indexing] + + class MockedRequest(object): """ Test object intended to be used to mock certain aspects of requests. Takes kwargs which will be passed as named fields to MockedRequest. More arguments could be added if other @@ -8,3 +29,77 @@ def __init__(self, **kwargs): self.effective_principals = ['system.Everyone'] else: self.effective_principals = kwargs['principals_allowed'] # note this is not exactly what the field is + + +def test_search_view(workbook, es_testapp): + """ Test basic things about search view """ + res = es_testapp.get('/search/?type=Item').json + assert res['@type'] == ['ItemSearchResults', 'Search'] + assert res['@id'] == '/search/?type=Item' + assert res['@context'] == '/terms/' + assert res['notification'] == 'Success' + assert res['title'] == 'Search' + assert res['total'] > 0 + assert 'facets' in res + + # type facet should always have > 1 option here, even when it is selected + for facet in res['facets']: + if facet['field'] == 'type': + assert len(facet['terms']) > 1 + break + assert 'filters' in res + assert '@graph' in res + + +def test_search_with_no_query(workbook, es_testapp): + """ + using /search/ (with no query) should default to /search/?type=Item + thus, should satisfy same assertions as test_search_view + """ + notice_pytest_fixtures(workbook) + res = es_testapp.get('/search/').follow(status=200) + assert res.json['@type'] == ['ItemSearchResults', 'Search'] + assert res.json['@id'] == '/search/?type=Item' + assert res.json['@context'] == '/terms/' + assert res.json['notification'] == 'Success' + assert res.json['title'] == 'Search' + assert res.json['total'] > 0 + assert 'facets' in res + # test default facets (data type) + default_facets = [facet['field'] for facet in res.json['facets']] + assert 'type' in default_facets + # assert 'status' in default_facets uncomment this if status is added back -Will 5/13/2020 + assert 'filters' in res + assert '@graph' in res + + +def test_collections_redirect_to_search(workbook, es_testapp): + """ + we removed the collections page and redirect to search of that type + redirected_from is not used for search + """ + res = es_testapp.get('/user/', status=301).follow(status=200) + assert res.json['@type'] == ['UserSearchResults', 'ItemSearchResults', 'Search'] + assert res.json['@id'] == '/search/?type=User' + assert 'redirected_from' not in res.json['@id'] + assert res.json['@context'] == '/terms/' + assert res.json['notification'] == 'Success' + assert res.json['title'] == 'Search' + assert res.json['total'] > 0 + assert 'facets' in res + assert 'filters' in res + assert '@graph' in res + + +def test_collection_limit(workbook, es_testapp): + res = es_testapp.get('/user/?limit=1', status=301) + assert len(res.follow().json['@graph']) == 1 + + +def test_collection_actions_filtered_by_permission(workbook, es_testapp, anon_es_testapp): + res = es_testapp.get('/user/') + assert any(action for action in res.follow().json.get('actions', []) if action['name'] == 'add') + + # users not visible + res = anon_es_testapp.get('/user/', status=404) + assert len(res.json['@graph']) == 0 diff --git a/src/encoded/types/access_key.py b/src/encoded/types/access_key.py index 1a27878a4..718594557 100644 --- a/src/encoded/types/access_key.py +++ b/src/encoded/types/access_key.py @@ -10,7 +10,8 @@ access_key_reset_secret as sno_access_key_reset_secret, access_key_view_raw as sno_access_key_view_raw ) -from .base import SMAHTItem, mixin_smaht_permission_types, DELETED_ACL +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types, DELETED_ACL from .acl import ALLOW_AUTHENTICATED_CREATE_ACL, ONLY_ADMIN_VIEW_ACL diff --git a/src/encoded/types/base.py b/src/encoded/types/base.py index 9eaad2952..f241ca67e 100644 --- a/src/encoded/types/base.py +++ b/src/encoded/types/base.py @@ -1,10 +1,10 @@ from pyramid.view import view_config from snovault import AbstractCollection, abstract_collection, calculated_property from snovault.types.base import ( - Item, Collection, DELETED_ACL, ) +from snovault.types.base import Item as SnovaultItem from snovault.util import debug_log from snovault.validators import ( validate_item_content_post, @@ -52,16 +52,19 @@ class SMAHTCollection(Collection, AbstractCollection): def __init__(self, *args, **kw): super(Collection, self).__init__(*args, **kw) if hasattr(self, '__acl__'): - PRINT(f'DEBUG_PERMISSIONS: returning {self.__acl__} for {self.type_info.name}') + if DEBUG_PERMISSIONS: + PRINT(f'DEBUG_PERMISSIONS: returning {self.__acl__} for {self.type_info.name}') return # If no ACLs are defined for collection, allow submission centers to add/create if 'submission_centers' in self.type_info.factory.schema['properties']: - PRINT(f'DEBUG_PERMISSIONS: returning {ALLOW_SUBMISSION_CENTER_CREATE_ACL} for {self.type_info.name}') + if DEBUG_PERMISSIONS: + PRINT(f'DEBUG_PERMISSIONS: returning {ALLOW_SUBMISSION_CENTER_CREATE_ACL} for {self.type_info.name}') self.__acl__ = ALLOW_SUBMISSION_CENTER_CREATE_ACL else: self.__acl__ = ONLY_ADMIN_VIEW_ACL - PRINT(f'DEBUG_PERMISSIONS: using admin acl for {self.type_info.name}') + if DEBUG_PERMISSIONS: + PRINT(f'DEBUG_PERMISSIONS: using admin acl for {self.type_info.name}') @abstract_collection( @@ -71,7 +74,12 @@ def __init__(self, *args, **kw): 'description': 'Abstract collection of all SMaHT Items.', } ) -class SMAHTItem(Item): +class Item(SnovaultItem): + """ Note: originally denoted SMAHTItem, this rename breaks many things downstream in our type resolution + system, and generally varying the name in the class from the type definition name below (item_type) does + not work as you would intend - there is not really any reason to allow this setting and should just default + to the snake case version of self.__name__ + """ item_type = 'item' AbstractCollection = AbstractCollection Collection = SMAHTCollection @@ -146,7 +154,7 @@ def __ac_local_roles__(self): return roles -@calculated_property(context=SMAHTItem.AbstractCollection, category='action') +@calculated_property(context=Item.AbstractCollection, category='action') def add(context, request): """smth.""" if request.has_permission('add', context): @@ -159,7 +167,7 @@ def add(context, request): } -@calculated_property(context=SMAHTItem, category='action') +@calculated_property(context=Item, category='action') def edit(context, request): """smth.""" if request.has_permission('edit'): @@ -171,7 +179,7 @@ def edit(context, request): } -@calculated_property(context=SMAHTItem, category='action') +@calculated_property(context=Item, category='action') def create(context, request): if request.has_permission('create'): return { @@ -201,17 +209,17 @@ def collection_add(context, request, render=None): return sno_collection_add(context, request, render) -@view_config(context=SMAHTItem, permission='edit', request_method='PUT', +@view_config(context=Item, permission='edit', request_method='PUT', validators=[validate_item_content_put]) -@view_config(context=SMAHTItem, permission='edit', request_method='PATCH', +@view_config(context=Item, permission='edit', request_method='PATCH', validators=[validate_item_content_patch]) -@view_config(context=SMAHTItem, permission='edit_unvalidated', request_method='PUT', +@view_config(context=Item, permission='edit_unvalidated', request_method='PUT', validators=[no_validate_item_content_put], request_param=['validate=false']) -@view_config(context=SMAHTItem, permission='edit_unvalidated', request_method='PATCH', +@view_config(context=Item, permission='edit_unvalidated', request_method='PATCH', validators=[no_validate_item_content_patch], request_param=['validate=false']) -@view_config(context=SMAHTItem, permission='index', request_method='GET', +@view_config(context=Item, permission='index', request_method='GET', validators=[validate_item_content_in_place], request_param=['check_only=true']) @debug_log diff --git a/src/encoded/types/consortium.py b/src/encoded/types/consortium.py index 451abb0c1..05ef741db 100644 --- a/src/encoded/types/consortium.py +++ b/src/encoded/types/consortium.py @@ -1,6 +1,6 @@ from snovault import collection from snovault import load_schema -from .base import SMAHTItem +from .base import Item as SMAHTItem from .acl import ONLY_ADMIN_VIEW_ACL diff --git a/src/encoded/types/document.py b/src/encoded/types/document.py index dc66b552f..abdbb0a0c 100644 --- a/src/encoded/types/document.py +++ b/src/encoded/types/document.py @@ -1,7 +1,8 @@ from copy import deepcopy from snovault import collection from encoded_core.types.document import Document as CoreDocument -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types from .acl import CONSORTIUM_MEMBER_CREATE_ACL diff --git a/src/encoded/types/file.py b/src/encoded/types/file.py index 0097f7394..90839bbed 100644 --- a/src/encoded/types/file.py +++ b/src/encoded/types/file.py @@ -4,7 +4,8 @@ from encoded_core.types.file_submitted import FileSubmitted as CoreFileSubmitted from encoded_core.types.file_reference import FileReference as CoreFileReference from encoded_core.types.file_processed import FileProcessed as CoreFileProcessed -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types from .acl import ONLY_ADMIN_VIEW_ACL @@ -22,6 +23,7 @@ 'description': 'Listing of Files', }) class File(SMAHTItem, CoreFile): + item_type = 'file' schema = mixin_smaht_permission_types(ENCODED_CORE_FILE_SCHEMA) diff --git a/src/encoded/types/file_format.py b/src/encoded/types/file_format.py index 59e40c819..06d8dbfd9 100644 --- a/src/encoded/types/file_format.py +++ b/src/encoded/types/file_format.py @@ -1,7 +1,8 @@ from snovault import collection from copy import deepcopy from encoded_core.types.file_format import FileFormat as CoreFileFormat -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types from .acl import ONLY_ADMIN_VIEW_ACL diff --git a/src/encoded/types/filter_set.py b/src/encoded/types/filter_set.py index a07f1ea29..60cfcf0c2 100644 --- a/src/encoded/types/filter_set.py +++ b/src/encoded/types/filter_set.py @@ -1,7 +1,8 @@ from copy import deepcopy from snovault import collection from snovault.types.filter_set import FilterSet as SnovaultFilterSet -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types from .acl import ALLOW_CONSORTIUM_CREATE_ACL diff --git a/src/encoded/types/higlass_view_config.py b/src/encoded/types/higlass_view_config.py index f462fb309..289acc5c9 100644 --- a/src/encoded/types/higlass_view_config.py +++ b/src/encoded/types/higlass_view_config.py @@ -1,7 +1,8 @@ from copy import deepcopy from snovault import collection from encoded_core.types.higlass_view_config import HiglassViewConfig as CoreHiglassViewConfig -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types ENCODED_CORE_HIGLASS_VIEW_CONFIG_SCHEMA = deepcopy(CoreHiglassViewConfig.schema) diff --git a/src/encoded/types/image.py b/src/encoded/types/image.py index 3ed0d1290..b3a2b8dc9 100644 --- a/src/encoded/types/image.py +++ b/src/encoded/types/image.py @@ -1,7 +1,8 @@ from copy import deepcopy from snovault import collection from encoded_core.types.image import Image as CoreImage -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types ENCODED_CORE_IMAGE_SCHEMA = deepcopy(CoreImage.schema) diff --git a/src/encoded/types/meta_workflow.py b/src/encoded/types/meta_workflow.py index b88f52de1..ca97cf789 100644 --- a/src/encoded/types/meta_workflow.py +++ b/src/encoded/types/meta_workflow.py @@ -2,7 +2,8 @@ from snovault import collection from encoded_core.types.meta_workflow import MetaWorkflow as CoreMetaWorkflow from encoded_core.types.meta_workflow import MetaWorkflowRun as CoreMetaWorkflowRun -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types ENCODED_CORE_META_WORKFLOW_SCHEMA = deepcopy(CoreMetaWorkflow.schema) diff --git a/src/encoded/types/page.py b/src/encoded/types/page.py index 878865a55..17df15851 100644 --- a/src/encoded/types/page.py +++ b/src/encoded/types/page.py @@ -1,7 +1,8 @@ from copy import deepcopy from snovault import collection from encoded_core.types.page import Page as CorePage -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types ENCODED_CORE_PAGE_SCHEMA = deepcopy(CorePage.schema) diff --git a/src/encoded/types/quality_metric.py b/src/encoded/types/quality_metric.py index 8f98e6ad1..ebe2a7a3c 100644 --- a/src/encoded/types/quality_metric.py +++ b/src/encoded/types/quality_metric.py @@ -2,7 +2,8 @@ from snovault import collection, abstract_collection from encoded_core.types.quality_metric import QualityMetric as CoreQualityMetric from encoded_core.types.quality_metric import QualityMetricGeneric as CoreQualityMetricGeneric -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types ENCODED_CORE_QC_GENERAL_SCHEMA = deepcopy(CoreQualityMetricGeneric.schema) @@ -16,6 +17,7 @@ 'description': 'Listing of quality metrics', }) class QualityMetric(SMAHTItem): + item_type = 'quality_metric' schema = mixin_smaht_permission_types(ENCODED_CORE_QC_SCHEMA) diff --git a/src/encoded/types/software.py b/src/encoded/types/software.py index 6913159c0..599537123 100644 --- a/src/encoded/types/software.py +++ b/src/encoded/types/software.py @@ -1,7 +1,8 @@ from copy import deepcopy from snovault import collection from encoded_core.types.software import Software as CoreSoftware -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types ENCODED_CORE_SOFTWARE_SCHEMA = deepcopy(CoreSoftware.schema) diff --git a/src/encoded/types/static_section.py b/src/encoded/types/static_section.py index 49d62eeac..e488fc295 100644 --- a/src/encoded/types/static_section.py +++ b/src/encoded/types/static_section.py @@ -1,7 +1,8 @@ from copy import deepcopy from snovault import collection from encoded_core.types.user_content import StaticSection as CoreStaticSection -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types ENCODED_CORE_STATIC_SECTION_SCHEMA = deepcopy(CoreStaticSection.schema) diff --git a/src/encoded/types/submission_center.py b/src/encoded/types/submission_center.py index 1d1622f23..cfb82a840 100644 --- a/src/encoded/types/submission_center.py +++ b/src/encoded/types/submission_center.py @@ -1,5 +1,5 @@ from snovault import collection, load_schema -from .base import SMAHTItem +from .base import Item as SMAHTItem @collection( diff --git a/src/encoded/types/tracking_item.py b/src/encoded/types/tracking_item.py index 28b1a5ad5..595431c4b 100644 --- a/src/encoded/types/tracking_item.py +++ b/src/encoded/types/tracking_item.py @@ -1,7 +1,8 @@ from copy import deepcopy from snovault import collection from encoded_core.types.tracking_item import TrackingItem as CoreTrackingItem -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types ENCODED_CORE_TRACKING_ITEM_SCHEMA = deepcopy(CoreTrackingItem.schema) diff --git a/src/encoded/types/user.py b/src/encoded/types/user.py index b14cf2a1f..d831ad7cb 100644 --- a/src/encoded/types/user.py +++ b/src/encoded/types/user.py @@ -1,7 +1,8 @@ from snovault import collection from snovault.types.user import User as SnovaultUser from copy import deepcopy -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types SNOVAULT_USER_SCHEMA = deepcopy(SnovaultUser.schema) diff --git a/src/encoded/types/user_content.py b/src/encoded/types/user_content.py index 4f32ecf6b..2ae35fb4a 100644 --- a/src/encoded/types/user_content.py +++ b/src/encoded/types/user_content.py @@ -1,7 +1,8 @@ from copy import deepcopy from snovault import abstract_collection from encoded_core.types.user_content import UserContent as CoreUserContent -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types ENCODED_CORE_USER_CONTENT_SCHEMA = deepcopy(CoreUserContent.schema) diff --git a/src/encoded/types/workflow.py b/src/encoded/types/workflow.py index 2c1060ab3..35df5be38 100644 --- a/src/encoded/types/workflow.py +++ b/src/encoded/types/workflow.py @@ -3,7 +3,8 @@ from encoded_core.types.workflow import Workflow as CoreWorkflow from encoded_core.types.workflow import WorkflowRun as CoreWorkflowRun from encoded_core.types.workflow import WorkflowRunAwsem as CoreWorkflowRunAwsem -from .base import SMAHTItem, mixin_smaht_permission_types +from .base import Item as SMAHTItem +from .base import mixin_smaht_permission_types ENCODED_CORE_WORKFLOW_SCHEMA = deepcopy(CoreWorkflow.schema) From defd45370191d706b5b6e8e763d2cf5ca32cb61b Mon Sep 17 00:00:00 2001 From: William Ronchetti Date: Wed, 19 Jul 2023 15:01:14 -0400 Subject: [PATCH 4/7] build configuration --- .github/workflows/main.yml | 20 +++++++++++++++++--- Makefile | 5 ++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 548bf05b5..bdbde5b54 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -22,6 +22,11 @@ jobs: # The type of runner that the job will run on runs-on: ubuntu-20.04 + # Build matrix + strategy: + matrix: + test_type: [ 'UNIT', 'DOCKER' ] + # Steps represent a sequence of tasks that will be executed as part of the job steps: # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it @@ -31,6 +36,7 @@ jobs: python-version: 3.9 - name: Install/Link Postgres + if: ${{ matrix.test_type == 'UNIT' }} run: | sudo apt-get install curl ca-certificates gnupg curl https://www.postgresql.org/media/keys/ACCC4CF8.asc | sudo apt-key add - @@ -41,6 +47,7 @@ jobs: sudo ln -s /usr/lib/postgresql/11/bin/initdb /usr/local/bin/initdb - name: Install Deps + if: ${{ matrix.test_type == 'UNIT' }} run: | make build @@ -51,7 +58,8 @@ jobs: aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} aws-region: us-east-1 - - name: QA + - name: QA (UNIT) + if: ${{ matrix.test_type == 'UNIT' }} env: # S3_ENCRYPT_KEY: ${{ secrets.S3_ENCRYPT_KEY }} # # The need for this old environment variable name will go away soon. @@ -59,7 +67,13 @@ jobs: # This will be the new environment variable name. TEST_JOB_ID: smaht-test-indexing-${{ github.run_number }}-${{ matrix.python_version }}- run: | - make test + make remote-test + + - name: Docker Build + if: ${{ matrix.test_type == 'DOCKER' }} + run: | + touch deploy/docker/local/docker_development.ini # cheap substitute for prepare-docker to make ignored file + docker build . - name: Cleanup if: ${{ always() }} @@ -68,6 +82,6 @@ jobs: # The need for this old environment variable name will go away soon. # TRAVIS_JOB_ID: sno-x-test-${{ github.run_number }}- # This will be the new environment variable name. - TEST_JOB_ID: sno-test-indexing-${{ github.run_number }}-${{ matrix.python_version }}- + TEST_JOB_ID: smaht-test-indexing-${{ github.run_number }}-${{ matrix.python_version }}- run: | poetry run wipe-test-indices $TEST_JOB_ID search-fourfront-testing-opensearch-kqm7pliix4wgiu4druk2indorq.us-east-1.es.amazonaws.com:443 diff --git a/Makefile b/Makefile index 385048db8..99e8a2562 100644 --- a/Makefile +++ b/Makefile @@ -169,9 +169,8 @@ test-static: poetry run python -m pytest -vv -m static make lint -remote-test: # Actually, we don't normally use this. Instead the GA workflow sets up two parallel tests. - make remote-test-unit - make remote-test-npm +remote-test: # smaht-portal uses this make target for now as tests are not that burdensome + pytest -vv --aws-auth --durations=20 --es search-opensearch-smaht-testing-ykavtw57jz4cx4f2gqewhu4b44.us-east-1.es.amazonaws.com:443 remote-test-npm: # Note this only does the 'not indexing' tests poetry run python -m pytest -xvv -r w --instafail --force-flaky --max-runs=2 --timeout=600 -m "not manual and not integratedx and not performance and not broken and not broken_remotely and not sloppy and not indexing and not static" --aws-auth --durations=20 --cov src/encoded --es search-cgap-unit-testing-opensearch-tcs45cjpwgdzoi7pafr6oewq6u.us-east-1.es.amazonaws.com:443 From 6b8b2fc740ffcdfc47ebab11e5bdde49492fbb94 Mon Sep 17 00:00:00 2001 From: William Ronchetti Date: Wed, 19 Jul 2023 15:10:55 -0400 Subject: [PATCH 5/7] version, cleanup fix --- .github/workflows/main.yml | 2 +- CHANGELOG.rst | 8 ++++++++ poetry.lock | 9 ++++----- pyproject.toml | 4 ++-- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index bdbde5b54..e595c9bff 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -84,4 +84,4 @@ jobs: # This will be the new environment variable name. TEST_JOB_ID: smaht-test-indexing-${{ github.run_number }}-${{ matrix.python_version }}- run: | - poetry run wipe-test-indices $TEST_JOB_ID search-fourfront-testing-opensearch-kqm7pliix4wgiu4druk2indorq.us-east-1.es.amazonaws.com:443 + poetry run wipe-test-indices $TEST_JOB_ID search-opensearch-smaht-testing-ykavtw57jz4cx4f2gqewhu4b44.us-east-1.es.amazonaws.com:443 diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f44eed3d9..457fe1f53 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,14 @@ smaht-portal Change Log ---------- +0.0.5 +===== + +* Improve testing by porting relevant tests as needed +* Changes to accommodate working search/other tests +* Allow testing with ES in GA with smaht-development credentials +* Build Docker as part of GA + 0.0.4 ===== diff --git a/poetry.lock b/poetry.lock index a93d3ef76..449535fec 100644 --- a/poetry.lock +++ b/poetry.lock @@ -865,14 +865,14 @@ test-randomorder = ["pytest-randomly"] [[package]] name = "dcicsnovault" -version = "8.2.0.1b42" +version = "8.2.0.1b45" description = "Storage support for 4DN Data Portals." category = "main" optional = false python-versions = ">=3.8.1,<3.10" files = [ - {file = "dcicsnovault-8.2.0.1b42-py3-none-any.whl", hash = "sha256:b34b9899384b8ed02004e169a73b50232274e6603a177f6e12588d5297b37424"}, - {file = "dcicsnovault-8.2.0.1b42.tar.gz", hash = "sha256:afdc4972a1cc2148afd1ab3d36c00f74d024b4e2311be56574b43a613a89f375"}, + {file = "dcicsnovault-8.2.0.1b45-py3-none-any.whl", hash = "sha256:86e79026a3108366c1215113eac6cb9c776142370d9c49b6693bd72382eafbc4"}, + {file = "dcicsnovault-8.2.0.1b45.tar.gz", hash = "sha256:16931980239377e5bfd7b6b55b3870f82e8400ba54ca895cae6d8cc825a1fb97"}, ] [package.dependencies] @@ -894,7 +894,6 @@ psycopg2-binary = ">=2.9.1,<3.0.0" PyBrowserID = ">=0.10.0,<1" pyjwt = ">=2.6.0,<3.0.0" pyramid = "1.10.4" -pyramid_localroles = ">=0.1,<1" pyramid-multiauth = ">=0.9.0,<1" pyramid-retry = ">=1.0,<2.0" pyramid-tm = ">=2.5,<3.0" @@ -3133,4 +3132,4 @@ test = ["zope.testing"] [metadata] lock-version = "2.0" python-versions = ">=3.9.1,<3.10" -content-hash = "f077ec928233983ca83ad637620e2c91c1b0faaaa8ca4b5c8de8664dd37245aa" +content-hash = "4f8b19950eebeff4454807dfecc04db694f0d7df91a478dda22588d4cf90cd00" diff --git a/pyproject.toml b/pyproject.toml index 7c4b77e98..aab6b4f5c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "encoded" -version = "0.0.4" +version = "0.0.5" description = "SMaHT Data Analysis Portal" authors = ["4DN-DCIC Team "] license = "MIT" @@ -42,7 +42,7 @@ certifi = ">=2021.5.30" chardet = "3.0.4" codeguru-profiler-agent = "^1.2.4" colorama = "0.3.3" -dcicsnovault = "8.2.0.1b42" +dcicsnovault = "8.2.0.1b45" dcicutils = "7.5.1.1b0" encoded-core = "^0.0.2" elasticsearch = "7.13.4" From 9df081061943607d1b57d24f20e35e7726c6bbac Mon Sep 17 00:00:00 2001 From: William Ronchetti Date: Wed, 19 Jul 2023 15:18:33 -0400 Subject: [PATCH 6/7] dont cleanup on Docker --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index e595c9bff..a9103a87f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -76,7 +76,7 @@ jobs: docker build . - name: Cleanup - if: ${{ always() }} + if: ${{ always() && matrix.test_type == 'UNIT' }} env: S3_ENCRYPT_KEY: ${{ secrets.S3_ENCRYPT_KEY }} # The need for this old environment variable name will go away soon. From b554fccde31adb244d1c5814fb60787df14bd860 Mon Sep 17 00:00:00 2001 From: William Ronchetti Date: Fri, 21 Jul 2023 14:34:17 -0400 Subject: [PATCH 7/7] take some changes from Kent --- src/encoded/tests/test_ingestion_listener.py | 4 +-- src/encoded/tests/verifier.py | 35 ++++++++++---------- src/encoded/types/base.py | 4 +-- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/encoded/tests/test_ingestion_listener.py b/src/encoded/tests/test_ingestion_listener.py index 07d4cec01..e986950df 100644 --- a/src/encoded/tests/test_ingestion_listener.py +++ b/src/encoded/tests/test_ingestion_listener.py @@ -17,7 +17,7 @@ def wait_for_queue_to_catch_up(queue_manager, n, initially=False): """ - Wait until queue has done the things we told it to do. + Pause for enough time that probably the queue has done the things we told it to do. Right now this just sleeps for QUEUE_CATCH_UP_WAIT_SECONDS seconds in any non-initial situation (i.e., where initially is False), @@ -76,7 +76,7 @@ class MockedEnv: @classmethod def new_name(cls): - cls._last_version = last_version = int(datetime.datetime.now().timestamp() * 1000000) + cls._last_version = last_version = int(datetime.datetime.now().timestamp() * 1000000) # microseconds return "%s%s" % (cls.MOCKED_ENV_PREFIX, last_version) diff --git a/src/encoded/tests/verifier.py b/src/encoded/tests/verifier.py index 7f188047c..038be6c17 100644 --- a/src/encoded/tests/verifier.py +++ b/src/encoded/tests/verifier.py @@ -1,21 +1,21 @@ -from dcicutils.misc_utils import ignored +from dcicutils.misc_utils import ignored, PRINT from functools import wraps from snovault import TYPES from .test_create_mapping import test_create_mapping_correctly_maps_embeds -#from .test_embedding import test_add_default_embeds, test_manual_embeds -#from .test_schemas import compute_master_mixins, test_load_schema +# from .test_embedding import test_add_default_embeds, test_manual_embeds +# from .test_schemas import compute_master_mixins, test_load_schema def verifier(func): @wraps(func) def wrapper(*args, **kwargs): - print("running tests " + func.__name__) + PRINT(f"running tests {func.__name__}") try: res = func(*args, **kwargs) except Exception as e: - print("test failed with exception " + str(e.args)) + PRINT(f"test failed with exception {e}") else: - print("success") + PRINT("success") return res return wrapper @@ -51,19 +51,20 @@ def verify_profile(item_type, indexer_testapp): # is this something we actually know about? profile = indexer_testapp.get("/profiles/" + item_type + ".json").json assert profile + # transform profile page path to camel case item type item_type_camel = profile['id'].strip('.json').split('/')[-1] return item_type_camel -@verifier -def verify_schema(item_type_camel, registry): - # test schema - test_load_schema(item_type_camel + ".json", compute_master_mixins(), registry) +# @verifier +# def verify_schema(item_type_camel, registry): +# # test schema +# test_load_schema(item_type_camel + ".json", compute_master_mixins(), registry) @verifier def verify_can_embed(item_type_camel, es_item, indexer_testapp, registry): - # get the embedds + # get the embeds pyr_item_type = registry[TYPES].by_item_type[item_type_camel] embeds = pyr_item_type.embedded @@ -79,10 +80,10 @@ def verify_indexing(item_uuid, indexer_testapp): assert res -@verifier -def verify_embeds(registry, item_type): - test_add_default_embeds(registry, item_type) - test_manual_embeds(registry, item_type) +# @verifier +# def verify_embeds(registry, item_type): +# test_add_default_embeds(registry, item_type) +# test_manual_embeds(registry, item_type) @verifier @@ -96,10 +97,10 @@ def verify_item(item_uuid, indexer_testapp, testapp, registry): verify_get_by_accession(es_item, item_type, indexer_testapp) verify_get_from_db(item_uuid, item_type, indexer_testapp) item_type_camel = verify_profile(item_type, indexer_testapp) - verify_schema(item_type_camel, registry) + # verify_schema(item_type_camel, registry) verify_can_embed(item_type_camel, es_item, indexer_testapp, registry) verify_indexing(item_uuid, indexer_testapp) - verify_embeds(registry, item_type_camel) + # verify_embeds(registry, item_type_camel) verify_mapping(registry, item_type_camel) diff --git a/src/encoded/types/base.py b/src/encoded/types/base.py index f241ca67e..afb5c3313 100644 --- a/src/encoded/types/base.py +++ b/src/encoded/types/base.py @@ -53,13 +53,13 @@ def __init__(self, *args, **kw): super(Collection, self).__init__(*args, **kw) if hasattr(self, '__acl__'): if DEBUG_PERMISSIONS: - PRINT(f'DEBUG_PERMISSIONS: returning {self.__acl__} for {self.type_info.name}') + PRINT(f'DEBUG_PERMISSIONS: using {self.__acl__} for {self.type_info.name}') return # If no ACLs are defined for collection, allow submission centers to add/create if 'submission_centers' in self.type_info.factory.schema['properties']: if DEBUG_PERMISSIONS: - PRINT(f'DEBUG_PERMISSIONS: returning {ALLOW_SUBMISSION_CENTER_CREATE_ACL} for {self.type_info.name}') + PRINT(f'DEBUG_PERMISSIONS: using {ALLOW_SUBMISSION_CENTER_CREATE_ACL} for {self.type_info.name}') self.__acl__ = ALLOW_SUBMISSION_CENTER_CREATE_ACL else: self.__acl__ = ONLY_ADMIN_VIEW_ACL