From 2eda2b06819cdc93c842739c8715ad93a6c61aa6 Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Sun, 14 Mar 2021 12:32:16 -0400 Subject: [PATCH 01/13] unrelated: bring in 'make kibana-start-test', 'make psql-test', 'make restart', 'make test-performance', and 'make test-integration'. Change order of 'make test-npm' and 'make test-unit' within 'make test' so that unit comes first. --- Makefile | 27 ++++++++++++--- scripts/kibana-start | 80 ++++++++++++++++++++++++++++++++++++++++---- scripts/psql-start | 70 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+), 11 deletions(-) create mode 100755 scripts/psql-start diff --git a/Makefile b/Makefile index 09d4ea9a6e..9f15498f5a 100644 --- a/Makefile +++ b/Makefile @@ -95,11 +95,17 @@ deploy2: # spins up waittress to serve the application pserve development.ini psql-dev: # starts psql with the url after 'sqlalchemy.url =' in development.ini - @psql `grep 'sqlalchemy[.]url =' development.ini | sed -E 's/^.* = (.*)/\1/'` + @scripts/psql-start dev -kibana-start: +psql-test: # starts psql with a url constructed from data in 'ps aux'. + @scripts/psql-start test + +kibana-start: # starts a dev version of kibana (default port) scripts/kibana-start +kibana-start-test: # starts a test version of kibana (port chosen for active tests) + scripts/kibana-start test + kibana-stop: scripts/kibana-stop @@ -114,19 +120,27 @@ clean-python: pip freeze | xargs pip uninstall -y test: - make test-unit make test-npm + make test-unit + +retest: + bin/test -vv --last-failed test-any: bin/test -vv --timeout=200 - test-npm: bin/test -vv --timeout=200 -m "working and not manual and not integratedx and not performance and not broken and not sloppy and workbook" test-unit: bin/test -vv --timeout=200 -m "working and not manual and not integratedx and not performance and not broken and not sloppy and not workbook" +test-performance: + bin/test -vv --timeout=200 -m "working and not manual and not integratedx and performance and not broken and not sloppy" + +test-integrated: + bin/test -vv --timeout=200 -m "working and not manual and (integrated or integratedx) and not performance and not broken and not sloppy" + travis-test: # Actually, we don't normally use this. Instead the GA workflow sets up two parallel tests. make travis-test-npm make travis-test-unit @@ -154,11 +168,14 @@ info: $(info - Use 'make configure' to install poetry. You should not have to do this directly.) $(info - Use 'make deploy1' to spin up postgres/elasticsearch and load inserts.) $(info - Use 'make deploy2' to spin up the application server.) - $(info - Use 'make kibana-start' to start kibana, and 'make kibana-stop' to stop it.) + $(info - Use 'make kibana-start' to start kibana on the default local ES port, and 'make kibana-stop' to stop it.) + $(info - Use 'make kibana-start-test' to start kibana on the port being used for active testing, and 'make kibana-stop' to stop it.) $(info - Use 'make kill' to kill postgres and elasticsearch proccesses. Please use with care.) $(info - Use 'make moto-setup' to install moto, for less flaky tests. Implied by 'make build'.) $(info - Use 'make npm-setup' to build the front-end. Implied by 'make build'.) $(info - Use 'make psql-dev' to start psql on data associated with an active 'make deploy1'.) + $(info - Use 'make psql-test' to start psql on data associated with an active test.) + $(info - Use 'make retest' to run failing tests from the previous test run.) $(info - Use 'make test' to run tests with normal options similar to what we use on GitHub Actions.) $(info - Use 'make test-any' to run tests without marker constraints (i.e., with no '-m' option).) $(info - Use 'make update' to update dependencies (and the lock file).) diff --git a/scripts/kibana-start b/scripts/kibana-start index 7f08948d19..3b311bf451 100755 --- a/scripts/kibana-start +++ b/scripts/kibana-start @@ -1,5 +1,60 @@ #!/bin/bash +docker_kibana_image=docker.elastic.co/kibana/kibana-oss:6.8.9 +docker_kibana_port=5601 +local_kibana_port=${docker_kibana_port} +default_es_port=9200 +local_es_port=${default_es_port} + +if [ "$1" = "test" ]; then + + # This deletes all the output that doesn't match our pattern and includes only what does. + # Ref: https://stackoverflow.com/questions/6011661/regexp-sed-suppress-no-match-output + port=`ps aux | sed -E '/.*elasticsearch.*-Ehttp[.]port=([0-9]+)[^0-9].*/!d;s//\1/'` + + if [ -z "${port}" ]; then + echo "Cannot find test port." + exit 1 + elif [ `echo "${port}" | wc -l` -gt 1 ]; then + echo "Found multiple test ports:" + echo "${port}" + exit 1 + fi + + if [[ "${port}" =~ ^[0-9]+$ ]]; then + local_es_port="${port}" + echo "Using local_es_port=${local_es_port}." + else + echo "Port format is wrong: ${port}" + exit 1 + fi + +elif [[ "$1" =~ ^[0-9]+$ ]]; then + + local_es_port=$1 + echo "Using local_es_port=${local_es_port}." + +elif [ $# -gt 0 -a "$1" != "dev" ]; then + + echo "Syntax: $0 [ | test | dev ]" + echo " Starts kibana. By default, or if 'dev' is given, uses the standard port ${default_es_port}." + echo " If es-port is an integer, that port is used." + echo " If es-port is the word 'test', a test port is found using 'ps aux'." + echo " Note that this will choose its own http port, which will be 5601 for es-port=9200," + echo " or else a generated port number 10000+( % 55536) otherwise." + exit 1 + +fi + +if [ "${local_es_port}" != "${default_es_port}" ]; then + # 0-1023 are reserved ports + # 1024-65535 are available for custom use, but usualy 1024-9999 are assigned manually + # we'll generate a port in the range 10000-65535. that might sometimes collide, but probably VERY rarely. + local_kibana_port=$(( $local_es_port % 55536 + 10000 )) +fi + +echo "Using local_kibana_port=${local_kibana_port}" + docker --version if [ $? -ne 0 ]; then @@ -11,25 +66,38 @@ fi existing_network=`docker network ls | grep localnet` if [ -z "${existing_network}" ]; then + echo "creating localnet --driver=bridge" docker network create localnet --driver=bridge else echo "docker localnet is already set up. From 'docker network':" - echo " ${existing_network}" + # echo " ${existing_network}" fi -existing_kibana=`docker ps | egrep 'kibana:[0-9]*.[0-9]+.*[ ].*'` +# This pattern is structured so that if we need to extract the container id (e.g., to kill it), it will be in \1. +# But I figured out a way not to have to kill the old one, so that part of the pattern isn't used any more. +# I retained the first group just for possible future use. At this point, this just detects whether we have a +# handler for $local_kibana_port at all. This pattern also anticipates kibana images named 'kibana' or 'kibana-oss'. +# I other image names come up, for example if aws creates its own naming convention, it will need adjusting. +# -kmp 31-Jan-2021 +kibana_docker_pattern="([0-9a-f]+) .*kibana(-oss)?:[0-9]+[.][0-9]+[.].*0[.]0[.]0[.]0[:]${local_kibana_port}-[>]" + +existing_kibana=`docker ps | egrep "${kibana_docker_pattern}"` if [ -z "${existing_kibana}" ]; then - docker run -d --network localnet -p 5601:5601 -e ELASTICSEARCH_URL=http://host.docker.internal:9200 kibana:5.6.16 + echo "Kibana is not already running for ES port ${local_es_port}" + docker_es_url="http://host.docker.internal:${local_es_port}" + docker run -d --network localnet -p ${local_kibana_port}:${docker_kibana_port} -e ELASTICSEARCH_URL=${docker_es_url} ${docker_kibana_image} + + echo "Waiting for kibana to start..." + sleep 5 else - echo "Kibana is already running. From 'docker ps':" + echo "Kibana is already listening on port ${local_kibana_port} for elasticsearch on port ${local_es_port}:" echo " ${existing_kibana}" fi -local_kibana_url="http://localhost:5601/app/kibana#/dev_tools/console?_g=()" +local_kibana_url="http://localhost:${local_kibana_port}/app/kibana#/dev_tools/console?_g=()" echo "Opening kibana in browser at '${local_kibana_url}'..." open "${local_kibana_url}" & - diff --git a/scripts/psql-start b/scripts/psql-start new file mode 100755 index 0000000000..8957093bf8 --- /dev/null +++ b/scripts/psql-start @@ -0,0 +1,70 @@ +#!/bin/bash + +port=$1 + +dev_url_line=`grep 'sqlalchemy[.]url =' development.ini` + +dev_url=`echo "${dev_url_line}" | sed -E 's/^.* = (.*)$/\1/'` +dev_port=`echo "${dev_url_line}" | sed -E 's|^.* = .*:([0-9]+)/postgres[?].*$|\1|'` + + +# echo "dev_url=${dev_url}" +# echo "dev_port=${dev_port}" + + +# There seem be two processes, one for postgres and one for postgres-engine. +# The relevant data can be obtained from either, but matching both +# the match for postgres[^-] excludes the matches on postgres-engine so we +# can assume the match is unique. + +if [ "$port" = 'test' ]; then + + test_process=`ps aux | grep '.*[p]ostgres -D.*/private[a-zA-Z0-9_/-]*/postgresql[^-]'` + + if [ -z "${test_process}" ]; then + + echo "No test process found." + exit 1 + + else + + test_url=`echo "$test_process" | sed -E 's|^.*postgres[ ]+-D[ ]+([/a-zA-Z0-9_-]+)[ ]+.*-p[ ]+([0-9]+)([^0-9].*)?$|postgresql://postgres@localhost:\2/postgres?host=\1|'` + psql "${test_url}" + + # psql `ps aux | grep '.*[p]ostgres -D.*/private[a-zA-Z0-9_/-]*/postgresql[^-]' | sed -E 's|^.*postgres[ ]+-D[ ]+([/a-zA-Z0-9_-]+)[ ]+.*-p[ ]+([0-9]+)([^0-9].*)?$|postgresql://postgres@localhost:\2/postgres?host=\1|'` + + fi + +elif [ "$port" = 'dev' -o "$port" = "$dev_port" ]; then + + dev_url=`grep 'sqlalchemy[.]url =' development.ini | sed -E 's/^.* = (.*)/\1/'` + psql "${dev_url}" + +elif [[ "${port}" =~ ^[0-9]+$ ]]; then + + port_process=`ps aux | grep ".*[p]ostgres -D.*/private[a-zA-Z0-9_/-]*/postgresql[^-].*-p[ ]+${port}.*"` + + if [ -z "${port_process}" ]; then + + echo "No postgres process found on port ${port}." + exit 1 + + else + + port_url=`echo "$test_process" | sed -E 's|^.*postgres[ ]+-D[ ]+([/a-zA-Z0-9_-]+)[ ]+.*-p[ ]+([0-9]+)([^0-9].*)?$|postgresql://postgres@localhost:\2/postgres?host=\1|'` + psql "${port_url}" + + # psql `ps aux | grep '.*[p]ostgres -D.*/private[a-zA-Z0-9_/-]*/postgresql[^-]' | sed -E 's|^.*postgres[ ]+-D[ ]+([/a-zA-Z0-9_-]+)[ ]+.*-p[ ]+([0-9]+)([^0-9].*)?$|postgresql://postgres@localhost:\2/postgres?host=\1|'` + + fi + +else + + echo "Syntax: $0 [ | test | dev ]" + echo "" + echo "Starts psql for debugging in a way that corresponds to the given port." + echo "The port can be an integer or one of the special tokens 'dev' or 'test'." + echo "If 'dev' is given, the port from development.ini (currently '${dev_port}') is used." + echo "If 'test' is given, the port will be found from data in 'ps aux'." + +fi From 6fa56b5ac8cbe52bc05b57b632ece059dae8e6e9 Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Sun, 14 Mar 2021 12:40:24 -0400 Subject: [PATCH 02/13] Port renderers support support and associated tests from cgap. Port the rearranged fixture support from cgap-portal, which includes repairs to some of the html fixtures that specify they want text/html content. Move ORDER definition from datafixtures.py to conftest_settings.py. Adjust imports of ORDER. Mark test_fixtures as sloppy (causing this file not to run). Adjust test_indexing to use es_app/es_app_settings in places (as cgap does). --- src/encoded/renderers.py | 186 ++++++++++++++++------ src/encoded/tests/conftest.py | 193 ++++++++++++++++++++--- src/encoded/tests/conftest_settings.py | 29 ++++ src/encoded/tests/datafixtures.py | 32 +--- src/encoded/tests/test_create_mapping.py | 2 +- src/encoded/tests/test_embedding.py | 2 +- src/encoded/tests/test_fixtures.py | 3 +- src/encoded/tests/test_indexing.py | 51 +++--- src/encoded/tests/test_renderers.py | 128 +++++++++++++++ src/encoded/tests/test_views.py | 2 +- 10 files changed, 502 insertions(+), 126 deletions(-) create mode 100644 src/encoded/tests/test_renderers.py diff --git a/src/encoded/renderers.py b/src/encoded/renderers.py index cb71afa9bb..7cf1724cbf 100644 --- a/src/encoded/renderers.py +++ b/src/encoded/renderers.py @@ -4,35 +4,33 @@ import psutil import time -from pkg_resources import resource_filename -from urllib.parse import urlencode, urlparse +from dcicutils.misc_utils import environ_bool, PRINT, ignored from functools import lru_cache +from pkg_resources import resource_filename from pyramid.events import BeforeRender, subscriber from pyramid.httpexceptions import ( HTTPMovedPermanently, HTTPPreconditionFailed, HTTPUnauthorized, - # HTTPForbidden, HTTPUnsupportedMediaType, HTTPNotAcceptable, HTTPServerError ) -# from pyramid.security import forget +from pyramid.response import Response from pyramid.settings import asbool from pyramid.threadlocal import manager -from pyramid.response import Response from pyramid.traversal import split_path_info, _join_path_tuple -# from snovault.validation import CSRFTokenError -# from subprocess_middleware.tween import SubprocessTween from subprocess_middleware.worker import TransformWorker +from urllib.parse import urlencode, urlparse from webob.cookies import Cookie +from .util import content_type_allowed log = logging.getLogger(__name__) def includeme(config): - ''' + """ Can get tween ordering by executing the following on command-line from root dir: `bin/ptween development.ini` @@ -67,13 +65,15 @@ def includeme(config): This means that if handler(request) is called, then the downstream tweens are acted upon it, until response is returned. It's an ONION! - ''' + """ config.add_tween('.renderers.validate_request_tween_factory', under='snovault.stats.stats_tween_factory') - # DISABLED - .add_tween('.renderers.remove_expired_session_cookies_tween_factory', under='.renderers.validate_request_tween_factory') + # DISABLED - .add_tween('.renderers.remove_expired_session_cookies_tween_factory', + # under='.renderers.validate_request_tween_factory') config.add_tween('.renderers.render_page_html_tween_factory', under='.renderers.validate_request_tween_factory') - # The above tweens, when using response (= `handler(request)`) act on the _transformed_ response (containing HTML body). + # The above tweens, when using response (= `handler(request)`) act on the _transformed_ response + # (containing HTML body). # The below tweens run _before_ the JS rendering. Responses in these tweens have not been transformed to HTML yet. config.add_tween('.renderers.set_response_headers_tween_factory', under='.renderers.render_page_html_tween_factory') @@ -93,6 +93,7 @@ def validate_request_tween_factory(handler, registry): Apache config: SetEnvIf Request_Method HEAD X_REQUEST_METHOD=HEAD """ + ignored(registry) def validate_request_tween(request): @@ -107,19 +108,18 @@ def validate_request_tween(request): # Includes page text/html requests. return handler(request) - elif request.content_type != 'application/json': - if request.content_type == 'application/x-www-form-urlencoded' and request.path[0:10] == '/metadata/': - # Special case to allow us to POST to metadata TSV requests via form submission - return handler(request) + elif content_type_allowed(request): + return handler(request) + + else: detail = "Request content type %s is not 'application/json'" % request.content_type raise HTTPUnsupportedMediaType(detail) - return handler(request) - return validate_request_tween def security_tween_factory(handler, registry): + ignored(registry) def security_tween(request): """ @@ -132,7 +132,7 @@ def security_tween(request): """ expected_user = request.headers.get('X-If-Match-User') - if expected_user is not None: # Not sure when this is the case + if expected_user is not None: # Not sure when this is the case if request.authenticated_userid != 'mailto.' + expected_user: detail = 'X-If-Match-User does not match' raise HTTPPreconditionFailed(detail) @@ -147,15 +147,18 @@ def security_tween(request): raise HTTPUnauthorized( title="No Access", comment="Invalid Authorization header or Auth Challenge response.", - headers={'WWW-Authenticate': "Bearer realm=\"{}\"; Basic realm=\"{}\"".format(request.domain, request.domain) } + headers={ + 'WWW-Authenticate': ("Bearer realm=\"{}\"; Basic realm=\"{}\"" + .format(request.domain, request.domain)) + } ) - if hasattr(request, 'auth0_expired'): # Add some security-related headers on the up-swing response = handler(request) if request.auth0_expired: - #return response + # return response + # # If have the attribute and it is true, then our session has expired. # This is true for both AJAX requests (which have request.authorization) & browser page # requests (which have cookie); both cases handled in authentication.py @@ -176,7 +179,10 @@ def security_tween(request): path='/' ) # = Same as response.delete_cookie(..) response.status_code = 401 - response.headers['WWW-Authenticate'] = "Bearer realm=\"{}\", title=\"Session Expired\"; Basic realm=\"{}\"".format(request.domain, request.domain) + response.headers['WWW-Authenticate'] = ( + "Bearer realm=\"{}\", title=\"Session Expired\"; Basic realm=\"{}\"" + .format(request.domain, request.domain) + ) else: # We have JWT and it's not expired. Add 'X-Request-JWT' & 'X-User-Info' header. # For performance, only do it if should transform to HTML as is not needed on every request. @@ -188,9 +194,10 @@ def security_tween(request): # This header is parsed in renderer.js, or, more accurately, # by libs/react-middleware.js which is imported by server.js and compiled into # renderer.js. Is used to get access to User Info on initial web page render. - response.headers['X-Request-JWT'] = request.cookies.get('jwtToken','') - user_info = request.user_info.copy() # Re-ified property set in authentication.py - del user_info["id_token"] # Redundant - don't need this in SSR nor browser as get from X-Request-JWT. + response.headers['X-Request-JWT'] = request.cookies.get('jwtToken', '') + user_info = request.user_info.copy() # Re-ified property set in authentication.py + # Redundant - don't need this in SSR nor browser as get from X-Request-JWT. + del user_info["id_token"] response.headers['X-User-Info'] = json.dumps(user_info) else: response.headers['X-Request-JWT'] = "null" @@ -203,20 +210,22 @@ def security_tween(request): # requests from Authorization header which acts like a CSRF token. # See authentication.py - get_jwt() - #token = request.headers.get('X-CSRF-Token') - #if token is not None: - # # Avoid dirtying the session and adding a Set-Cookie header - # # XXX Should consider if this is a good idea or not and timeouts - # if token == dict.get(request.session, '_csrft_', None): - # return handler(request) - # raise CSRFTokenError('Incorrect CSRF token') + # Alex notes that we do not use request.session so this is probably very old. -kmp 4-Mar-2021 + + # token = request.headers.get('X-CSRF-Token') + # if token is not None: + # # Avoid dirtying the session and adding a Set-Cookie header + # # XXX Should consider if this is a good idea or not and timeouts + # if token == dict.get(request.session, '_csrft_', None): + # return handler(request) + # raise CSRFTokenError('Incorrect CSRF token') # raise CSRFTokenError('Missing CSRF token') return security_tween def remove_expired_session_cookies_tween_factory(handler, registry): - ''' + """ CURRENTLY DISABLED Original purpose of this was to remove expired (session?) cookies. See: https://github.com/ENCODE-DCC/encoded/commit/75854803c99e5044a6a33aedb3a79d750481b6cd#diff-bc19a9793a1b3b4870cff50e7c7c9bd1R135 @@ -224,7 +233,8 @@ def remove_expired_session_cookies_tween_factory(handler, registry): We disable it for now via removing from tween chain as are using JWT tokens and handling their removal in security_tween_factory & authentication.py as well as client-side (upon "Logout" action). If needed for some reason, can re-enable. - ''' + """ # noQA - not going to break the long URL line above + ignored(registry) ignore = { '/favicon.ico', @@ -235,8 +245,8 @@ def remove_expired_session_cookies_tween(request): return handler(request) session = request.session - #if session or session._cookie_name not in request.cookies: - # return handler(request) + # if session or session._cookie_name not in request.cookies: + # return handler(request) response = handler(request) # Below seems to be empty always; though we do have some in request.cookies @@ -260,7 +270,9 @@ def remove_expired_session_cookies_tween(request): def set_response_headers_tween_factory(handler, registry): - '''Add additional response headers here''' + """Add additional response headers here""" + ignored(registry) + def set_response_headers_tween(request): response = handler(request) response.headers['X-Request-URL'] = request.url @@ -316,15 +328,80 @@ def canonical_redirect(event): raise HTTPMovedPermanently(location=location, detail="Redirected from " + str(request.path_info)) +# Web browsers send an Accept request header for initial (e.g. non-AJAX) page requests +# which should contain 'text/html' +MIME_TYPE_HTML = 'text/html' +MIME_TYPE_JSON = 'application/json' +MIME_TYPE_LD_JSON = 'application/ld+json' + +MIME_TYPES_SUPPORTED = [MIME_TYPE_JSON, MIME_TYPE_HTML, MIME_TYPE_LD_JSON] +MIME_TYPE_DEFAULT = MIME_TYPES_SUPPORTED[0] +MIME_TYPE_TRIAGE_MODE = 'modern' # if this doesn't work, fall back to 'legacy' + +DEBUG_MIME_TYPES = environ_bool("DEBUG_MIME_TYPES", default=False) + + +def best_mime_type(request, mode=MIME_TYPE_TRIAGE_MODE): + # TODO: I think this function does nothing but return MIME_TYPES_SUPPORTED[0] -kmp 3-Feb-2021 + """ + Given a request, tries to figure out the best kind of MIME type to use in response + based on what kinds of responses we support and what was requested. + + In the case we can't comply, we just use application/json whether or not that's what was asked for. + """ + if mode == 'legacy': + # See: + # https://tedboy.github.io/flask/generated/generated/werkzeug.Accept.best_match.html#werkzeug-accept-best-match + # Note that this is now deprecated, or will be. The message is oddly worded ("will be deprecated") + # that presumably means "will be removed". Deprecation IS the warning of actual action, not the action itself. + # "This is currently maintained for backward compatibility, and will be deprecated in the future. + # AcceptValidHeader.best_match() uses its own algorithm (one not specified in RFC 7231) to determine + # what is a best match. The algorithm has many issues, and does not conform to RFC 7231." + # Anyway, we were getting this warning during testing: + # DeprecationWarning: The behavior of AcceptValidHeader.best_match is currently + # being maintained for backward compatibility, but it will be deprecated in the future, + # as it does not conform to the RFC. + # TODO: Once the modern replacement is shown to work, we should remove this conditional branch. + result = request.accept.best_match(MIME_TYPES_SUPPORTED, MIME_TYPE_DEFAULT) + else: + options = request.accept.acceptable_offers(MIME_TYPES_SUPPORTED) + if not options: + # TODO: Probably we should return a 406 response by raising HTTPNotAcceptable if + # no acceptable types are available. (Certainly returning JSON in this case is + # not some kind of friendly help toa naive user with an old browser.) + # Ref: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status + result = MIME_TYPE_DEFAULT + else: + mime_type, score = options[0] + result = mime_type + if DEBUG_MIME_TYPES: + PRINT("Using mime type", result, "for", request.method, request.url) + for k, v in request.headers.items(): + PRINT("%s: %s" % (k, v)) + PRINT("----------") + return result + + @lru_cache(maxsize=16) def should_transform(request, response): - ''' + """ Determines whether to transform the response from JSON->HTML/JS depending on type of response - and what the request is looking for to be returned via e.g. request Accept, Authorization header. - In case of no Accept header, attempts to guess. - - Memoized via `lru_cache`. Cache size is set to be 16 (> 1) in case sub-requests fired off during handling. - ''' + and what the request is looking for to be returned via these criteria, which are tried in order + until one succeeds: + + * If the request method is other than GET or HEAD, returns False. + * If the response.content_type is other than 'application/json', returns False. + * If a 'frame=' query param is given and not 'page' (the default), returns False. + * If a 'format=json' query param is given explicitly, + * For 'format=html', returns True. + * For 'format=json', returns False. + This rule does not match if 'format=' is not given explicitly. + If 'format=' is given an explicit value of ther than 'html' or 'json', an HTTPNotAcceptable error will be raised. + * If the first element of MIME_TYPES_SUPPORTED[0] is 'text/html', returns True. + * Otherwise, in all remaining cases, returns False. + + NOTE: Memoized via `lru_cache`. Cache size is set to be 16 (> 1) in case sub-requests fired off during handling. + """ # We always return JSON in response to POST, PATCH, etc. if request.method not in ('GET', 'HEAD'): return False @@ -333,6 +410,13 @@ def should_transform(request, response): if response.content_type != 'application/json': return False + # This is weirdly redundant with the next clause starting at 'format =' below. -kmp 11-Mar-2021 + # If we have a 'frame' that is not None or page, force JSON, since our UI doesn't handle all various + # forms of the data, just embedded/page. + request_frame = request.params.get("frame", "page") + if request_frame != "page": + return False + # The `format` URI param allows us to override request's 'Accept' header. format = request.params.get('format') if format is not None: @@ -342,16 +426,18 @@ def should_transform(request, response): if format == 'html': return True else: - raise HTTPNotAcceptable("Improper format URI parameter", comment="The format URI parameter should be set to either html or json.") + raise HTTPNotAcceptable("Improper format URI parameter", + comment="The format URI parameter should be set to either html or json.") # Web browsers send an Accept request header for initial (e.g. non-AJAX) page requests # which should contain 'text/html' # See: https://tedboy.github.io/flask/generated/generated/werkzeug.Accept.best_match.html#werkzeug-accept-best-match - mime_type = request.accept.best_match(['text/html', 'application/json', 'application/ld+json'], 'application/json') - format = mime_type.split('/', 1)[1] # Will be 1 of 'html', 'json', 'json-ld' + mime_type = best_mime_type(request) + format = mime_type.split('/', 1)[1] # Will be 1 of 'html', 'json', 'json-ld' - # N.B. ld+json (JSON-LD) is likely more unique case and might be sent by search engines (?) which can parse JSON-LDs. - # At some point we could maybe have it to be same as making an `@@object` or `?frame=object` request (?) esp if fill + # N.B. ld+json (JSON-LD) is likely more unique case and might be sent by search engines (?) + # which can parse JSON-LDs. At some point we could maybe have it to be same as + # making an `@@object` or `?frame=object` request (?) esp if fill # out @context response w/ schema(s) (or link to schema) if format == 'html': @@ -378,7 +464,9 @@ class TransformErrorResponse(HTTPServerError): rss_limit = 256 * (1024 ** 2) # MB - reload_process = True if registry.settings.get('reload_templates', False) else lambda proc: psutil.Process(proc.pid).memory_info().rss > rss_limit + reload_process = (True + if registry.settings.get('reload_templates', False) + else lambda proc: psutil.Process(proc.pid).memory_info().rss > rss_limit) # TransformWorker inits and manages a subprocess # it re-uses the subprocess so interestingly data in JS global variables diff --git a/src/encoded/tests/conftest.py b/src/encoded/tests/conftest.py index 190682e395..11ad9e95ab 100644 --- a/src/encoded/tests/conftest.py +++ b/src/encoded/tests/conftest.py @@ -18,24 +18,28 @@ from snovault.elasticsearch import ELASTIC_SEARCH, create_mapping from snovault.util import generate_indexer_namespace_for_testing from .. import main +from ..loadxl import load_all from .conftest_settings import make_app_settings_dictionary -# Done in pytest.ini now. -# -# pytest_plugins = [ -# 'encoded.tests.datafixtures', -# 'snovault.tests.serverfixtures', -# ] +""" +README: + * This file contains application level fixtures and hooks in the server/data fixtures present in + other files. + * There are "app" based fixtures that rely only on postgres, and + "es_app" fixtures that use both postgres and ES (for search/ES related testing) +""" @pytest.fixture(autouse=True) def autouse_external_tx(external_tx): + notice_pytest_fixtures(external_tx) pass @pytest.fixture(scope='session') -def app_settings(request, wsgi_server_host_port, conn, DBSession): +def app_settings(request, wsgi_server_host_port, conn, DBSession): # noQA - We didn't choose the fixture name. + notice_pytest_fixtures(request, wsgi_server_host_port, conn, DBSession) settings = make_app_settings_dictionary() settings['auth0.audiences'] = 'http://%s:%s' % wsgi_server_host_port # add some here for file testing @@ -43,6 +47,27 @@ def app_settings(request, wsgi_server_host_port, conn, DBSession): return settings +INDEXER_NAMESPACE_FOR_TESTING = generate_indexer_namespace_for_testing('ff') + + +@pytest.fixture(scope='session') +def es_app_settings(wsgi_server_host_port, elasticsearch_server, postgresql_server, aws_auth): + settings = make_app_settings_dictionary() + settings['create_tables'] = True + settings['persona.audiences'] = 'http://%s:%s' % wsgi_server_host_port # 2-tuple such as: ('localhost', '5000') + settings['elasticsearch.server'] = elasticsearch_server + settings['sqlalchemy.url'] = postgresql_server + settings['collection_datastore'] = 'elasticsearch' + settings['item_datastore'] = 'elasticsearch' + settings['indexer'] = True + settings['indexer.namespace'] = INDEXER_NAMESPACE_FOR_TESTING + + # use aws auth to access elasticsearch + if aws_auth: + settings['elasticsearch.aws_auth'] = aws_auth + return settings + + def pytest_configure(): logging.basicConfig(format='%(message)s') logging.getLogger('sqlalchemy.engine').setLevel(logging.WARNING) @@ -63,6 +88,7 @@ def filter(self, record): @pytest.yield_fixture def threadlocals(request, dummy_request, registry): + notice_pytest_fixtures(request, dummy_request, registry) threadlocal_manager.push({'request': dummy_request, 'registry': registry}) yield dummy_request threadlocal_manager.pop() @@ -99,11 +125,22 @@ def dummy_request(root, registry, app): @pytest.fixture(scope='session') def app(app_settings): - """WSGI application level functional testing. - """ + """ WSGI application level functional testing. """ return main({}, **app_settings) +@pytest.fixture(scope='session') +def es_app(es_app_settings, **kwargs): + """ + App that uses both Postgres and ES - pass this as "app" argument to TestApp. + Pass all kwargs onto create_mapping + """ + app = main({}, **es_app_settings) + create_mapping.run(app, **kwargs) + + return app + + @pytest.fixture def registry(app): return app.registry @@ -124,27 +161,69 @@ def root(registry): return registry[ROOT] +# Available Fixtures +# ------------------ +# +# ################## +-----------------------------------------+----------------------------------------------------+ +# ################## | Basic Application | Application with ES + Postgres | +# ################## +-----------------------+-----------------+---------------------------+------------------------+ +# ################## | JSON content | HTML content | JSON content | HTML content | +# -------------------+-----------------------+-----------------+---------------------------+------------------------+ +# Anonymous User | anontestapp | anonhtmltestapp | anon_es_testapp | anon_html_es_testapp | +# -------------------+-----------------------+-----------------+---------------------------+------------------------+ +# System User | testapp | htmltestapp | es_testapp | html_es_testapp | +# -------------------+-----------------------+-----------------+---------------------------+------------------------+ +# Authenticated User | authenticated_testapp | ----- | authenticated_es_testapp | ----- | +# -------------------+-----------------------+-----------------+---------------------------+------------------------+ +# Submitter User | submitter_testapp | ----- | ----- | ----- | +# -------------------+-----------------------+-----------------+---------------------------+------------------------+ +# Indexer User | ----- | ----- | indexer_testapp | ----- | +# -------------------+-----------------------+-----------------+---------------------------+------------------------+ +# Embed User | embed_testapp | ----- | ----- | ----- | +# -------------------+-----------------------+-----------------+---------------------------+------------------------+ +# # TODO: Reconsider naming to have some underscores interspersed for better readability. # e.g., html_testapp rather than htmltestapp, and especially anon_html_test_app rather than anonhtmltestapp. # -kmp 03-Feb-2020 +@pytest.fixture +def anontestapp(app): + """TestApp for anonymous user (i.e., no user specified), accepting JSON data.""" + environ = { + 'HTTP_ACCEPT': 'application/json', + } + return webtest.TestApp(app, environ) + + @pytest.fixture def anonhtmltestapp(app): - return webtest.TestApp(app) + """TestApp for anonymous (not logged in) user, accepting text/html content.""" + environ = { + 'HTTP_ACCEPT': 'text/html' + } + test_app = webtest.TestApp(app, environ) + return test_app @pytest.fixture -def htmltestapp(app): - """TestApp for TEST user and no HTTP_ACCEPT limitation, so HTML content can be tested.""" - # TODO: Name may be misleading. If only for HTML testing, seems like it should be text/html. - # Or if it spans CSS and other things, maybe call it page_content_testapp? -kmp 03-Feb-2020 +def anon_es_testapp(es_app): + """ TestApp simulating a bare Request entering the application (with ES enabled) """ environ = { - 'REMOTE_USER': 'TEST', + 'HTTP_ACCEPT': 'application/json', } - return webtest.TestApp(app, environ) + return webtest.TestApp(es_app, environ) + + +@pytest.fixture +def anon_html_es_testapp(es_app): + """TestApp with ES + Postgres for anonymous (not logged in) user, accepting text/html content.""" + environ = { + 'HTTP_ACCEPT': 'text/html' + } + return webtest.TestApp(es_app, environ) -@pytest.fixture(scope="module") +@pytest.fixture(scope="session") def testapp(app): """TestApp for username TEST, accepting JSON data.""" environ = { @@ -155,12 +234,34 @@ def testapp(app): @pytest.fixture -def anontestapp(app): - """TestApp for anonymous user (i.e., no user specified), accepting JSON data.""" +def htmltestapp(app): + """TestApp for TEST user, accepting text/html content.""" + environ = { + 'HTTP_ACCEPT': 'text/html', + 'REMOTE_USER': 'TEST', + } + test_app = webtest.TestApp(app, environ) + return test_app + + +@pytest.fixture(scope='session') +def es_testapp(es_app): + """ TestApp with ES + Postgres. Must be imported where it is needed. """ environ = { 'HTTP_ACCEPT': 'application/json', + 'REMOTE_USER': 'TEST', } - return webtest.TestApp(app, environ) + return webtest.TestApp(es_app, environ) + + +@pytest.fixture +def html_es_testapp(es_app): + """TestApp with ES + Postgres for TEST user, accepting text/html content.""" + environ = { + 'HTTP_ACCEPT': 'text/html', + 'REMOTE_USER': 'TEST', + } + return webtest.TestApp(es_app, environ) @pytest.fixture @@ -173,6 +274,16 @@ def authenticated_testapp(app): return webtest.TestApp(app, environ) +@pytest.fixture +def authenticated_es_testapp(es_app): + """ TestApp for authenticated non-admin user with ES """ + environ = { + 'HTTP_ACCEPT': 'application/json', + 'REMOTE_USER': 'TEST_AUTHENTICATED', + } + return webtest.TestApp(es_app, environ) + + @pytest.fixture def submitter_testapp(app): """TestApp for a non-admin user (TEST_SUBMITTER), accepting JSON data.""" @@ -184,13 +295,14 @@ def submitter_testapp(app): @pytest.fixture -def indexer_testapp(app): - """TestApp for indexing (user INDEXER), accepting JSON data.""" +def indexer_testapp(es_app): + """ Indexer testapp, meant for manually triggering indexing runs by posting to /index. + Always uses the ES app (obviously, but not so obvious previously) """ environ = { 'HTTP_ACCEPT': 'application/json', 'REMOTE_USER': 'INDEXER', } - return webtest.TestApp(app, environ) + return webtest.TestApp(es_app, environ) @pytest.fixture @@ -209,3 +321,38 @@ def wsgi_app(wsgi_server): return webtest.TestApp(wsgi_server) +class WorkbookCache: + """ Caches whether or not we have already provisioned the workbook. """ + done = None + + @classmethod + def initialize_if_needed(cls, es_app): + if not cls.done: + cls.done = cls.make_fresh_workbook(es_app) + + @classmethod + def make_fresh_workbook(cls, es_app): + environ = { + 'HTTP_ACCEPT': 'application/json', + 'REMOTE_USER': 'TEST', + } + testapp = webtest.TestApp(es_app, environ) + + # Just load the workbook inserts + # Note that load_all returns None for success or an Exception on failure. + load_res = load_all(testapp, pkg_resources.resource_filename('encoded', 'tests/data/workbook-inserts/'), []) + + if isinstance(load_res, Exception): + raise load_res + elif load_res: + raise RuntimeError("load_all returned a true value that was not an exception.") + + testapp.post_json('/index', {}) + return True + + +@pytest.fixture(scope='session') +def workbook(es_app): + """ Loads a bunch of data (tests/data/workbook-inserts) into the system on first run + (session scope doesn't work). """ + WorkbookCache.initialize_if_needed(es_app) diff --git a/src/encoded/tests/conftest_settings.py b/src/encoded/tests/conftest_settings.py index 9b87c70673..0745bcbc64 100644 --- a/src/encoded/tests/conftest_settings.py +++ b/src/encoded/tests/conftest_settings.py @@ -37,3 +37,32 @@ def make_app_settings_dictionary(): return _app_settings.copy() + + +ORDER = [ + 'user', 'award', 'lab', 'static_section', 'higlass_view_config', 'page', + 'ontology', 'ontology_term', 'file_format', 'badge', 'organism', 'gene', + 'genomic_region', 'bio_feature', 'target', 'imaging_path', 'publication', + 'publication_tracking', 'document', 'image', 'vendor', 'construct', + 'modification', 'experiment_type', 'protocol', 'sop_map', 'biosample_cell_culture', + 'individual_human', 'individual_mouse', 'individual_fly', 'individual_primate', + 'individual_chicken', 'individual_zebrafish', 'biosource', 'antibody', 'enzyme', + 'treatment_rnai', 'treatment_agent', + 'biosample', 'quality_metric_fastqc', 'quality_metric_bamcheck', 'quality_metric_rnaseq', + 'quality_metric_bamqc', 'quality_metric_pairsqc', 'quality_metric_margi', + 'quality_metric_dedupqc_repliseq', 'quality_metric_chipseq', 'quality_metric_workflowrun', + 'quality_metric_atacseq', 'quality_metric_rnaseq_madqc', 'quality_metric_qclist', + 'microscope_setting_d1', 'microscope_setting_d2', + 'microscope_setting_a1', 'microscope_setting_a2', 'file_fastq', + 'file_processed', 'file_reference', 'file_calibration', 'file_microscopy', + 'file_set', 'file_set_calibration', 'file_set_microscope_qc', + 'file_vistrack', 'experiment_hi_c', 'experiment_capture_c', + 'experiment_repliseq', 'experiment_atacseq', 'experiment_chiapet', + 'experiment_damid', 'experiment_seq', 'experiment_tsaseq', + 'experiment_mic', 'experiment_set', 'experiment_set_replicate', + 'data_release_update', 'software', 'analysis_step', 'workflow', + 'workflow_mapping', 'workflow_run_sbg', 'workflow_run_awsem', + 'tracking_item', 'quality_metric_flag', + 'summary_statistic', 'summary_statistic_hi_c', 'workflow_run', + 'microscope_configuration' +] diff --git a/src/encoded/tests/datafixtures.py b/src/encoded/tests/datafixtures.py index 7ca2853b72..8c709aac70 100644 --- a/src/encoded/tests/datafixtures.py +++ b/src/encoded/tests/datafixtures.py @@ -3,34 +3,10 @@ from uuid import uuid4 - -ORDER = [ - 'user', 'award', 'lab', 'static_section', 'higlass_view_config', 'page', - 'ontology', 'ontology_term', 'file_format', 'badge', 'organism', 'gene', - 'genomic_region', 'bio_feature', 'target', 'imaging_path', 'publication', - 'publication_tracking', 'document', 'image', 'vendor', 'construct', - 'modification', 'experiment_type', 'protocol', 'sop_map', 'biosample_cell_culture', - 'individual_human', 'individual_mouse', 'individual_fly', 'individual_primate', - 'individual_chicken', 'individual_zebrafish', 'biosource', 'antibody', 'enzyme', - 'treatment_rnai', 'treatment_agent', - 'biosample', 'quality_metric_fastqc', 'quality_metric_bamcheck', 'quality_metric_rnaseq', - 'quality_metric_bamqc', 'quality_metric_pairsqc', 'quality_metric_margi', - 'quality_metric_dedupqc_repliseq', 'quality_metric_chipseq', 'quality_metric_workflowrun', - 'quality_metric_atacseq', 'quality_metric_rnaseq_madqc', 'quality_metric_qclist', - 'microscope_setting_d1', 'microscope_setting_d2', - 'microscope_setting_a1', 'microscope_setting_a2', 'file_fastq', - 'file_processed', 'file_reference', 'file_calibration', 'file_microscopy', - 'file_set', 'file_set_calibration', 'file_set_microscope_qc', - 'file_vistrack', 'experiment_hi_c', 'experiment_capture_c', - 'experiment_repliseq', 'experiment_atacseq', 'experiment_chiapet', - 'experiment_damid', 'experiment_seq', 'experiment_tsaseq', - 'experiment_mic', 'experiment_set', 'experiment_set_replicate', - 'data_release_update', 'software', 'analysis_step', 'workflow', - 'workflow_mapping', 'workflow_run_sbg', 'workflow_run_awsem', - 'tracking_item', 'quality_metric_flag', - 'summary_statistic', 'summary_statistic_hi_c', 'workflow_run', - 'microscope_configuration' -] +# If anyone else needs to import ORDER, it got moved, so get it from +# its new nome in .conftest_settings. -kmp 14-Mar-2021 +# +# from .conftest_settings import ORDER @pytest.fixture diff --git a/src/encoded/tests/test_create_mapping.py b/src/encoded/tests/test_create_mapping.py index aae1140355..3e7681b216 100644 --- a/src/encoded/tests/test_create_mapping.py +++ b/src/encoded/tests/test_create_mapping.py @@ -5,7 +5,7 @@ from snovault.elasticsearch.create_mapping import type_mapping from snovault.util import add_default_embeds from unittest.mock import patch, MagicMock -from .datafixtures import ORDER +from .conftest_settings import ORDER from ..commands import create_mapping_on_deploy from ..commands.create_mapping_on_deploy import ( ITEM_INDEX_ORDER, diff --git a/src/encoded/tests/test_embedding.py b/src/encoded/tests/test_embedding.py index 236f167484..b3e2bfd2a5 100644 --- a/src/encoded/tests/test_embedding.py +++ b/src/encoded/tests/test_embedding.py @@ -3,7 +3,7 @@ from snovault import TYPES from snovault.util import add_default_embeds, crawl_schemas_by_embeds from ..types.base import get_item_or_none -from .datafixtures import ORDER +from .conftest_settings import ORDER pytestmark = [pytest.mark.setone, pytest.mark.working] diff --git a/src/encoded/tests/test_fixtures.py b/src/encoded/tests/test_fixtures.py index 01dfada4a5..ebf0075973 100644 --- a/src/encoded/tests/test_fixtures.py +++ b/src/encoded/tests/test_fixtures.py @@ -6,7 +6,8 @@ from ..tests import datafixtures -pytestmark = [pytest.mark.setone, pytest.mark.working, pytest.mark.schema] +# in cgap these are marked broken -kmp 24-Feb-2021 +pytestmark = [pytest.mark.setone, pytest.mark.working, pytest.mark.schema, pytest.mark.indexing, pytest.mark.sloppy] @pytest.yield_fixture(scope='session') diff --git a/src/encoded/tests/test_indexing.py b/src/encoded/tests/test_indexing.py index 7e8c11c24f..ac2e568540 100644 --- a/src/encoded/tests/test_indexing.py +++ b/src/encoded/tests/test_indexing.py @@ -24,21 +24,23 @@ build_index_record, compare_against_existing_mapping ) -from snovault.elasticsearch.interfaces import INDEXER_QUEUE from snovault.elasticsearch.indexer_utils import get_namespaced_index +from snovault.elasticsearch.interfaces import INDEXER_QUEUE from sqlalchemy import MetaData, func from timeit import default_timer as timer from unittest import mock from zope.sqlalchemy import mark_changed -from .. import main +from .. import main, loadxl from ..util import delay_rerun from ..verifier import verify_item -from .workbook_fixtures import app_settings # why does this care?? does it? -kmp 12-Mar-2021 +# from .workbook_fixtures import app_settings from .test_permissions import wrangler, wrangler_testapp -notice_pytest_fixtures(app_settings, wrangler, wrangler_testapp) - +notice_pytest_fixtures( + # app_settings, + wrangler, wrangler_testapp +) pytestmark = [pytest.mark.working, pytest.mark.indexing, pytest.mark.flaky(rerun_filter=delay_rerun, max_runs=2)] @@ -59,17 +61,19 @@ def test_postgres_version(session): @pytest.yield_fixture(scope='session', params=[False]) -def app(app_settings, request): +def app(es_app_settings, request): # for now, don't run with mpindexer. Add `True` to params above to do so if request.param: - app_settings['mpindexer'] = True - app = main({}, **app_settings) + # 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'] = False + app = main({}, **es_app_settings) yield app - DBSession = app.registry[DBSESSION] + db_session = app.registry[DBSESSION] # Dispose connections so postgres can tear down. - DBSession.bind.pool.dispose() + db_session.bind.pool.dispose() @pytest.yield_fixture(autouse=True) @@ -128,6 +132,7 @@ def test_indexing_simple(app, testapp, indexer_testapp): count += 1 assert res.json['total'] >= 2 assert uuid in uuids + namespaced_indexing = get_namespaced_index(app, 'indexing') indexing_doc = es.get(index=namespaced_indexing, doc_type='indexing', id='latest_indexing') indexing_source = indexing_doc['_source'] @@ -154,7 +159,7 @@ def test_create_mapping_on_indexing(app, testapp, registry, elasticsearch): item_types = TEST_COLLECTIONS # check that mappings and settings are in index for item_type in item_types: - item_mapping = type_mapping(registry[TYPES], item_type) + type_mapping(registry[TYPES], item_type) try: namespaced_index = get_namespaced_index(app, item_type) item_index = es.indices.get(index=namespaced_index) @@ -184,7 +189,7 @@ def test_file_processed_detailed(app, testapp, indexer_testapp, award, lab, file } fp_res = testapp.post_json('/file_processed', item) test_fp_uuid = fp_res.json['@graph'][0]['uuid'] - res = testapp.post_json('/file_processed', item) + testapp.post_json('/file_processed', item) indexer_testapp.post_json('/index', {'record': True}) # Todo, input a list of accessions / uuids: @@ -285,10 +290,11 @@ def test_real_validation_error(app, indexer_testapp, testapp, lab, award, file_f assert val_err_view['validation_errors'] == es_res['_source']['validation_errors'] -# @pytest.mark.performance +# TODO: This might need to use es_testapp now. -kmp 14-Mar-2021 +@pytest.mark.performance @pytest.mark.skip(reason="need to update perf-testing inserts") def test_load_and_index_perf_data(testapp, indexer_testapp): - ''' + """ ~~ CURRENTLY NOT WORKING ~~ PERFORMANCE TESTING @@ -299,7 +305,7 @@ def test_load_and_index_perf_data(testapp, indexer_testapp): nightly through the mastertest_deployment process in the torb repo it takes roughly 25 to run. Note: run with bin/test -s -m performance to see the prints from the test - ''' + """ insert_dir = pkg_resources.resource_filename('encoded', 'tests/data/perf-testing/') inserts = [f for f in os.listdir(insert_dir) if os.path.isfile(os.path.join(insert_dir, f))] @@ -317,7 +323,7 @@ def test_load_and_index_perf_data(testapp, indexer_testapp): # load -em up start = timer() - with mock.patch('encoded.loadxl.get_app') as mocked_app: + with mock.patch.object(loadxl, 'get_app') as mocked_app: mocked_app.return_value = testapp.app data = {'store': json_inserts} res = testapp.post_json('/load_data', data, # status=200 @@ -333,22 +339,23 @@ def test_load_and_index_perf_data(testapp, indexer_testapp): # check a couple random inserts for item in test_inserts: start = timer() - assert testapp.get("/" + item['data']['uuid'] + "?frame=raw").json['uuid'] + assert testapp.get("/" + item['data']['uuid'] + "?frame=raw").json['uuid'] # noQA stop = timer() frame_time = stop - start start = timer() - assert testapp.get("/" + item['data']['uuid']).follow().json['uuid'] + assert testapp.get("/" + item['data']['uuid']).follow().json['uuid'] # noQA stop = timer() embed_time = stop - start - print("PERFORMANCE: Time to query item %s - %s raw: %s embed %s" % (item['type_name'], item['data']['uuid'], + print("PERFORMANCE: Time to query item %s - %s raw: %s embed %s" % (item['type_name'], item['data']['uuid'], # noQA frame_time, embed_time)) # userful for seeing debug messages # assert False -def test_permissions_database_applies_permissions(award, lab, file_formats, wrangler_testapp, anontestapp, indexer_testapp): +def test_permissions_database_applies_permissions(award, lab, file_formats, wrangler_testapp, anon_es_testapp, + indexer_testapp): """ Tests that anontestapp gets view denied when using datastore=database """ file_item_body = { 'award': award['uuid'], @@ -361,7 +368,7 @@ def test_permissions_database_applies_permissions(award, lab, file_formats, wran item_id = res['@graph'][0]['@id'] indexer_testapp.post_json('/index', {'record': True}) time.sleep(1) # let es catch up - res = anontestapp.get('/' + item_id).json + res = anon_es_testapp.get('/' + item_id).json assert res['file_format'] == {'error': 'no view permissions'} - res = anontestapp.get('/' + item_id + '?datastore=database').json + res = anon_es_testapp.get('/' + item_id + '?datastore=database').json assert res['file_format'] == {'error': 'no view permissions'} diff --git a/src/encoded/tests/test_renderers.py b/src/encoded/tests/test_renderers.py new file mode 100644 index 0000000000..3ca09a7f5c --- /dev/null +++ b/src/encoded/tests/test_renderers.py @@ -0,0 +1,128 @@ +import pytest + +from dcicutils.misc_utils import filtered_warnings +from dcicutils.qa_utils import MockResponse +from unittest import mock +from pyramid.testing import DummyRequest +from .. import renderers +from ..renderers import ( + best_mime_type, should_transform, MIME_TYPES_SUPPORTED, MIME_TYPE_DEFAULT, + MIME_TYPE_JSON, MIME_TYPE_HTML, MIME_TYPE_LD_JSON, +) + + +pytestmark = [pytest.mark.setone, pytest.mark.working] + + +DEFAULT_SHOULD_TRANSFORM = (MIME_TYPE_DEFAULT == MIME_TYPE_HTML) + + +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' + assert MIME_TYPES_SUPPORTED == [MIME_TYPE_JSON, MIME_TYPE_HTML, MIME_TYPE_LD_JSON] + assert MIME_TYPE_DEFAULT == MIME_TYPE_JSON + + +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 + + +def test_best_mime_type_traditional(): + + test_best_mime_type('application/json') + + +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(): + + 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) + + print("method=", method, + "format=", _format, + "params=", params, + "requested=", requested_mime_type, + "response_content_type=", response_content_type, + "frame=", frame, + ) + + if req.method not in ('GET', 'HEAD'): + assert not should_transform(req, resp) + elif resp.content_type != 'application/json': + # If the response MIME type is not application/json, + # it just can't be transformed at all. + assert not should_transform(req, resp) + elif params.get("frame", "page") != 'page': + assert not should_transform(req, resp) + elif _format is not None: + assert should_transform(req, resp) is (_format == 'html') + else: + assert should_transform(req, resp) is DEFAULT_SHOULD_TRANSFORM + + +def test_should_transform_without_best_mime_type(): + + 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_views.py b/src/encoded/tests/test_views.py index 776cd1cfbf..9f1299191f 100644 --- a/src/encoded/tests/test_views.py +++ b/src/encoded/tests/test_views.py @@ -8,7 +8,7 @@ from pyramid.compat import ascii_native_ from snovault import TYPES from urllib.parse import urlparse -from .datafixtures import ORDER +from .conftest_settings import ORDER pytestmark = [pytest.mark.setone, pytest.mark.working, pytest.mark.schema] From e06feb0edec2b1253656fe076fe838c549106cbe Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Sun, 14 Mar 2021 13:00:44 -0400 Subject: [PATCH 03/13] Repair test_fixtures.py to understand the new location of ORDER. --- src/encoded/tests/test_fixtures.py | 53 +++++++++++++++++------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/src/encoded/tests/test_fixtures.py b/src/encoded/tests/test_fixtures.py index ebf0075973..185ce2f2f1 100644 --- a/src/encoded/tests/test_fixtures.py +++ b/src/encoded/tests/test_fixtures.py @@ -3,11 +3,11 @@ from unittest import mock -from ..tests import datafixtures +from ..tests import conftest_settings # in cgap these are marked broken -kmp 24-Feb-2021 -pytestmark = [pytest.mark.setone, pytest.mark.working, pytest.mark.schema, pytest.mark.indexing, pytest.mark.sloppy] +pytestmark = [pytest.mark.setone, pytest.mark.working, pytest.mark.schema, pytest.mark.indexing] @pytest.yield_fixture(scope='session') @@ -101,16 +101,22 @@ def test_fixtures2(minitestdata2, testapp): def test_order_complete(app, conn): + order_source_module = conftest_settings # TODO: This could use a doc string or comment. -kent & eric 29-Jun-2020 - print("original datafixtures.ORDER =", datafixtures.ORDER) - print("original len(datafixtures.ORDER) =", len(datafixtures.ORDER)) - assert "access_key" not in datafixtures.ORDER - order_for_testing = datafixtures.ORDER + ["access_key"] - with mock.patch.object(datafixtures, "ORDER", order_for_testing): - print("mocked datafixtures.ORDER =", datafixtures.ORDER) - print("len(mocked datafixtures.ORDER) =", len(datafixtures.ORDER)) - assert "access_key" in datafixtures.ORDER - ORDER = datafixtures.ORDER + print("original order_source_module.ORDER =", order_source_module.ORDER) + print("original len(order_source_module.ORDER) =", len(order_source_module.ORDER)) + assert "access_key" not in order_source_module.ORDER + print("confirmed: 'access_key' is NOT in ORDER") + order_for_testing = order_source_module.ORDER + ["access_key"] + assert order_source_module.ORDER is not order_for_testing + assert len(order_for_testing) == len(order_source_module.ORDER) + 1 + with mock.patch.object(order_source_module, "ORDER", order_for_testing): + print("=" * 24, "binding ORDER to add 'access_key'", "=" * 24) + print("mocked order_source_module.ORDER =", order_source_module.ORDER) + print("len(mocked order_source_module.ORDER) =", len(order_source_module.ORDER)) + assert "access_key" in order_source_module.ORDER + print("confirmed: 'access_key' IS in ORDER") + patched_order = order_source_module.ORDER environ = { 'HTTP_ACCEPT': 'application/json', 'REMOTE_USER': 'TEST', @@ -118,22 +124,25 @@ def test_order_complete(app, conn): testapp = webtest.TestApp(app, environ) master_types = [] profiles = testapp.get('/profiles/?frame=raw').json + print("constructing master_types from /profiles/?frame=raw") for a_type in profiles: if profiles[a_type].get('id') and profiles[a_type]['isAbstract'] is False: schema_name = profiles[a_type]['id'].split('/')[-1][:-5] master_types.append(schema_name) - print(ORDER) - print(master_types) - print(len(ORDER)) - print(len(master_types)) + print("patched_order=", patched_order) + print("master_types=", master_types) + print("len(patched_order)=", len(patched_order)) + print("len(master_types)=", len(master_types)) - missing_types = [i for i in master_types if i not in ORDER] - extra_types = [i for i in ORDER if i not in master_types] - print(missing_types) - print(extra_types) + missing_types = [i for i in master_types if i not in patched_order] + extra_types = [i for i in patched_order if i not in master_types] + print("missing_types=", missing_types) + print("extra_types=", extra_types) assert missing_types == [] assert extra_types == [] - print("restored datafixtures.ORDER =", datafixtures.ORDER) - print("restored len(datafixtures.ORDER) =", len(datafixtures.ORDER)) - assert "access_key" not in datafixtures.ORDER + print("=" * 24, "exiting bound context for ORDER", "=" * 24) + print("restored order_source_module.ORDER =", order_source_module.ORDER) + print("restored len(order_source_module.ORDER) =", len(order_source_module.ORDER)) + assert "access_key" not in order_source_module.ORDER + print("confirmed: 'access_key' is NOT in ORDER") \ No newline at end of file From ae0b49d75874341a74848dd01b80db7d3a903bf8 Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Tue, 16 Mar 2021 02:52:43 -0400 Subject: [PATCH 04/13] Major version bump because of change to rendering defaults when Accept not given. --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 5bac8d2d7c..b14b3551d0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [tool.poetry] # Note: Various modules refer to this system as "encoded", not "fourfront". name = "encoded" -version = "2.5.2" +version = "3.0.0" description = "4DN-DCIC Fourfront" authors = ["4DN-DCIC Team "] license = "MIT" From 9f9e86a6e0dc35d8b0329291cd28e74cb4b7b6e7 Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Thu, 18 Mar 2021 04:26:17 -0400 Subject: [PATCH 05/13] Invert the order of unit and npm tests in Makefile. --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 9f15498f5a..51d35eca42 100644 --- a/Makefile +++ b/Makefile @@ -120,8 +120,8 @@ clean-python: pip freeze | xargs pip uninstall -y test: - make test-npm make test-unit + make test-npm retest: bin/test -vv --last-failed @@ -142,8 +142,8 @@ test-integrated: bin/test -vv --timeout=200 -m "working and not manual and (integrated or integratedx) and not performance and not broken and not sloppy" travis-test: # Actually, we don't normally use this. Instead the GA workflow sets up two parallel tests. - make travis-test-npm make travis-test-unit + make travis-test-npm travis-test-npm: # Note this only does the 'not indexing' tests bin/test -vv --force-flaky --max-runs=3 --timeout=400 -m "working and not manual and not integratedx and not performance and not broken and not sloppy and workbook" --aws-auth --durations=10 --cov src/encoded --es search-fourfront-testing-6-8-kncqa2za2r43563rkcmsvgn2fq.us-east-1.es.amazonaws.com:443 @@ -160,7 +160,7 @@ help: info: @: $(info Here are some 'make' options:) $(info - Use 'make aws-ip-ranges' to download latest ip range information. Invoked automatically when needed.) - $(info - Use 'make build' (or 'make macbuild' on OSX Catalina) to build only application dependencies.) + $(info - Use 'make build' (or 'make macbuild' on OSX Catalina) to build onlgggsgsgggy application dependencies.) $(info - Use 'make build-dev' (or 'make macbuild-dev' on OSX Catalina) to build all dependencies, even locust.) $(info - Use 'make build-locust' to install locust. Do not do this unless you know what you are doing.) $(info - Use 'make clean' to clear out (non-python) dependencies.) From 2668f1fb0f01204791e0a744ee515d95b7593565 Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Thu, 18 Mar 2021 04:29:31 -0400 Subject: [PATCH 06/13] Bump patch version instead of major version now that we're compatible. --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index b14b3551d0..cd0787ae16 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [tool.poetry] # Note: Various modules refer to this system as "encoded", not "fourfront". name = "encoded" -version = "3.0.0" +version = "2.5.7" description = "4DN-DCIC Fourfront" authors = ["4DN-DCIC Team "] license = "MIT" From 097fea28781791873166410b41ae75efe1bb4e78 Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Fri, 19 Mar 2021 10:10:14 -0400 Subject: [PATCH 07/13] Bump patch version after merge from master. --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index cd0787ae16..2caa525cad 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [tool.poetry] # Note: Various modules refer to this system as "encoded", not "fourfront". name = "encoded" -version = "2.5.7" +version = "2.5.8" description = "4DN-DCIC Fourfront" authors = ["4DN-DCIC Team "] license = "MIT" From 9cef96232df9e5da1c411ad5750e10f4a8b82aeb Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Mon, 22 Mar 2021 23:40:37 -0400 Subject: [PATCH 08/13] PEP8 in loadxl.py --- src/encoded/loadxl.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/encoded/loadxl.py b/src/encoded/loadxl.py index 0072a30bd5..b6510963d8 100644 --- a/src/encoded/loadxl.py +++ b/src/encoded/loadxl.py @@ -9,6 +9,7 @@ import webtest from base64 import b64encode +from dcicutils.misc_utils import ignored from PIL import Image from pkg_resources import resource_filename from pyramid.paster import get_app @@ -97,6 +98,7 @@ def load_data_view(context, request): 2) store in form of {'item_type': [items], 'item_type2': [items]} item_type should be same as insert file names i.e. file_fastq """ + ignored(context) # this is a bit wierd but want to reuse load_data functionality so I'm rolling with it config_uri = request.json.get('config_uri', 'production.ini') patch_only = request.json.get('patch_only', False) From 1ba81b0d248e94617fae1b137779db8470273548 Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Tue, 23 Mar 2021 00:13:53 -0400 Subject: [PATCH 09/13] Indent properly. --- .../data/workbook-inserts/static_section.json | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/encoded/tests/data/workbook-inserts/static_section.json b/src/encoded/tests/data/workbook-inserts/static_section.json index 835eb8d494..7afed1051f 100644 --- a/src/encoded/tests/data/workbook-inserts/static_section.json +++ b/src/encoded/tests/data/workbook-inserts/static_section.json @@ -1,7 +1,15 @@ -[{ - "name" : "search-info-header.Workflow", - "uuid" : "442c8aa0-dc6c-43d7-814a-854af460b001", - "section_type" : "Search Info Header", - "title" : "Workflow Information", - "body" : "
  • Workflows define steps and their inputs & outputs. They are structurally similar to Common Workflow Language (CWL) definitions.
  • Workflow Runs are unique runs of each Workflow and have specific files associated with them.
" -}] +[ + { + "name" : "search-info-header.Workflow", + "uuid" : "442c8aa0-dc6c-43d7-814a-854af460b001", + "section_type" : "Search Info Header", + "title" : "Workflow Information", + "body" : "
  • Workflows define steps and their inputs & outputs. They are structurally similar to Common Workflow Language (CWL) definitions.
  • Workflow Runs are unique runs of each Workflow and have specific files associated with them.
" + }, + { + "name": "help.user-guide.rest-api.rest_api_submission", + "uuid": "442c8aa0-dc6c-43d7-814a-854af460b020", + "title": "", + "file": "/docs/source/rest_api_submission.rst" + } +] From c546fbe1905a909e896ae6c57b46795e3da76411 Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Tue, 23 Mar 2021 00:14:47 -0400 Subject: [PATCH 10/13] Use es_xxx fixtures. Get rid of workbook_fixtures.py --- src/encoded/tests/test_aggregation.py | 27 +- src/encoded/tests/test_batch_download.py | 16 +- src/encoded/tests/test_purge_item_type.py | 58 +-- src/encoded/tests/test_search.py | 382 +++++++++--------- src/encoded/tests/test_static_page.py | 243 +++++------ src/encoded/tests/test_validation_errors.py | 19 +- ...tures.py => workbook_fixtures.py.DISABLED} | 0 7 files changed, 347 insertions(+), 398 deletions(-) rename src/encoded/tests/{workbook_fixtures.py => workbook_fixtures.py.DISABLED} (100%) diff --git a/src/encoded/tests/test_aggregation.py b/src/encoded/tests/test_aggregation.py index c59ab7cc06..c4bf4ac93d 100644 --- a/src/encoded/tests/test_aggregation.py +++ b/src/encoded/tests/test_aggregation.py @@ -2,15 +2,6 @@ from dcicutils.qa_utils import notice_pytest_fixtures from ..util import delay_rerun -from .workbook_fixtures import app_settings, app, workbook - - -# NOTE WELL: Even though app_settings and app are not autouse fixtures, they must be imported. -# Removing these will not cause fixtures by those names not to be found, but my guess is that -# it will find different versions of those fixtures, which is what will cause the tests to fail -# with: 404 "The resource could not be found." -# -kmp 28-Jun-2020 -notice_pytest_fixtures(app_settings, app, workbook) pytestmark = [pytest.mark.working, @@ -19,10 +10,10 @@ pytest.mark.flaky(rerun_filter=delay_rerun)] -def test_aggregation_facet(workbook, testapp): - notice_pytest_fixtures(workbook, testapp) +def test_aggregation_facet(workbook, es_testapp): + notice_pytest_fixtures(workbook, es_testapp) - res = testapp.get('/search/?type=ExperimentSetReplicate').json + res = es_testapp.get('/search/?type=ExperimentSetReplicate').json badge_facets = [facet for facet in res['facets'] if facet['title'] in ['Commendations', 'Warnings']] assert badge_facets @@ -32,10 +23,10 @@ def test_aggregation_facet(workbook, testapp): assert len([t for t in terms if t != 'No value']) == 3 -def test_aggregation_itemview(workbook, testapp): - notice_pytest_fixtures(workbook, testapp) +def test_aggregation_itemview(workbook, es_testapp): + notice_pytest_fixtures(workbook, es_testapp) - res = testapp.get('/experiment-set-replicates/4DNESAAAAAA1/').json + res = es_testapp.get('/experiment-set-replicates/4DNESAAAAAA1/').json assert 'aggregated-items' in res.keys() parents = ''.join([badge['parent'] for badge in res['aggregated-items']['badges']]) assert 'biosample' in parents and 'experiment-set-replicate' in parents @@ -44,10 +35,10 @@ def test_aggregation_itemview(workbook, testapp): assert len(items) == len(list(set(items))) -def test_aggregation_view(workbook, testapp): - notice_pytest_fixtures(workbook, testapp) +def test_aggregation_view(workbook, es_testapp): + notice_pytest_fixtures(workbook, es_testapp) - res = testapp.get('/experiment-set-replicates/4DNESAAAAAA1/@@aggregated-items').json + res = es_testapp.get('/experiment-set-replicates/4DNESAAAAAA1/@@aggregated-items').json agg = res['aggregated_items'] assert 'badges' in agg.keys() assert len(agg['badges']) >= 3 diff --git a/src/encoded/tests/test_batch_download.py b/src/encoded/tests/test_batch_download.py index 082e5cc8f2..9f65d7652f 100644 --- a/src/encoded/tests/test_batch_download.py +++ b/src/encoded/tests/test_batch_download.py @@ -1,25 +1,15 @@ import pytest -from dcicutils.qa_utils import notice_pytest_fixtures from ..util import delay_rerun -# Use workbook fixture from BDD tests (including elasticsearch) -from .workbook_fixtures import app_settings, app, workbook -# NOTE WELL: app-settings and app are not used here explicitly but are probably still needed. -# See longer explanation at top of test_aggregation.py -kmp 28-Jun-2020 -notice_pytest_fixtures(app_settings, app, workbook) - -pytestmark = [# pytest.mark.indexing, - pytest.mark.workbook, - pytest.mark.flaky(rerun_filter=delay_rerun)] +pytestmark = [pytest.mark.workbook, pytest.mark.flaky(rerun_filter=delay_rerun)] # , pytest.mark.indexing @pytest.mark.skip(reason="update data when we have a working experiment") -def test_report_download(testapp, workbook): - notice_pytest_fixtures(testapp, workbook) +def test_report_download(es_testapp, workbook): - res = testapp.get('/report.tsv?type=Experiment&sort=accession') + res = es_testapp.get('/report.tsv?type=Experiment&sort=accession') assert res.headers['content-type'] == 'text/tsv; charset=UTF-8' disposition = res.headers['content-disposition'] assert disposition == 'attachment;filename="report.tsv"' diff --git a/src/encoded/tests/test_purge_item_type.py b/src/encoded/tests/test_purge_item_type.py index 08e6f7d2b5..a82c558315 100644 --- a/src/encoded/tests/test_purge_item_type.py +++ b/src/encoded/tests/test_purge_item_type.py @@ -1,18 +1,11 @@ import pytest import time -from dcicutils.qa_utils import notice_pytest_fixtures -from .workbook_fixtures import app_settings, app, workbook -from encoded.commands.purge_item_type import purge_item_type_from_storage - -notice_pytest_fixtures(app_settings, app, workbook) - - -pytestmark = [pytest.mark.working, pytest.mark.workbook] +from ..commands.purge_item_type import purge_item_type_from_storage @pytest.fixture -def dummy_static_section(testapp): +def dummy_static_section(es_testapp): static_section = { # from workbook_inserts "name": "search-info-header.Workflow_copy", "uuid": "442c8aa0-dc6c-43d7-814a-854af460b015", @@ -20,12 +13,12 @@ def dummy_static_section(testapp): "title": "Workflow Information", "body": "Some text to be rendered as a header" } - testapp.post_json('/static_section', static_section, status=201) - testapp.post_json('/index', {'record': True}) + es_testapp.post_json('/static_section', static_section, status=201) + es_testapp.post_json('/index', {'record': True}) @pytest.fixture -def many_dummy_static_sections(testapp): +def many_dummy_static_sections(es_testapp): static_section_template = { "name": "search-info-header.Workflow", "section_type": "Search Info Header", @@ -35,35 +28,42 @@ def many_dummy_static_sections(testapp): paths = [] for i in range(6): # arbitrarily defined static_section_template['name'] = 'search-info-header.Workflow:%s' % i - resp = testapp.post_json('/static_section', static_section_template, status=201).json + resp = es_testapp.post_json('/static_section', static_section_template, status=201).json paths.append(resp['@graph'][0]['@id']) - testapp.post_json('/index', {'record': True}) + es_testapp.post_json('/index', {'record': True}) return paths -@pytest.mark.parametrize('item_type', ['static_section']) # maybe should test some other types... -def test_purge_item_type_from_db(testapp, dummy_static_section, item_type): +@pytest.mark.parametrize('item_type', ['static_section']) # XXX: Maybe parametrize on a few types? +def test_purge_item_type_from_db(es_testapp, dummy_static_section, item_type): """ Tests purging all items of a certain item type from the DB """ - assert purge_item_type_from_storage(testapp, [item_type]) is True - testapp.post_json('/index', {'record': True}) - testapp.get('/search/?type=StaticSection', status=404) - testapp.get('/static-sections/442c8aa0-dc6c-43d7-814a-854af460b015?datastore=database', status=404) + # Next 3 lines experimental + # es_testapp.post_json('/index', {'record': True}) + # time.sleep(10) + # purge_item_type_from_storage(es_testapp, ['pages']) # get rid of these guys, could have links left over + assert purge_item_type_from_storage(es_testapp, [item_type]) is True + es_testapp.post_json('/index', {'record': True}) + es_testapp.get('/search/?type=StaticSection', status=404) + es_testapp.get('/static-sections/442c8aa0-dc6c-43d7-814a-854af460b015?datastore=database', status=404) -def test_purge_item_type_from_db_many(testapp, many_dummy_static_sections): +def test_purge_item_type_from_db_many(es_testapp, many_dummy_static_sections): """ Tests posting/deleting several static sections and checking all are gone """ paths_to_check = many_dummy_static_sections - assert purge_item_type_from_storage(testapp, ['static_section']) is True - testapp.post_json('/index', {'record': True}) + assert purge_item_type_from_storage(es_testapp, ['static_section']) is True + es_testapp.post_json('/index', {'record': True}) path_string = '%s?datastore=database' for path in paths_to_check: - testapp.get(path_string % path, status=404) - testapp.get('/search/?type=StaticSection', status=404) + es_testapp.get(path_string % path, status=404) + es_testapp.get('/search/?type=StaticSection', status=404) -def test_purge_item_type_with_links_fails(testapp, workbook): +# Just this one test is a workbook test, but note well that it does not modify the workbook. +# It should just try and fail, so that should be OK for the workbook. -kmp 22-Mar-2021 +@pytest.mark.workbook +def test_purge_item_type_with_links_fails(es_testapp, workbook): """ Tries to remove 'lab', which should fail since it has links """ - testapp.post_json('/index', {'record': True}) # must index everything so individual links show up + es_testapp.post_json('/index', {'record': True}) # must index everything so individual links show up time.sleep(5) # wait for indexing to catch up - assert not purge_item_type_from_storage(testapp, ['lab']) - testapp.post_json('/index', {'record': True}) + assert not purge_item_type_from_storage(es_testapp, ['lab']) + es_testapp.post_json('/index', {'record': True}) diff --git a/src/encoded/tests/test_search.py b/src/encoded/tests/test_search.py index 9c604e4c0f..a226450bf4 100644 --- a/src/encoded/tests/test_search.py +++ b/src/encoded/tests/test_search.py @@ -2,16 +2,12 @@ import pytest from dcicutils.misc_utils import Retry -from dcicutils.qa_utils import notice_pytest_fixtures from datetime import datetime, timedelta from snovault import TYPES, COLLECTIONS from snovault.elasticsearch import create_mapping from snovault.elasticsearch.indexer_utils import get_namespaced_index from snovault.util import add_default_embeds from ..commands.run_upgrader_on_inserts import get_inserts -# Use workbook fixture from BDD tests (including elasticsearch) -from .workbook_fixtures import app_settings, app, workbook -# from ..util import customized_delay_rerun pytestmark = [ @@ -19,14 +15,14 @@ pytest.mark.schema, # pytest.mark.indexing, pytest.mark.workbook, - #pytest.mark.flaky(rerun_filter=customized_delay_rerun(sleep_seconds=10)) + # pytest.mark.flaky(rerun_filter=customized_delay_rerun(sleep_seconds=10)) ] -### IMPORTANT +# ### IMPORTANT # uses the inserts in ./data/workbook_inserts # design your tests accordingly -notice_pytest_fixtures(app_settings, app, workbook) +# notice_pytest_fixtures(app_settings, app, workbook) # just a little helper function @@ -43,9 +39,9 @@ def recursively_find_uuids(json, uuids): return uuids -def test_search_view(workbook, testapp): - res = testapp.get('/search/?type=Item').json - assert res['@type'] == ['ItemSearchResults','Search'] +def test_search_view(workbook, es_testapp): + 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' @@ -56,10 +52,10 @@ def test_search_view(workbook, testapp): assert '@graph' in res -def test_search_with_no_query(workbook, testapp): +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 - res = testapp.get('/search/').follow(status=200) + 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/' @@ -75,10 +71,10 @@ def test_search_with_no_query(workbook, testapp): assert '@graph' in res -def test_collections_redirect_to_search(workbook, testapp): +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 = testapp.get('/biosamples/', status=301).follow(status=200) + res = es_testapp.get('/biosamples/', status=301).follow(status=200) assert res.json['@type'] == ['BiosampleSearchResults', 'ItemSearchResults', 'Search'] assert res.json['@id'] == '/search/?type=Biosample' assert 'redirected_from' not in res.json['@id'] @@ -91,8 +87,8 @@ def test_collections_redirect_to_search(workbook, testapp): assert '@graph' in res -def test_search_with_embedding(workbook, testapp): - res = testapp.get('/search/?type=Biosample&limit=all').json +def test_search_with_embedding(workbook, es_testapp): + res = es_testapp.get('/search/?type=Biosample&limit=all').json # Use a specific biosample, found by accession from test data # Check the embedding /types/biosample.py entry; test ensures # that the actual embedding matches that @@ -112,61 +108,61 @@ def test_search_with_embedding(workbook, testapp): assert test_json['lab'].get('awards') is None -def test_file_search_type(workbook, testapp): +def test_file_search_type(workbook, es_testapp): """ Tests that searching on a type that inherits from File adds a FileSearchResults identifier in the @type field """ - res = testapp.get('/search/?type=FileProcessed').json + res = es_testapp.get('/search/?type=FileProcessed').json assert 'FileSearchResults' in res['@type'] - res = testapp.get('/search/?type=Biosample').json + res = es_testapp.get('/search/?type=Biosample').json assert 'FileSearchResults' not in res['@type'] assert res['@type'][0] == 'BiosampleSearchResults' assert res['@type'][1] == 'ItemSearchResults' - res = testapp.get('/search/?type=FileProcessed&type=Biosample').json + res = es_testapp.get('/search/?type=FileProcessed&type=Biosample').json assert 'FileSearchResults' not in res['@type'] - res = testapp.get('/search/?type=FileProcessed&type=FileReference').json + res = es_testapp.get('/search/?type=FileProcessed&type=FileReference').json assert 'FileSearchResults' in res['@type'] - res = testapp.get('/search').follow().json + res = es_testapp.get('/search').follow().json assert 'FileSearchResults' not in res['@type'] - res = testapp.get('/search/?type=File').json + res = es_testapp.get('/search/?type=File').json assert 'FileSearchResults' in res['@type'] assert res['@type'].count('FileSearchResults') == 1 - res = testapp.get('/search/?type=FileFastq').json + res = es_testapp.get('/search/?type=FileFastq').json assert res['@type'][0] == 'FileFastqSearchResults' assert res['@type'][1] == 'FileSearchResults' assert res['@type'][2] == 'ItemSearchResults' assert res['@type'][3] == 'Search' - res = testapp.get('/search/?type=FileFastq&type=Biosample').json + res = es_testapp.get('/search/?type=FileFastq&type=Biosample').json assert res['@type'][0] == 'ItemSearchResults' - res = testapp.get('/search/?type=FileFastq&type=File').json + res = es_testapp.get('/search/?type=FileFastq&type=File').json assert res['@type'][0] == 'FileSearchResults' -def test_search_with_simple_query(workbook, testapp): +def test_search_with_simple_query(workbook, es_testapp): # run a simple query with type=Organism and q=mouse - res = testapp.get('/search/?type=Organism&q=mouse').json + res = es_testapp.get('/search/?type=Organism&q=mouse').json assert res['@type'] == ['OrganismSearchResults', 'ItemSearchResults', 'Search'] assert len(res['@graph']) > 0 # get the uuids from the results mouse_uuids = [org['uuid'] for org in res['@graph'] if 'uuid' in org] # run the same search with type=Item - res = testapp.get('/search/?type=Item&q=mouse').json + res = es_testapp.get('/search/?type=Item&q=mouse').json assert len(res['@graph']) > 0 all_uuids = [item['uuid'] for item in res['@graph'] if 'uuid' in item] # make sure all uuids found in the first search are present in the second assert set(mouse_uuids).issubset(set(all_uuids)) # run with q=mous returns the same hits... - res = testapp.get('/search/?type=Item&q=mous').json + res = es_testapp.get('/search/?type=Item&q=mous').json mous_uuids = [item['uuid'] for item in res['@graph'] if 'uuid' in item] # make sure all uuids found in the first search are present in the second assert set(mouse_uuids).issubset(set(mous_uuids)) # run with q=musculus, which should return the same hits as mouse - res = testapp.get('/search/?type=Item&q=musculus').json + res = es_testapp.get('/search/?type=Item&q=musculus').json musculus_uuids = [item['uuid'] for item in res['@graph'] if 'uuid' in item] # make sure all uuids found in the first search are present in the second assert set(mouse_uuids).issubset(set(musculus_uuids)) # run with q=mauze (misspelled) and ensure uuids are not in results - res = testapp.get('/search/?type=Item&q=mauxz', status=[200, 404]).json + res = es_testapp.get('/search/?type=Item&q=mauxz', status=[200, 404]).json # make this test robust by either assuming no results are found # (true when this test was written) # OR that results that happen to contain "mauze" do not include what @@ -176,33 +172,33 @@ def test_search_with_simple_query(workbook, testapp): assert not set(mouse_uuids).issubset(set(mauxz_uuids)) -def test_search_ngram(workbook, testapp): +def test_search_ngram(workbook, es_testapp): """ Tests ngram behavior for various use cases """ - res = testapp.get('/search/?type=Organism&q=mo').json + res = es_testapp.get('/search/?type=Organism&q=mo').json assert len(res['@graph']) == 1 - res = testapp.get('/search/?type=Organism&q=hu').json + res = es_testapp.get('/search/?type=Organism&q=hu').json assert len(res['@graph']) == 1 - res = testapp.get('/search/?type=Organism&q=ma').json + res = es_testapp.get('/search/?type=Organism&q=ma').json assert len(res['@graph']) == 1 # or search - res = testapp.get('/search/?type=Organism&q=(mu|an)').follow().json + res = es_testapp.get('/search/?type=Organism&q=(mu|an)').follow().json assert len(res['@graph']) == 2 # or not search - res = testapp.get('/search/?type=Organism&q=(ho|-an)').follow().json + res = es_testapp.get('/search/?type=Organism&q=(ho|-an)').follow().json assert len(res['@graph']) == 2 # by uuid subset - res = testapp.get('/search/?type=Organism&q=3413218c').json + res = es_testapp.get('/search/?type=Organism&q=3413218c').json assert len(res['@graph']) == 2 # uuid difference beyond max_ngram - res = testapp.get('/search/?type=Organism&q=3413218c-3f').json + res = es_testapp.get('/search/?type=Organism&q=3413218c-3f').json assert len(res['@graph']) == 2 # uuid difference before max_ngram, no results - res = testapp.get('/search/?type=Organism&q=3413218d', status=404) + res = es_testapp.get('/search/?type=Organism&q=3413218d', status=404) -def test_search_facets_and_columns_order(workbook, testapp, registry): +def test_search_facets_and_columns_order(workbook, es_testapp, registry): # TODO: Adjust ordering of mixed-in facets, perhaps sort by lookup or something, in order to un-xfail. test_type = 'experiment_set_replicate' type_info = registry[TYPES].by_item_type[test_type] @@ -214,30 +210,30 @@ def test_search_facets_and_columns_order(workbook, testapp, registry): # remove any disabled facets schema_facets = [fct for fct in schema_facets if not fct[1].get('disabled', False)] sort_facets = sorted(schema_facets, key=lambda fct: fct[1].get('order', 0)) - res = testapp.get('/search/?type=ExperimentSetReplicate&limit=all').json - for i,val in enumerate(sort_facets): + res = es_testapp.get('/search/?type=ExperimentSetReplicate&limit=all').json + for i, val in enumerate(sort_facets): assert res['facets'][i]['field'] == val[0] # assert order of columns when we officially upgrade to python 3.6 (ordered dicts) - for key,val in schema.get('columns', {}).items(): + for key, val in schema.get('columns', {}).items(): assert res['columns'][key]['title'] == val['title'] -def test_search_embedded_file_by_accession(workbook, testapp): - res = testapp.get('/search/?type=ExperimentHiC&files.accession=4DNFIO67APU1').json +def test_search_embedded_file_by_accession(workbook, es_testapp): + res = es_testapp.get('/search/?type=ExperimentHiC&files.accession=4DNFIO67APU1').json assert len(res['@graph']) > 0 item_uuids = [item['uuid'] for item in res['@graph'] if 'uuid' in item] for item_uuid in item_uuids: - item_res = testapp.get('/experiments-hi-c/%s/' % item_uuid, status=301) + item_res = es_testapp.get('/experiments-hi-c/%s/' % item_uuid, status=301) exp = item_res.follow().json file_uuids = [f['uuid'] for f in exp['files']] assert '46e82a90-49e5-4c33-afab-9ec90d65faa0' in file_uuids @pytest.fixture -def mboI_dts(testapp, workbook): +def mboI_dts(es_testapp, workbook): # returns a dictionary of strings of various date and datetimes # relative to the creation date of the mboI one object in test inserts - enz = testapp.get('/search/?type=Enzyme&name=MboI').json['@graph'][0] + enz = es_testapp.get('/search/?type=Enzyme&name=MboI').json['@graph'][0] cdate = enz['date_created'] _date, _time = cdate.split('T') @@ -257,9 +253,9 @@ def mboI_dts(testapp, workbook): } -def test_search_date_range_find_within(mboI_dts, testapp, workbook): +def test_search_date_range_find_within(mboI_dts, es_testapp, workbook): # the MboI enzyme should be returned with all the provided pairs - gres = testapp.get('/search/?type=Enzyme&name=MboI').json + gres = es_testapp.get('/search/?type=Enzyme&name=MboI').json g_uuids = [item['uuid'] for item in gres['@graph'] if 'uuid' in item] dts = {k: v.replace(':', '%3A') for k, v in mboI_dts.items()} datepairs = [ @@ -272,23 +268,23 @@ def test_search_date_range_find_within(mboI_dts, testapp, workbook): for dp in datepairs: search = '/search/?type=Enzyme&date_created.from=%s&date_created.to=%s' % dp - sres = testapp.get(search).json + sres = es_testapp.get(search).json s_uuids = [item['uuid'] for item in sres['@graph'] if 'uuid' in item] assert set(g_uuids).issubset(set(s_uuids)) -def test_search_with_nested_integer(testapp, workbook): +def test_search_with_nested_integer(es_testapp, workbook): search0 = '/search/?type=ExperimentHiC' - s0res = testapp.get(search0).json + s0res = es_testapp.get(search0).json s0_uuids = [item['uuid'] for item in s0res['@graph'] if 'uuid' in item] search1 = '/search/?type=ExperimentHiC&files.file_size.to=1500' - s1res = testapp.get(search1).json + s1res = es_testapp.get(search1).json s1_uuids = [item['uuid'] for item in s1res['@graph'] if 'uuid' in item] assert len(s1_uuids) > 0 search2 = '/search/?type=ExperimentHiC&files.file_size.from=1501' - s2res = testapp.get(search2).json + s2res = es_testapp.get(search2).json s2_uuids = [item['uuid'] for item in s2res['@graph'] if 'uuid' in item] assert len(s2_uuids) > 0 @@ -297,7 +293,7 @@ def test_search_with_nested_integer(testapp, workbook): assert set(s1_uuids) | set(s2_uuids) == set(s0_uuids) -def test_search_date_range_dontfind_without(mboI_dts, testapp, workbook): +def test_search_date_range_dontfind_without(mboI_dts, es_testapp, workbook): # the MboI enzyme should be returned with all the provided pairs dts = {k: v.replace(':', '%3A') for k, v in mboI_dts.items()} datepairs = [ @@ -307,30 +303,30 @@ def test_search_date_range_dontfind_without(mboI_dts, testapp, workbook): ] for dp in datepairs: search = '/search/?type=Enzyme&date_created.from=%s&date_created.to=%s' % dp - assert testapp.get(search, status=404) + assert es_testapp.get(search, status=404) -def test_search_query_string_AND_NOT_cancel_out(workbook, testapp): +def test_search_query_string_AND_NOT_cancel_out(workbook, es_testapp): # if you use + and - with same field you should get no result search = '/search/?q=cell+-cell&type=Biosource' - assert testapp.get(search, status=404) + assert es_testapp.get(search, status=404) -def test_search_multiple_types(workbook, testapp): +def test_search_multiple_types(workbook, es_testapp): # multiple types work with @type in response search = '/search/?type=Biosample&type=ExperimentHiC' - res = testapp.get(search).json + res = es_testapp.get(search).json assert res['@type'] == ['ItemSearchResults', 'Search'] -def test_search_query_string_with_booleans(workbook, testapp): +def test_search_query_string_with_booleans(workbook, es_testapp): """ moved references to res_not_induced and not_induced_uuids, which were passing locally but failing on travis for some undetermined reason... will look into this more later """ search = '/search/?type=Biosource&q=GM12878' - res_stem = testapp.get(search).json + res_stem = es_testapp.get(search).json assert len(res_stem['@graph']) > 1 bios_uuids = [r['uuid'] for r in res_stem['@graph'] if 'uuid' in r] swag_bios = '331111bc-8535-4448-903e-854af460b888' @@ -338,44 +334,47 @@ def test_search_query_string_with_booleans(workbook, testapp): # assert induced_stem_uuid not in not_induced_uuids # now search for stem +induced (AND is now "+") search_and = '/search/?type=Biosource&q=swag+%2BGM12878' - res_both = testapp.get(search_and).json + res_both = es_testapp.get(search_and).json both_uuids = [r['uuid'] for r in res_both['@graph'] if 'uuid' in r] assert len(both_uuids) == 1 assert swag_bios in both_uuids # search with OR ("|") search_or = '/search/?type=Biosource&q=swag+%7CGM12878' - res_or = testapp.get(search_or).json + res_or = es_testapp.get(search_or).json or_uuids = [r['uuid'] for r in res_or['@graph'] if 'uuid' in r] assert len(or_uuids) > 1 assert swag_bios in or_uuids # search with NOT ("-") search_not = '/search/?type=Biosource&q=GM12878+-swag' - res_not = testapp.get(search_not).json + res_not = es_testapp.get(search_not).json not_uuids = [r['uuid'] for r in res_not['@graph'] if 'uuid' in r] assert swag_bios not in not_uuids @pytest.mark.broken # test doesn't work, this will keep make from running it @pytest.mark.skip # In case of running the file by name, this still doesn't want to run -def test_metadata_tsv_view(workbook, htmltestapp): +def test_metadata_tsv_view(workbook, html_es_testapp): FILE_ACCESSION_COL_INDEX = 3 FILE_DOWNLOAD_URL_COL_INDEX = 0 - def check_tsv(result_rows, len_requested = None): + def check_tsv(result_rows, len_requested=None): info_row = result_rows.pop(0) header_row = result_rows.pop(0) assert header_row[FILE_ACCESSION_COL_INDEX] == 'File Accession' - assert header_row.index('File Download URL') == FILE_DOWNLOAD_URL_COL_INDEX # Ensure we have this column - assert len(result_rows) > 0 # We at least have some rows. + assert header_row.index('File Download URL') == FILE_DOWNLOAD_URL_COL_INDEX # Ensure we have this column + assert len(result_rows) > 0 # We at least have some rows. for row_index in range(1): - assert len(result_rows[row_index][FILE_ACCESSION_COL_INDEX]) > 4 # We have a value for File Accession - assert 'http' in result_rows[row_index][FILE_DOWNLOAD_URL_COL_INDEX] # Make sure it seems like a valid URL. + assert len(result_rows[row_index][FILE_ACCESSION_COL_INDEX]) > 4 # We have a value for File Accession + assert 'http' in result_rows[row_index][FILE_DOWNLOAD_URL_COL_INDEX] # Make sure it seems like a valid URL. assert '/@@download/' in result_rows[row_index][FILE_DOWNLOAD_URL_COL_INDEX] - assert result_rows[row_index][FILE_ACCESSION_COL_INDEX] in result_rows[row_index][FILE_DOWNLOAD_URL_COL_INDEX] # That File Accession is also in File Download URL of same row. - assert len(result_rows[row_index][FILE_ACCESSION_COL_INDEX]) < len(result_rows[row_index][FILE_DOWNLOAD_URL_COL_INDEX]) + # That File Accession is also in File Download URL of same row. + assert (result_rows[row_index][FILE_ACCESSION_COL_INDEX] + in result_rows[row_index][FILE_DOWNLOAD_URL_COL_INDEX]) + assert (len(result_rows[row_index][FILE_ACCESSION_COL_INDEX]) + < len(result_rows[row_index][FILE_DOWNLOAD_URL_COL_INDEX])) # Last some rows should be 'summary' rows. And have empty spaces for 'Download URL' / first column. summary_start_row = None @@ -394,44 +393,47 @@ def check_tsv(result_rows, len_requested = None): assert int(result_rows[summary_start_row + 4][4]) == summary_start_row assert int(result_rows[summary_start_row + 5][4]) <= summary_start_row - # run a simple GET query with type=ExperimentSetReplicate - res = htmltestapp.get('/metadata/type=ExperimentSetReplicate/metadata.tsv') # OLD URL FORMAT IS USED -- TESTING REDIRECT TO NEW URL - res = res.maybe_follow() # Follow redirect -- https://docs.pylonsproject.org/projects/webtest/en/latest/api.html#webtest.response.TestResponse.maybe_follow + res = html_es_testapp.get('/metadata/type=ExperimentSetReplicate/metadata.tsv') # OLD URL FORMAT IS USED -- TESTING REDIRECT TO NEW URL + res = res.maybe_follow() # Follow redirect -- https://docs.pylonsproject.org/projects/webtest/en/latest/api.html#webtest.response.TestResponse.maybe_follow assert 'text/tsv' in res.content_type - result_rows = [ row.rstrip(' \r').split('\t') for row in res.body.decode('utf-8').split('\n') ] # Strip out carriage returns and whatnot. Make a plain multi-dim array. + # Strip out carriage returns and whatnot. Make a plain multi-dim array. + result_rows = [row.rstrip(' \r').split('\t') + for row in res.body.decode('utf-8').split('\n')] check_tsv(result_rows) # Perform POST w/ accession triples (main case, for BrowseView downloads) - res2_post_data = { # N.B. '.post', not '.post_json' is used. This dict is converted to POST form values, with key values STRINGIFIED, not to POST JSON request. - "accession_triples" : [ - ["4DNESAAAAAA1","4DNEXO67APU1","4DNFIO67APU1"], - ["4DNESAAAAAA1","4DNEXO67APU1","4DNFIO67APT1"], - ["4DNESAAAAAA1","4DNEXO67APT1","4DNFIO67APV1"], - ["4DNESAAAAAA1","4DNEXO67APT1","4DNFIO67APY1"], - ["4DNESAAAAAA1","4DNEXO67APV1","4DNFIO67APZ1"], - ["4DNESAAAAAA1","4DNEXO67APV1","4DNFIO67AZZ1"] + res2_post_data = { # N.B. '.post', not '.post_json' is used. This dict is converted to POST form values, with key values STRINGIFIED, not to POST JSON request. + "accession_triples": [ + ["4DNESAAAAAA1", "4DNEXO67APU1", "4DNFIO67APU1"], + ["4DNESAAAAAA1", "4DNEXO67APU1", "4DNFIO67APT1"], + ["4DNESAAAAAA1", "4DNEXO67APT1", "4DNFIO67APV1"], + ["4DNESAAAAAA1", "4DNEXO67APT1", "4DNFIO67APY1"], + ["4DNESAAAAAA1", "4DNEXO67APV1", "4DNFIO67APZ1"], + ["4DNESAAAAAA1", "4DNEXO67APV1", "4DNFIO67AZZ1"] ], - 'download_file_name' : 'metadata_TEST.tsv' + 'download_file_name': 'metadata_TEST.tsv' } - res2 = htmltestapp.post('/metadata/?type=ExperimentSetReplicate', { k : json.dumps(v) for k,v in res2_post_data.items() }) # NEWER URL FORMAT + res2 = html_es_testapp.post('/metadata/?type=ExperimentSetReplicate', + {k: json.dumps(v) + for k, v in res2_post_data.items()}) # NEWER URL FORMAT assert 'text/tsv' in res2.content_type - result_rows = [ row.rstrip(' \r').split('\t') for row in res2.body.decode('utf-8').split('\n') ] + result_rows = [row.rstrip(' \r').split('\t') for row in res2.body.decode('utf-8').split('\n')] check_tsv(result_rows, len(res2_post_data['accession_triples'])) -def test_default_schema_and_non_schema_facets(workbook, testapp, registry): +def test_default_schema_and_non_schema_facets(workbook, es_testapp, registry): test_type = 'biosample' type_info = registry[TYPES].by_item_type[test_type] schema = type_info.schema embeds = add_default_embeds(test_type, registry[TYPES], type_info.embedded_list, schema) # we're looking for this specific facet, which is not in the schema assert 'biosource.biosource_type' in embeds - res = testapp.get('/search/?type=Biosample&biosource.biosource_type=immortalized+cell+line').json + res = es_testapp.get('/search/?type=Biosample&biosource.biosource_type=immortalized+cell+line').json assert 'facets' in res facet_fields = [ facet['field'] for facet in res['facets'] ] assert 'type' in facet_fields @@ -443,23 +445,23 @@ def test_default_schema_and_non_schema_facets(workbook, testapp, registry): assert 'biosource.biosource_type' in facet_fields -def test_search_query_string_no_longer_functional(workbook, testapp): +def test_search_query_string_no_longer_functional(workbook, es_testapp): # since we now use simple_query_string, cannot use field:value or range # expect 404s, since simple_query_string doesn't return exceptions search_field = '/search/?q=name%3Ahuman&type=Item' - res_field = testapp.get(search_field, status=404) + res_field = es_testapp.get(search_field, status=404) assert len(res_field.json['@graph']) == 0 search_range = '/search/?q=date_created%3A>2018-01-01&type=Item' - res_search = testapp.get(search_range, status=404) + res_search = es_testapp.get(search_range, status=404) assert len(res_search.json['@graph']) == 0 -def test_search_with_added_display_title(workbook, testapp, registry): +def test_search_with_added_display_title(workbook, es_testapp, registry): # 4DNBS1234567 is display_title for biosample search = '/search/?type=ExperimentHiC&biosample=4DNBS1234567' # 301 because search query is changed - res_json = testapp.get(search, status=301).follow(status=200).json + res_json = es_testapp.get(search, status=301).follow(status=200).json assert res_json['@id'] == '/search/?type=ExperimentHiC&biosample.display_title=4DNBS1234567' added_facet = [fct for fct in res_json['facets'] if fct['field'] == 'biosample.display_title'] # use the title from biosample in experiment schema @@ -468,25 +470,25 @@ def test_search_with_added_display_title(workbook, testapp, registry): exps = [exp['uuid'] for exp in res_json['@graph']] # make sure the search result is the same for the explicit query - res_json2 = testapp.get(res_json['@id']).json + res_json2 = es_testapp.get(res_json['@id']).json exps2 = [exp['uuid'] for exp in res_json2['@graph']] assert set(exps) == set(exps2) # 'sort' also adds display_title for ascending and descending queries for use_sort in ['biosample', '-biosample']: search = '/search/?type=ExperimentHiC&sort=%s' % use_sort - res_json = testapp.get(search, status=301).follow(status=200).json + res_json = es_testapp.get(search, status=301).follow(status=200).json assert res_json['@id'] == '/search/?type=ExperimentHiC&sort=%s.display_title' % use_sort # regular sort queries remain unchanged search = '/search/?type=ExperimentHiC&sort=uuid' - res_json = testapp.get(search).json + res_json = es_testapp.get(search).json assert res_json['@id'] == '/search/?type=ExperimentHiC&sort=uuid' # check to see that added facet doesn't conflict with existing facet title # query below will change to file_format.display_title=fastq search = '/search/?type=File&file_format=fastq' - res_json = testapp.get(search, status=301).follow(status=200).json + res_json = es_testapp.get(search, status=301).follow(status=200).json assert res_json['@id'] == '/search/?type=File&file_format.display_title=fastq' # find title from schema ff_title = registry[TYPES]['File'].schema['properties']['file_format']['title'] @@ -496,16 +498,16 @@ def test_search_with_added_display_title(workbook, testapp, registry): assert added_ff_facet[0]['title'] == ff_title + ' (Title)' -def test_search_with_no_value(workbook, testapp): +def test_search_with_no_value(workbook, es_testapp): search = '/search/?description=No+value&description=GM12878+prepared+for+HiC&type=Biosample' - res_json = testapp.get(search).json + res_json = es_testapp.get(search).json # grab some random results for item in res_json['@graph']: maybe_null = item.get('description') assert( maybe_null is None or maybe_null == 'GM12878 prepared for HiC') res_ids = [r['uuid'] for r in res_json['@graph'] if 'uuid' in r] search2 = '/search/?description=GM12878+prepared+for+HiC&type=Biosample' - res_json2 = testapp.get(search2).json + res_json2 = es_testapp.get(search2).json # just do 1 res here check_item = res_json2['@graph'][0] assert(check_item.get('description') == 'GM12878 prepared for HiC') @@ -513,15 +515,15 @@ def test_search_with_no_value(workbook, testapp): assert(set(res_ids2) <= set(res_ids)) -def test_search_with_static_header(workbook, testapp): +def test_search_with_static_header(workbook, es_testapp): """ Performs a search which should be accompanied by a search header """ search = '/search/?type=Workflow' - res_json = testapp.get(search, status=404).json # no items, just checking hdr + res_json = es_testapp.get(search, status=404).json # no items, just checking hdr assert 'search_header' in res_json assert 'content' in res_json['search_header'] assert res_json['search_header']['title'] == 'Workflow Information' - search = '/search/?type=workflow' # check type resolution - res_json = testapp.get(search, status=404).json + search = '/search/?type=workflow' # check type resolution + res_json = es_testapp.get(search, status=404).json assert 'search_header' in res_json assert 'content' in res_json['search_header'] assert res_json['search_header']['title'] == 'Workflow Information' @@ -531,17 +533,17 @@ def test_search_with_static_header(workbook, testapp): ## Tests for collections (search 301s) ## ######################################### -def test_collection_limit(workbook, testapp): - res = testapp.get('/biosamples/?limit=2', status=301) +def test_collection_limit(workbook, es_testapp): + res = es_testapp.get('/biosamples/?limit=2', status=301) assert len(res.follow().json['@graph']) == 2 -def test_collection_actions_filtered_by_permission(workbook, testapp, anontestapp): - res = testapp.get('/biosamples/') +def test_collection_actions_filtered_by_permission(workbook, es_testapp, anon_es_testapp): + res = es_testapp.get('/biosamples/') assert any(action for action in res.follow().json.get('actions', []) if action['name'] == 'add') # biosamples not visible - res = anontestapp.get('/biosamples/', status=404) + res = anon_es_testapp.get('/biosamples/', status=404) assert len(res.json['@graph']) == 0 @@ -552,12 +554,12 @@ def check_item_type(client, item_type): @pytest.mark.flaky -def test_index_data_workbook(app, workbook, testapp, indexer_testapp, htmltestapp): - es = app.registry['elasticsearch'] +def test_index_data_workbook(es_app, workbook, es_testapp, indexer_testapp, html_es_testapp): + es = es_app.registry['elasticsearch'] # we need to reindex the collections to make sure numbers are correct - create_mapping.run(app, sync_index=True) + create_mapping.run(es_app, sync_index=True) # check counts and ensure they're equal - testapp_counts = testapp.get('/counts') + testapp_counts = es_testapp.get('/counts') split_counts = testapp_counts.json['db_es_total'].split() assert(int(split_counts[1]) == int(split_counts[3])) # 2nd is db, 4th is es for item_name, item_counts in testapp_counts.json['db_es_compare'].items(): @@ -569,8 +571,8 @@ def test_index_data_workbook(app, workbook, testapp, indexer_testapp, htmltestap # check ES counts directly. Must skip abstract collections # must change counts result ("ItemName") to item_type format - item_type = app.registry[COLLECTIONS][item_name].type_info.item_type - namespaced_index = get_namespaced_index(app, item_type) + item_type = es_app.registry[COLLECTIONS][item_name].type_info.item_type + namespaced_index = get_namespaced_index(es_app, item_type) es_direct_count = es.count(index=namespaced_index, doc_type=item_type).get('count') assert es_item_count == es_direct_count @@ -578,7 +580,7 @@ def test_index_data_workbook(app, workbook, testapp, indexer_testapp, htmltestap if es_item_count == 0: continue # check items in search result individually - res = check_item_type(client=testapp, item_type=item_type) + res = check_item_type(client=es_testapp, item_type=item_type) for item_res in res.json.get('@graph', []): index_view_res = es.get(index=namespaced_index, doc_type=item_type, id=item_res['uuid'])['_source'] @@ -590,26 +592,26 @@ def test_index_data_workbook(app, workbook, testapp, indexer_testapp, htmltestap assert found_uuids <= set([link['uuid'] for link in index_view_res['linked_uuids_embedded']]) # if uuids_rev_linking to me, make sure they show up in @@links if len(index_view_res.get('uuids_rev_linked_to_me', [])) > 0: - links_res = testapp.get('/' + item_res['uuid'] + '/@@links', status=200) + links_res = es_testapp.get('/' + item_res['uuid'] + '/@@links', status=200) link_uuids = [lnk['uuid'] for lnk in links_res.json.get('uuids_linking_to')] assert set(index_view_res['uuids_rev_linked_to_me']) <= set(link_uuids) # previously test_html_pages try: - html_res = htmltestapp.get(item_res['@id']) + html_res = html_es_testapp.get(item_res['@id']) assert html_res.body.startswith(b'') except Exception as e: pass -###################################### -## Search-based visualization tests ## -###################################### +# ###################################### +# ## Search-based visualization tests ## +# ###################################### -def test_barplot_aggregation_endpoint(workbook, testapp): +def test_barplot_aggregation_endpoint(workbook, es_testapp): # Check what we get back - - search_result = testapp.get('/browse/?type=ExperimentSetReplicate&experimentset_type=replicate').json + search_result = es_testapp.get('/browse/?type=ExperimentSetReplicate&experimentset_type=replicate').json search_result_count = len(search_result['@graph']) # We should get back same count as from search results here. But on Travis oftentime we don't, so we compare either against count of inserts --or-- count returned from regular results. @@ -618,9 +620,9 @@ def test_barplot_aggregation_endpoint(workbook, testapp): # Now, test the endpoint after ensuring we have the data correctly loaded into ES. # We should get back same count as from search results here. - res = testapp.post_json('/bar_plot_aggregations', { - "search_query_params" : { "type" : ['ExperimentSetReplicate'] }, - "fields_to_aggregate_for" : ["experiments_in_set.experiment_type.display_title", "award.project"] + res = es_testapp.post_json('/bar_plot_aggregations', { + "search_query_params": {"type": ['ExperimentSetReplicate']}, + "fields_to_aggregate_for": ["experiments_in_set.experiment_type.display_title", "award.project"] }).json print() @@ -629,22 +631,22 @@ def test_barplot_aggregation_endpoint(workbook, testapp): assert (res['total']['experiment_sets'] == count_exp_set_test_inserts) or (res['total']['experiment_sets'] == search_result_count) - assert res['field'] == 'experiments_in_set.experiment_type.display_title' # top level field + assert res['field'] == 'experiments_in_set.experiment_type.display_title' # top level field assert isinstance(res['terms'], dict) is True assert len(res["terms"].keys()) > 0 - #assert isinstance(res['terms']["CHIP-seq"], dict) is True # A common term likely to be found. + # assert isinstance(res['terms']["CHIP-seq"], dict) is True # A common term likely to be found. - #assert res["terms"]["CHIP-seq"]["field"] == "award.project" # Child-field + # assert res["terms"]["CHIP-seq"]["field"] == "award.project" # Child-field # We only have 4DN as single award.project in test inserts so should have values in all buckets, though probably less than total. - #assert res["terms"]["CHIP-seq"]["total"]["experiment_sets"] > 0 - #assert res["terms"]["CHIP-seq"]["total"]["experiment_sets"] < count_exp_set_test_inserts + # assert res["terms"]["CHIP-seq"]["total"]["experiment_sets"] > 0 + # assert res["terms"]["CHIP-seq"]["total"]["experiment_sets"] < count_exp_set_test_inserts - #assert res["terms"]["CHIP-seq"]["terms"]["4DN"]["experiment_sets"] > 0 - #assert res["terms"]["CHIP-seq"]["terms"]["4DN"]["experiment_sets"] < count_exp_set_test_inserts + # assert res["terms"]["CHIP-seq"]["terms"]["4DN"]["experiment_sets"] > 0 + # assert res["terms"]["CHIP-seq"]["terms"]["4DN"]["experiment_sets"] < count_exp_set_test_inserts @pytest.fixture(scope='session') @@ -708,10 +710,10 @@ def hidden_facet_data_two(): @pytest.fixture(scope='module') # TODO consider this further... -def hidden_facet_test_data(testapp, hidden_facet_data_one, hidden_facet_data_two): - testapp.post_json('/TestingHiddenFacets', hidden_facet_data_one, status=201) - testapp.post_json('/TestingHiddenFacets', hidden_facet_data_two, status=201) - testapp.post_json('/index', {'record': False}) +def hidden_facet_test_data(es_testapp, hidden_facet_data_one, hidden_facet_data_two): + es_testapp.post_json('/TestingHiddenFacets', hidden_facet_data_one, status=201) + es_testapp.post_json('/TestingHiddenFacets', hidden_facet_data_two, status=201) + es_testapp.post_json('/index', {'record': False}) class TestSearchHiddenAndAdditionalFacets: @@ -736,24 +738,24 @@ def check_and_verify_result(facets, desired_facet, number_expected): continue break - def test_search_default_hidden_facets_dont_show(self, testapp, hidden_facet_test_data): - facets = testapp.get('/search/?type=TestingHiddenFacets').json['facets'] + def test_search_default_hidden_facets_dont_show(self, es_testapp, hidden_facet_test_data): + facets = es_testapp.get('/search/?type=TestingHiddenFacets').json['facets'] actual = [facet['field'] for facet in facets] assert self.DEFAULT_FACETS == sorted(actual) @pytest.mark.parametrize('facet', ADDITIONAL_FACETS) - def test_search_one_additional_facet(self, testapp, hidden_facet_test_data, facet): + def test_search_one_additional_facet(self, es_testapp, hidden_facet_test_data, facet): """ Tests that specifying each of the 'additional' facets works correctly """ - facets = testapp.get('/search/?type=TestingHiddenFacets&additional_facet=%s' % facet).json['facets'] + facets = es_testapp.get('/search/?type=TestingHiddenFacets&additional_facet=%s' % facet).json['facets'] expected = self.DEFAULT_FACETS + [facet] actual = [facet['field'] for facet in facets] assert sorted(expected) == sorted(actual) - def test_search_multiple_additional_facets(self, testapp, hidden_facet_test_data): + def test_search_multiple_additional_facets(self, es_testapp, hidden_facet_test_data): """ Tests that enabling multiple additional facets works """ - facets = testapp.get('/search/?type=TestingHiddenFacets' - '&additional_facet=unfaceted_string' - '&additional_facet=unfaceted_integer').json['facets'] + facets = es_testapp.get('/search/?type=TestingHiddenFacets' + '&additional_facet=unfaceted_string' + '&additional_facet=unfaceted_integer').json['facets'] expected = self.DEFAULT_FACETS + self.ADDITIONAL_FACETS for facet in facets: assert facet['field'] in expected @@ -763,18 +765,18 @@ def test_search_multiple_additional_facets(self, testapp, hidden_facet_test_data assert facet['aggregation_type'] == 'terms' @pytest.mark.parametrize('facet', DEFAULT_HIDDEN_FACETS) - def test_search_one_additional_default_hidden_facet(self, testapp, hidden_facet_test_data, facet): + def test_search_one_additional_default_hidden_facet(self, es_testapp, hidden_facet_test_data, facet): """ Tests that passing default_hidden facets to additional_facets works correctly """ - facets = testapp.get('/search/?type=TestingHiddenFacets&additional_facet=%s' % facet).json['facets'] + facets = es_testapp.get('/search/?type=TestingHiddenFacets&additional_facet=%s' % facet).json['facets'] expected = self.DEFAULT_FACETS + [facet] actual = [facet['field'] for facet in facets] assert sorted(expected) == sorted(actual) - def test_search_multiple_additional_default_hidden_facets(self, testapp, hidden_facet_test_data): + def test_search_multiple_additional_default_hidden_facets(self, es_testapp, hidden_facet_test_data): """ Tests that passing multiple hidden_facets as additionals works correctly """ - facets = testapp.get('/search/?type=TestingHiddenFacets' - '&additional_facet=last_name' - '&additional_facet=sid').json['facets'] + facets = es_testapp.get('/search/?type=TestingHiddenFacets' + '&additional_facet=last_name' + '&additional_facet=sid').json['facets'] expected = self.DEFAULT_FACETS + self.DEFAULT_HIDDEN_FACETS for facet in facets: assert facet['field'] in expected @@ -787,12 +789,12 @@ def test_search_multiple_additional_default_hidden_facets(self, testapp, hidden_ ['last_name', 'unfaceted_integer'], # second slot holds number field ['unfaceted_string', 'sid'] ]) - def test_search_mixing_additional_and_default_hidden(self, testapp, hidden_facet_test_data, _facets): + def test_search_mixing_additional_and_default_hidden(self, es_testapp, hidden_facet_test_data, _facets): """ Tests that we can mix additional_facets with those both on and off schema """ [sample_string_field, sample_number_field] = _facets - facets = testapp.get('/search/?type=TestingHiddenFacets' - '&additional_facet=%s' - '&additional_facet=%s' % (sample_string_field, sample_number_field)).json['facets'] + facets = es_testapp.get('/search/?type=TestingHiddenFacets' + '&additional_facet=%s' + '&additional_facet=%s' % (sample_string_field, sample_number_field)).json['facets'] expected = self.DEFAULT_FACETS + _facets actual = [facet['field'] for facet in facets] assert sorted(expected) == sorted(actual) @@ -803,9 +805,9 @@ def test_search_mixing_additional_and_default_hidden(self, testapp, hidden_facet assert facet['aggregation_type'] == 'terms' @pytest.mark.parametrize('_facet', DISABLED_FACETS) - def test_search_disabled_overrides_additional(self, testapp, hidden_facet_test_data, _facet): + def test_search_disabled_overrides_additional(self, es_testapp, hidden_facet_test_data, _facet): """ Hidden facets should NEVER be faceted on """ - facets = testapp.get('/search/?type=TestingHiddenFacets&additional_facet=%s' % _facet).json['facets'] + facets = es_testapp.get('/search/?type=TestingHiddenFacets&additional_facet=%s' % _facet).json['facets'] field_names = [facet['field'] for facet in facets] assert _facet not in field_names # always hidden should not be here, even if specified @@ -813,13 +815,13 @@ def test_search_disabled_overrides_additional(self, testapp, hidden_facet_test_d ('last_name', 'unfaceted_integer', 'disabled_integer'), # default_hidden second ('sid', 'unfaceted_string', 'disabled_string') # disabled always last ]) - def test_search_additional_mixing_disabled_default_hidden(self, testapp, hidden_facet_test_data, _facets): + def test_search_additional_mixing_disabled_default_hidden(self, es_testapp, hidden_facet_test_data, _facets): """ Tests that supplying multiple additional facets combined with hidden still respects the hidden restriction. """ - facets = testapp.get('/search/?type=TestingHiddenFacets' - '&additional_facet=%s' - '&additional_facet=%s' - '&additional_facet=%s' % (_facets[0], _facets[1], _facets[2])).json['facets'] + facets = es_testapp.get('/search/?type=TestingHiddenFacets' + '&additional_facet=%s' + '&additional_facet=%s' + '&additional_facet=%s' % (_facets[0], _facets[1], _facets[2])).json['facets'] expected = self.DEFAULT_FACETS + [_facets[0], _facets[1]] # first two should show actual = [facet['field'] for facet in facets] assert sorted(expected) == sorted(actual) @@ -828,10 +830,10 @@ def test_search_additional_mixing_disabled_default_hidden(self, testapp, hidden_ 'unfaceted_object.mother', 'unfaceted_object.father' ]) - def test_search_additional_object_facets(self, testapp, hidden_facet_test_data, _facet): + def test_search_additional_object_facets(self, es_testapp, hidden_facet_test_data, _facet): """ Tests that specifying an object field as an additional_facet works correctly """ - facets = testapp.get('/search/?type=TestingHiddenFacets' - '&additional_facet=%s' % _facet).json['facets'] + facets = es_testapp.get('/search/?type=TestingHiddenFacets' + '&additional_facet=%s' % _facet).json['facets'] expected = self.DEFAULT_FACETS + [_facet] actual = [facet['field'] for facet in facets] assert sorted(expected) == sorted(actual) @@ -841,11 +843,11 @@ def test_search_additional_object_facets(self, testapp, hidden_facet_test_data, ('unfaceted_array_of_objects.color', 3), ('unfaceted_array_of_objects.uid', 2.5) # stats avg ]) - def test_search_additional_nested_facets(self, testapp, hidden_facet_test_data, _facet, n_expected): + def test_search_additional_nested_facets(self, es_testapp, hidden_facet_test_data, _facet, n_expected): """ Tests that specifying an array of object field mapped with nested as an additional_facet works correctly. """ - [desired_facet] = [facet for facet in testapp.get('/search/?type=TestingHiddenFacets' - '&additional_facet=%s' % _facet).json['facets'] + [desired_facet] = [facet for facet in es_testapp.get('/search/?type=TestingHiddenFacets' + '&additional_facet=%s' % _facet).json['facets'] if facet['field'] == _facet] if 'terms' in desired_facet: assert len(desired_facet['terms']) == n_expected @@ -853,11 +855,11 @@ def test_search_additional_nested_facets(self, testapp, hidden_facet_test_data, assert desired_facet['avg'] == n_expected @pytest.fixture - def many_non_nested_facets(self, testapp, hidden_facet_test_data): - return testapp.get('/search/?type=TestingHiddenFacets' - '&additional_facet=non_nested_array_of_objects.fruit' - '&additional_facet=non_nested_array_of_objects.color' - '&additional_facet=non_nested_array_of_objects.uid').json['facets'] + def many_non_nested_facets(self, es_testapp, hidden_facet_test_data): + return es_testapp.get('/search/?type=TestingHiddenFacets' + '&additional_facet=non_nested_array_of_objects.fruit' + '&additional_facet=non_nested_array_of_objects.color' + '&additional_facet=non_nested_array_of_objects.uid').json['facets'] @pytest.mark.parametrize('_facet, n_expected', [ ('unfaceted_array_of_objects.fruit', 4), @@ -897,10 +899,10 @@ def bucket_range_data_raw(): @pytest.fixture(scope='module') # XXX: consider scope further - Will 11/5/2020 -def bucket_range_data(testapp, bucket_range_data_raw): +def bucket_range_data(es_testapp, bucket_range_data_raw): for entry in bucket_range_data_raw: - testapp.post_json('/TestingBucketRangeFacets', entry, status=201) - testapp.post_json('/index', {'record': False}) + es_testapp.post_json('/TestingBucketRangeFacets', entry, status=201) + es_testapp.post_json('/index', {'record': False}) class TestSearchBucketRangeFacets: @@ -930,30 +932,30 @@ def select_facet(facets, facet_name): (['special_integer', 'special_object_that_holds_integer.embedded_integer'], 5), (['array_of_objects_that_holds_integer.embedded_integer'], 10) ]) - def test_search_bucket_range_simple(self, testapp, bucket_range_data, expected_fields, expected_counts): + def test_search_bucket_range_simple(self, es_testapp, bucket_range_data, expected_fields, expected_counts): """ Tests searching a collection of documents with varying integer field types that have the same distribution - all of which should give the same results. """ - res = testapp.get('/search/?type=TestingBucketRangeFacets').json['facets'] + res = es_testapp.get('/search/?type=TestingBucketRangeFacets').json['facets'] self.verify_facet_counts(res, expected_fields, 2, expected_counts) @pytest.mark.parametrize('identifier', [ 'reverse', 'forward' ]) - def test_search_bucket_range_nested_qualifier(self, testapp, bucket_range_data, identifier): + def test_search_bucket_range_nested_qualifier(self, es_testapp, bucket_range_data, identifier): """ Tests aggregating on a nested field while selecting for a field within the nested object. """ - res = testapp.get('/search/?type=TestingBucketRangeFacets' - '&array_of_objects_that_holds_integer.embedded_identifier=%s' % identifier).json['facets'] + res = es_testapp.get('/search/?type=TestingBucketRangeFacets' + '&array_of_objects_that_holds_integer.embedded_identifier=%s' % identifier).json['facets'] self.verify_facet_counts(res, ['array_of_objects_that_holds_integer.embedded_integer'], 2, 10) @pytest.mark.parametrize('identifier', [ 'reverse', 'forward' ]) - def test_search_bucket_range_nested_qualifier(self, testapp, bucket_range_data, identifier): + def test_search_bucket_range_nested_qualifier(self, es_testapp, bucket_range_data, identifier): """ Tests aggregating on a nested field while selecting for a field within the nested object (no change). """ - res = testapp.get('/search/?type=TestingBucketRangeFacets' - '&array_of_objects_that_holds_integer.embedded_integer.from=6' - '&array_of_objects_that_holds_integer.embedded_identifier=%s' % identifier).json['facets'] + res = es_testapp.get('/search/?type=TestingBucketRangeFacets' + '&array_of_objects_that_holds_integer.embedded_integer.from=6' + '&array_of_objects_that_holds_integer.embedded_identifier=%s' % identifier).json['facets'] self.verify_facet_counts(res, ['array_of_objects_that_holds_integer.embedded_integer'], 2, 10) facet_with_labels = self.select_facet(res, 'array_of_objects_that_holds_integer.embedded_integer') diff --git a/src/encoded/tests/test_static_page.py b/src/encoded/tests/test_static_page.py index 0e7cf013eb..24cdb38d43 100644 --- a/src/encoded/tests/test_static_page.py +++ b/src/encoded/tests/test_static_page.py @@ -1,146 +1,117 @@ import pytest -import webtest from dcicutils.qa_utils import notice_pytest_fixtures -from .workbook_fixtures import app_settings, app # are these needed? -kmp 12-Mar-2021 - - -notice_pytest_fixtures(app_settings, app) - -pytestmark = [pytest.mark.indexing, pytest.mark.working] - - -@pytest.fixture(scope='module') -def help_page_section_json(): - return { - "title": "", - "name" : "help.user-guide.rest-api.rest_api_submission", - "file": "/docs/source/rest_api_submission.rst", - "uuid" : "442c8aa0-dc6c-43d7-814a-854af460b020" - } - -@pytest.fixture(scope='module') -def help_page_json(): - return { - "name": "help/user-guide/rest-api", - "title": "The REST-API", - "content": ["442c8aa0-dc6c-43d7-814a-854af460b020"], - "uuid": "a2aa8bb9-9dd9-4c80-bdb6-2349b7a3540d", - "table-of-contents": { - "enabled": True, - "header-depth": 4, - "list-styles": ["decimal", "lower-alpha", "lower-roman"] - } - } - -@pytest.fixture(scope='module') -def help_page_json_draft(): - return { - "name": "help/user-guide/rest-api-draft", - "title": "The REST-API", - "content": ["442c8aa0-dc6c-43d7-814a-854af460b020"], - "uuid": "a2aa8bb9-9dd9-4c80-bdb6-2349b7a3540c", - "table-of-contents": { - "enabled": True, - "header-depth": 4, - "list-styles": ["decimal", "lower-alpha", "lower-roman"] - }, - "status" : "draft" - } - -@pytest.fixture(scope='module') -def help_page_json_deleted(): - return { - "name": "help/user-guide/rest-api-deleted", - "title": "The REST-API", - "content": ["442c8aa0-dc6c-43d7-814a-854af460b020"], - "uuid": "a2aa8bb9-9dd9-4c80-bdb6-2349b7a3540a", - "table-of-contents": { - "enabled": True, - "header-depth": 4, - "list-styles": ["decimal", "lower-alpha", "lower-roman"] - }, - "status" : "deleted" - } - - -@pytest.fixture(scope='module') -def posted_help_page_section(testapp, help_page_section_json): - try: - res = testapp.post_json('/static-sections/', help_page_section_json, status=201) - val = res.json['@graph'][0] - except webtest.AppError: - res = testapp.get('/' + help_page_section_json['uuid'], status=301).follow() - val = res.json - return val - - -@pytest.fixture(scope='module') -def help_page(testapp, posted_help_page_section, help_page_json): - try: - res = testapp.post_json('/pages/', help_page_json, status=201) - val = res.json['@graph'][0] - except webtest.AppError: - res = testapp.get('/' + help_page_json['uuid'], status=301).follow() - val = res.json - return val - - -@pytest.fixture(scope='module') -def help_page_deleted(testapp, posted_help_page_section, help_page_json_draft): - try: - res = testapp.post_json('/pages/', help_page_json_draft, status=201) - val = res.json['@graph'][0] - except webtest.AppError: - res = testapp.get('/' + help_page_json_draft['uuid'], status=301).follow() - val = res.json - return val - - -@pytest.fixture(scope='module') -def help_page_restricted(testapp, posted_help_page_section, help_page_json_deleted): - try: - res = testapp.post_json('/pages/', help_page_json_deleted, status=201) - val = res.json['@graph'][0] - except webtest.AppError: - res = testapp.get('/' + help_page_json_deleted['uuid'], status=301).follow() - val = res.json - return val - - -def test_get_help_page(testapp, help_page): - help_page_url = "/" + help_page['name'] - res = testapp.get(help_page_url, status=200) - assert res.json['@id'] == help_page_url - assert res.json['@context'] == help_page_url - assert 'HelpPage' in res.json['@type'] - assert 'StaticPage' in res.json['@type'] - #assert res.json['content'] == help_page['content'] # No longer works latter is set to an @id of static_section - assert 'Accession and uuid are automatically assigned during initial posting' in res.json['content'][0]['content'] # Instead lets check what we have embedded on GET request is inside our doc file (rest_api_submission.md). - assert res.json['toc'] == help_page['table-of-contents'] +from ..util import workbook_lookup + + +pytestmark = [pytest.mark.working, pytest.mark.workbook] + + +def wait_for_index(testapp): + testapp.post_json("/index", {"record": False}) -def test_get_help_page_deleted(anonhtmltestapp, help_page_deleted): - help_page_url = "/" + help_page_deleted['name'] - anonhtmltestapp.get(help_page_url, status=403) +@pytest.fixture +def static_help_page_default(): + return workbook_lookup(item_type='Page', name='help/user-guide/rest-api') -def test_get_help_page_no_access(anonhtmltestapp, testapp, help_page_restricted): - help_page_url = "/" + help_page_restricted['name'] - anonhtmltestapp.get(help_page_url, status=403) - testapp.get(help_page_url, status=200) +def test_static_help_page_default(static_help_page_default): + assert static_help_page_default['name'] == 'help/user-guide/rest-api' -def test_page_unique_name(testapp, help_page, help_page_deleted): +@pytest.fixture +def static_help_page_draft(): + return workbook_lookup(item_type='Page', name='help/user-guide/rest-api-draft') + + +@pytest.fixture +def static_help_page_deleted(): + return workbook_lookup(item_type='Page', name='help/user-guide/rest-api-deleted') + + +def test_get_help_page(workbook, es_testapp, static_help_page_default): + wait_for_index(es_testapp) + help_page_url = "/" + static_help_page_default['name'] + res = es_testapp.get(help_page_url, status=200) + assert res.json['@id'] == help_page_url + assert res.json['@context'] == help_page_url + assert 'HelpPage' in res.json['@type'] + assert 'StaticPage' in res.json['@type'] + # assert res.json['content'] == help_page['content'] # No longer works latter is set to an @id of static_section + # Instead lets check what we have embedded on GET request is inside our doc file (rest_api_submission.md). + assert 'Accession and uuid are automatically assigned during initial posting' in res.json['content'][0]['content'] + assert res.json['toc'] == static_help_page_default['table-of-contents'] + + +def test_get_help_page_draft(workbook, anon_html_es_testapp, html_es_testapp, static_help_page_draft): + wait_for_index(html_es_testapp) + help_page_url = "/" + static_help_page_draft['name'] + anon_html_es_testapp.get(help_page_url, status=403) + html_es_testapp.get(help_page_url, status=200) + + +def test_get_help_page_deleted(workbook, anon_html_es_testapp, html_es_testapp, static_help_page_deleted): + wait_for_index(html_es_testapp) + help_page_url = "/" + static_help_page_deleted['name'] + anon_html_es_testapp.get(help_page_url, status=403) + html_es_testapp.get(help_page_url, status=200) # Why 200 and not 404? -kmp 23-Feb-2021 + + +def test_get_help_page_no_access(workbook, anon_es_testapp, es_testapp, anon_html_es_testapp, html_es_testapp, + static_help_page_default, static_help_page_draft, static_help_page_deleted): + notice_pytest_fixtures(workbook) + wait_for_index(es_testapp) + success = True + for app_name, testapp, role in [("anon_es", anon_es_testapp, 'anon'), + ("es", es_testapp, 'system'), + ("anon_html_es", anon_html_es_testapp, 'anon'), + ("html_es", html_es_testapp, 'system')]: + for help_page, is_public in [(static_help_page_default, True), + (static_help_page_draft, False), + (static_help_page_deleted, False)]: + expected_code = 200 if is_public else (403 if role == 'anon' else 200) + page_name = help_page['name'] + help_page_url = "/" + page_name + res = testapp.get(help_page_url, status=(200, 301, 403, 404)).maybe_follow() + actual_code = res.status_code + if actual_code == expected_code: + print("%s => %s: SUCCESS (%s)" % (app_name, page_name, actual_code)) + else: + print("%s => %s: FAILED (%s, not %s): %s..." + % (app_name, page_name, actual_code, expected_code, res.body[:20])) + success = False + assert success, "Test failed." + + +def check_page_unique_name(testapp, conflicting_page, page_to_patch): + wait_for_index(testapp) # POST again with same name and expect validation error - new_page = {'name': help_page['name']} - res = testapp.post_json('/page', new_page, status=422) - expected_val_err = "%s already exists with name '%s'" % (help_page['uuid'], new_page['name']) - actual_error_description = res.json['errors'][0]['description'] - print("expected:", expected_val_err) - print("actual:", actual_error_description) - assert expected_val_err in actual_error_description - - # also test PATCH of an existing page with another name - res = testapp.patch_json(help_page_deleted['@id'], {'name': new_page['name']}, status=422) - assert expected_val_err in res.json['errors'][0]['description'] + conflicting_document = {'name': conflicting_page['name']} + conflict_message = "%s already exists with name '%s'" % (conflicting_page['uuid'], conflicting_page['name']) + + def check_conflict(res): + actual_error_description = res.json['errors'][0]['description'] + print("expected:", conflict_message) + print("actual:", actual_error_description) + assert conflict_message in actual_error_description + + # Test that POST of a new page with the same name as an existing page is not allowed. + check_conflict(testapp.post_json('/page', conflicting_document, status=422)) + # Also test PATCH of an existing page with the same name as another existing page is not allowed. + page_to_patch_uuid_url = '/' + page_to_patch['uuid'] + check_conflict(testapp.patch_json(page_to_patch_uuid_url, conflicting_document, status=422)) + actual_page_to_patch = testapp.get(page_to_patch_uuid_url).maybe_follow().json + check_conflict(testapp.patch_json(actual_page_to_patch['@id'], conflicting_document, status=422)) + + +def test_page_unique_name(workbook, es_testapp, static_help_page_draft, static_help_page_default): + check_page_unique_name(testapp=es_testapp, + conflicting_page=static_help_page_default, + page_to_patch=static_help_page_draft) + + +def test_page_unique_name_deleted(workbook, es_testapp, static_help_page_draft, static_help_page_deleted): + check_page_unique_name(testapp=es_testapp, + conflicting_page=static_help_page_deleted, + page_to_patch=static_help_page_draft) diff --git a/src/encoded/tests/test_validation_errors.py b/src/encoded/tests/test_validation_errors.py index 0b6cba0bfc..177efce91e 100644 --- a/src/encoded/tests/test_validation_errors.py +++ b/src/encoded/tests/test_validation_errors.py @@ -1,11 +1,5 @@ import pytest -from dcicutils.qa_utils import notice_pytest_fixtures -from .workbook_fixtures import app_settings, workbook -# from ..util import delay_rerun - - -notice_pytest_fixtures(app_settings, workbook) pytestmark = [ pytest.mark.working, @@ -14,20 +8,21 @@ # pytest.mark.flaky(rerun_filter=delay_rerun), ] + @pytest.mark.skip(reason="validation_errors facet was removed in search.py") -def test_validation_err_facet(workbook, testapp): - res = testapp.get('/search/?type=ExperimentSetReplicate').json +def test_validation_err_facet(workbook, es_testapp): + res = es_testapp.get('/search/?type=ExperimentSetReplicate').json val_err_facets = [facet for facet in res['facets'] if facet['title'] == 'Validation Errors'] assert len(val_err_facets) == 1 assert val_err_facets[0]['aggregation_type'] == 'terms' -def test_validation_err_itemview(workbook, testapp): - res = testapp.get('/experiment-set-replicates/4DNESAAAAAA1/').json +def test_validation_err_itemview(workbook, es_testapp): + res = es_testapp.get('/experiment-set-replicates/4DNESAAAAAA1/').json assert 'validation-errors' in res.keys() -def test_validation_err_view(workbook, testapp): - res = testapp.get('/experiment-set-replicates/4DNESAAAAAA1/@@validation-errors').json +def test_validation_err_view(workbook, es_testapp): + res = es_testapp.get('/experiment-set-replicates/4DNESAAAAAA1/@@validation-errors').json assert res['@id'] == '/experiment-set-replicates/4DNESAAAAAA1/' assert 'validation_errors' in res diff --git a/src/encoded/tests/workbook_fixtures.py b/src/encoded/tests/workbook_fixtures.py.DISABLED similarity index 100% rename from src/encoded/tests/workbook_fixtures.py rename to src/encoded/tests/workbook_fixtures.py.DISABLED From 3ccba1db694b501e345bf74cef7992ad4f49e2fc Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Tue, 23 Mar 2021 02:26:34 -0400 Subject: [PATCH 11/13] Add missing file to source control. --- .../tests/data/workbook-inserts/page.json | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 src/encoded/tests/data/workbook-inserts/page.json diff --git a/src/encoded/tests/data/workbook-inserts/page.json b/src/encoded/tests/data/workbook-inserts/page.json new file mode 100644 index 0000000000..c8599e2c48 --- /dev/null +++ b/src/encoded/tests/data/workbook-inserts/page.json @@ -0,0 +1,37 @@ +[ + { + "name": "help/user-guide/rest-api", + "title": "The REST-API", + "content": ["442c8aa0-dc6c-43d7-814a-854af460b020"], + "uuid": "a2aa8bb9-9dd9-4c80-bdb6-2349b7a3540d", + "table-of-contents": { + "enabled": true, + "header-depth": 4, + "list-styles": ["decimal", "lower-alpha", "lower-roman"] + } + }, + { + "name": "help/user-guide/rest-api-draft", + "title": "The REST-API", + "content": ["442c8aa0-dc6c-43d7-814a-854af460b020"], + "uuid": "a2aa8bb9-9dd9-4c80-bdb6-2349b7a3540c", + "table-of-contents": { + "enabled": true, + "header-depth": 4, + "list-styles": ["decimal", "lower-alpha", "lower-roman"] + }, + "status": "draft" + }, + { + "name": "help/user-guide/rest-api-deleted", + "title": "The REST-API", + "content": ["442c8aa0-dc6c-43d7-814a-854af460b020"], + "uuid": "a2aa8bb9-9dd9-4c80-bdb6-2349b7a3540a", + "table-of-contents": { + "enabled": true, + "header-depth": 4, + "list-styles": ["decimal", "lower-alpha", "lower-roman"] + }, + "status": "deleted" + } +] From 5fe5173c5b59af54f4551b7f21b7793b8f13a669 Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Tue, 23 Mar 2021 08:06:53 -0400 Subject: [PATCH 12/13] Remove a comment. Fix a typo. --- Makefile | 2 +- src/encoded/tests/test_indexing.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 51d35eca42..a14e61e3ba 100644 --- a/Makefile +++ b/Makefile @@ -160,7 +160,7 @@ help: info: @: $(info Here are some 'make' options:) $(info - Use 'make aws-ip-ranges' to download latest ip range information. Invoked automatically when needed.) - $(info - Use 'make build' (or 'make macbuild' on OSX Catalina) to build onlgggsgsgggy application dependencies.) + $(info - Use 'make build' (or 'make macbuild' on OSX Catalina) to build only application dependencies.) $(info - Use 'make build-dev' (or 'make macbuild-dev' on OSX Catalina) to build all dependencies, even locust.) $(info - Use 'make build-locust' to install locust. Do not do this unless you know what you are doing.) $(info - Use 'make clean' to clear out (non-python) dependencies.) diff --git a/src/encoded/tests/test_indexing.py b/src/encoded/tests/test_indexing.py index ac2e568540..2e5fe36027 100644 --- a/src/encoded/tests/test_indexing.py +++ b/src/encoded/tests/test_indexing.py @@ -33,12 +33,10 @@ from .. import main, loadxl from ..util import delay_rerun from ..verifier import verify_item -# from .workbook_fixtures import app_settings from .test_permissions import wrangler, wrangler_testapp notice_pytest_fixtures( - # app_settings, wrangler, wrangler_testapp ) From ca3ed8580a7663d01ebe05b7ebeda8bd079a72cd Mon Sep 17 00:00:00 2001 From: Kent Pitman Date: Tue, 23 Mar 2021 10:04:32 -0400 Subject: [PATCH 13/13] Clean up renderers and test_renders. Fix a small detail in the kibana start code. --- scripts/kibana-start | 2 +- src/encoded/renderers.py | 5 +-- src/encoded/tests/test_renderers.py | 57 +++++++++++++++++++++-------- 3 files changed, 44 insertions(+), 20 deletions(-) diff --git a/scripts/kibana-start b/scripts/kibana-start index 3b311bf451..a62631651f 100755 --- a/scripts/kibana-start +++ b/scripts/kibana-start @@ -70,7 +70,7 @@ if [ -z "${existing_network}" ]; then docker network create localnet --driver=bridge else echo "docker localnet is already set up. From 'docker network':" - # echo " ${existing_network}" + echo " ${existing_network}" fi # This pattern is structured so that if we need to extract the container id (e.g., to kill it), it will be in \1. diff --git a/src/encoded/renderers.py b/src/encoded/renderers.py index 157215f9d5..744ee9a417 100644 --- a/src/encoded/renderers.py +++ b/src/encoded/renderers.py @@ -435,15 +435,14 @@ def should_transform(request, response): # Web browsers send an Accept request header for initial (e.g. non-AJAX) page requests # which should contain 'text/html' # See: https://tedboy.github.io/flask/generated/generated/werkzeug.Accept.best_match.html#werkzeug-accept-best-match - mime_type = best_mime_type(request) - mime_type_format = mime_type.split('/', 1)[1] # Will be 1 of 'html', 'json', 'json-ld' + mime_type = best_mime_type(request) # Result will be one of MIME_TYPES_SUPPORTED # N.B. ld+json (JSON-LD) is likely more unique case and might be sent by search engines (?) # which can parse JSON-LDs. At some point we could maybe have it to be same as # making an `@@object` or `?frame=object` request (?) esp if fill # out @context response w/ schema(s) (or link to schema) - return mime_type_format == 'html' + return mime_type == MIME_TYPE_HTML def render_page_html_tween_factory(handler, registry): diff --git a/src/encoded/tests/test_renderers.py b/src/encoded/tests/test_renderers.py index bfa2b61aa6..c86c54b1e4 100644 --- a/src/encoded/tests/test_renderers.py +++ b/src/encoded/tests/test_renderers.py @@ -8,29 +8,57 @@ from unittest import mock from .. import renderers from ..renderers import ( - should_transform, best_mime_type, MIME_TYPE_HTML, MIME_TYPE_JSON, MIME_TYPE_LD_JSON, - MIME_TYPES_SUPPORTED, MIME_TYPE_DEFAULT, MIME_TYPE_TRIAGE_MODE, + 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' - assert set(MIME_TYPES_SUPPORTED) == {MIME_TYPE_HTML, MIME_TYPE_JSON, MIME_TYPE_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... + assert MIME_TYPES_SUPPORTED == [MIME_TYPE_HTML, MIME_TYPE_JSON, MIME_TYPE_LD_JSON] assert MIME_TYPE_DEFAULT == MIME_TYPE_HTML + + # 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'] -def test_best_mime_type(the_constant_answer=MIME_TYPE_DEFAULT): - # TODO: I think this function is not doing the right thing. It SHOULD be looking at the Accept header +VARIOUS_MIME_TYPES_TO_TEST = ['*/*', 'text/html', 'application/json', 'application/ld+json', 'text/xml', 'who/cares'] + + +def test_best_mime_type(): + + # TODO: I think best_mime_type is not doing the right thing. It SHOULD be looking at the Accept header # not at a constant list. For now a test of legacy behavior to keep it stable, but at some point # we should experiment with making it responsive to bids in the Accept field. # -kmp 18-Mar-2021 + + 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 @@ -45,17 +73,6 @@ def test_best_mime_type(the_constant_answer=MIME_TYPE_DEFAULT): assert best_mime_type(req, 'modern') == the_constant_answer -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) - - -VARIOUS_MIME_TYPES_TO_TEST = ['*/*', 'text/html', 'application/json', 'application/ld+json', 'text/xml', 'who/cares'] - TYPICAL_URLS = [ 'http://whatever/foo', 'http://whatever/foo/', @@ -167,7 +184,15 @@ def test_should_transform(): 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()