From 3944a4489930f90fe10e8a9730afcf197fb9a74b Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Thu, 26 Sep 2024 15:40:57 +0200 Subject: [PATCH 01/34] Extend MigTestCase with such that it can provide a configuration object. A great deal of code that we wish to exercise under test requires a configuration object. The wiring to generate test configuration values is already in tree, but actually consuming it in tests was done in an ad-hoc fashion. Meanwhile the separate notion of a FakeConfiguration was added primarily to allow exercising code that needs a specific subset of vars set with specific values. This patch raises this use-case to the level of the common support library. Use the way a logger was exposed as a blueprint and add a configuration property to MigTestCase - this is a computed property that lazily creates a (by default) FakeConfiguration object. This object persists for the duration of the test. Provide a means for easily overriding this to make it a fully populated Configuration instance using the testconfig - this mechanism is _declarative_ such that the details of configuration loading are transparent to callers. Document this use via an extra test case in the support library tests. --- tests/support/__init__.py | 45 +++++++++++++++++++++++++++++++++++++++ tests/test_support.py | 34 ++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/tests/support/__init__.py b/tests/support/__init__.py index 1b04f74f2..b271645e8 100644 --- a/tests/support/__init__.py +++ b/tests/support/__init__.py @@ -40,6 +40,7 @@ import sys from unittest import TestCase, main as testmain +from tests.support.configsupp import FakeConfiguration from tests.support.suppconst import MIG_BASE, TEST_BASE, TEST_FIXTURE_DIR, \ TEST_OUTPUT_DIR @@ -48,6 +49,20 @@ # force defaults to a local environment os.environ['MIG_ENV'] = 'local' +# expose the configuraed environment as a constant +MIG_ENV = os.environ['MIG_ENV'] + +if MIG_ENV == 'local': + # force testconfig as the conig file path + is_py2 = PY2 + _conf_dir_suffix = "-py%s" % ('2' if is_py2 else '3',) + _conf_dir = "testconfs%s" % (_conf_dir_suffix,) + _local_conf = os.path.join( + MIG_BASE, 'envhelp/output', _conf_dir, 'MiGserver.conf') + _config_file = os.getenv('MIG_CONF', None) + if _config_file is None: + os.environ['MIG_CONF'] = _local_conf + # All MiG related code will at some point include bits from the mig module # namespace. Rather than have this knowledge spread through every test file, # make the sole responsbility of test files to find the support file and @@ -103,6 +118,7 @@ def __init__(self, *args): super(MigTestCase, self).__init__(*args) self._cleanup_checks = list() self._cleanup_paths = set() + self._configuration = None self._logger = None self._skip_logging = False @@ -153,6 +169,31 @@ def _reset_logging(self, stream): root_handler = root_logger.handlers[0] root_handler.stream = stream + # testcase defaults + + @staticmethod + def _make_configuration_instance(configuration_to_make): + if configuration_to_make == 'fakeconfig': + return FakeConfiguration() + elif configuration_to_make == 'testconfig': + from mig.shared.conf import get_configuration_object + return get_configuration_object(skip_log=True, disable_auth_log=True) + else: + raise AssertionError( + "MigTestCase: unknown configuration %r" % (configuration_to_make,)) + + def _provide_configuration(self): + return 'fakeconfig' + + @property + def configuration(self): + """Init a fake configuration if not already done""" + if self._configuration is None: + configuration_to_make = self._provide_configuration() + self._configuration = self._make_configuration_instance( + configuration_to_make) + return self._configuration + @property def logger(self): """Init a fake logger if not already done""" @@ -199,6 +240,10 @@ def assertPathExists(self, relative_path): assert not os.path.isabs( relative_path), "expected relative path within output folder" absolute_path = os.path.join(TEST_OUTPUT_DIR, relative_path) + return MigTestCase._absolute_path_kind(absolute_path) + + @staticmethod + def _absolute_path_kind(absolute_path): stat_result = os.lstat(absolute_path) if stat.S_ISLNK(stat_result.st_mode): return "symlink" diff --git a/tests/test_support.py b/tests/test_support.py index 34fb61247..3ecc3e458 100644 --- a/tests/test_support.py +++ b/tests/test_support.py @@ -4,7 +4,10 @@ import unittest from tests.support import MigTestCase, PY2, testmain, temppath, \ - AssertOver + AssertOver, FakeConfiguration + +from mig.shared.conf import get_configuration_object +from mig.shared.configuration import Configuration class InstrumentedAssertOver(AssertOver): @@ -39,6 +42,17 @@ def _class_attribute(self, name, **kwargs): else: return getattr(cls, name, None) + def test_provides_a_fake_configuration(self): + configuration = self.configuration + + self.assertIsInstance(configuration, FakeConfiguration) + + def test_provides_a_fake_configuration_for_the_duration_of_the_test(self): + c1 = self.configuration + c2 = self.configuration + + self.assertIs(c2, c1) + @unittest.skipIf(PY2, "Python 3 only") def test_unclosed_files_are_recorded(self): tmp_path = temppath("support-unclosed", self) @@ -88,5 +102,23 @@ def test_when_asserting_over_multiple_values_after(self): self.assertTrue(attempt_wrapper.was_check_callable_called()) +class SupportTestCase_overridden_configuration(MigTestCase): + def _provide_configuration(self): + return 'testconfig' + + def test_provides_the_test_configuration(self): + expected_last_dir = 'testconfs-py2' if PY2 else 'testconfs-py3' + + configuration = self.configuration + + # check we have a real config object + self.assertIsInstance(configuration, Configuration) + # check for having loaded a config file from a test config dir + config_file_path_parts = configuration.config_file.split(os.path.sep) + config_file_path_parts.pop() # discard file part + config_file_last_dir = config_file_path_parts.pop() + self.assertTrue(config_file_last_dir, expected_last_dir) + + if __name__ == '__main__': testmain() From 69aad05c05348bc44381af7036064723e3b11e3a Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 2 Oct 2024 14:20:22 +0200 Subject: [PATCH 02/34] fix path overrides in python3 when generating the testconfig --- envhelp/makeconfig.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/envhelp/makeconfig.py b/envhelp/makeconfig.py index 46de93e5d..f76664b0c 100644 --- a/envhelp/makeconfig.py +++ b/envhelp/makeconfig.py @@ -58,12 +58,17 @@ def write_testconfig(env_name, is_py2=False): 'destination': os.path.join(_ENVHELP_OUTPUT_DIR, confs_name), 'destination_suffix': "-py%s" % ('2' if is_py2 else '3',), } + if is_py2: - overrides.update(**{ - 'mig_code': '/usr/src/app/mig', - 'mig_certs': '/usr/src/app/envhelp/output/certs', - 'mig_state': '/usr/src/app/envhelp/output/state', - }) + conf_dir_path = '/usr/src/app' + else: + conf_dir_path = _ENVHELP_OUTPUT_DIR + overrides.update(**{ + 'mig_code': os.path.join(conf_dir_path, 'mig'), + 'mig_certs': os.path.join(conf_dir_path, 'certs'), + 'mig_state': os.path.join(conf_dir_path, 'state'), + }) + generate_confs(_ENVHELP_OUTPUT_DIR, **overrides) From 18fd0978dde7f168abe917820f24e4655fb4ca37 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Fri, 26 Jul 2024 16:39:19 +0200 Subject: [PATCH 03/34] Basic coverage of migwsgi. --- mig/wsgi-bin/migwsgi.py | 24 +++-- tests/test_mig_wsgi-bin_migwsgi.py | 139 +++++++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 5 deletions(-) create mode 100644 tests/test_mig_wsgi-bin_migwsgi.py diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index 73987133e..34d132a93 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -193,6 +193,13 @@ def application(environ, start_response): *start_response* is a helper function used to deliver the client response. """ + def _set_os_environ(value): + os.environ = value + + return _application(environ, start_response, _format_output=format_output, _set_environ=_set_os_environ, _wrap_wsgi_errors=wrap_wsgi_errors) + +def _application(environ, start_response, _format_output, _set_environ, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False): + # NOTE: pass app environ including apache and query args on to sub handlers # through the usual 'os.environ' channel expected in functionality # handlers. Special care is needed to avoid various sub-interpreter @@ -235,18 +242,18 @@ def application(environ, start_response): os_env_value)) # Assign updated environ to LOCAL os.environ for the rest of this session - os.environ = environ + _set_environ(environ) # NOTE: redirect stdout to stderr in python 2 only. It breaks logger in 3 # and stdout redirection apparently is already handled there. if sys.version_info[0] < 3: sys.stdout = sys.stderr - configuration = get_configuration_object() + configuration = get_configuration_object(_config_file, _skip_log) _logger = configuration.logger # NOTE: replace default wsgi errors to apache error log with our own logs - wrap_wsgi_errors(environ, configuration) + _wrap_wsgi_errors(environ, configuration) for line in env_sync_status: _logger.debug(line) @@ -363,7 +370,7 @@ def application(environ, start_response): output_objs.append(wsgi_entry) _logger.debug("call format %r output to %s" % (backend, output_format)) - output = format_output(configuration, backend, ret_code, ret_msg, + output = _format_output(configuration, backend, ret_code, ret_msg, output_objs, output_format) # _logger.debug("formatted %s output to %s" % (backend, output_format)) # _logger.debug("output:\n%s" % [output]) @@ -396,7 +403,14 @@ def application(environ, start_response): # NOTE: send response to client but don't crash e.g. on closed connection try: start_response(status, response_headers) + except IOError as ioe: + _logger.warning("WSGI %s for %s could not deliver output: %s" % + (backend, client_id, ioe)) + except Exception as exc: + _logger.error("WSGI %s for %s crashed during response: %s" % + (backend, client_id, exc)) + try: # NOTE: we consistently hit download error for archive files reaching ~2GB # with showfreezefile.py on wsgi but the same on cgi does NOT suffer # the problem for the exact same files. It seems wsgi has a limited @@ -410,7 +424,7 @@ def application(environ, start_response): _logger.info("WSGI %s yielding %d output parts (%db)" % (backend, chunk_parts, content_length)) # _logger.debug("send chunked %r response to client" % backend) - for i in xrange(chunk_parts): + for i in list(range(chunk_parts)): # _logger.debug("WSGI %s yielding part %d / %d output parts" % # (backend, i+1, chunk_parts)) # end index may be after end of content - but no problem diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py new file mode 100644 index 000000000..cf75693cc --- /dev/null +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -0,0 +1,139 @@ +# -*- coding: utf-8 -*- +# +# --- BEGIN_HEADER --- +# +# Copyright (C) 2003-2024 The MiG Project by the Science HPC Center at UCPH +# +# This file is part of MiG. +# +# MiG is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# MiG is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, +# USA. +# +# --- END_HEADER --- +# + +"""Unit tests for the MiG WSGI glue""" + +from configparser import ConfigParser +import importlib +import os +import stat +import sys + +from tests.support import MIG_BASE, MigTestCase, testmain + + +def create_output_returner(arranged=None): + def test_format_output(*args): + return arranged + return test_format_output + +def create_wsgi_environ(config_file, env_http_host=None, env_path_info=None): + environ = {} + environ['MIG_CONF'] = config_file + environ['HTTP_HOST'] = env_http_host + environ['PATH_INFO'] = env_path_info + environ['SCRIPT_URI'] = ''.join(('http://', environ['HTTP_HOST'], environ['PATH_INFO'])) + return environ + + +def noop(*args): + pass + + +from tests.support import PY2, is_path_within +from mig.shared.base import client_id_dir, client_dir_id, get_short_id, \ + invisible_path, allow_script, brief_list + + +_LOCAL_MIG_BASE = '/usr/src/app' if PY2 else MIG_BASE # account for execution in container +_PYTHON_MAJOR = '2' if PY2 else '3' +_TEST_CONF_DIR = os.path.join(MIG_BASE, "envhelp/output/testconfs-py%s" % (_PYTHON_MAJOR,)) +_TEST_CONF_FILE = os.path.join(_TEST_CONF_DIR, "MiGserver.conf") +_TEST_CONF_SYMLINK = os.path.join(MIG_BASE, "envhelp/output/testconfs") + + +def _assert_local_config(): + try: + link_stat = os.lstat(_TEST_CONF_SYMLINK) + assert stat.S_ISLNK(link_stat.st_mode) + configdir_stat = os.stat(_TEST_CONF_DIR) + assert stat.S_ISDIR(configdir_stat.st_mode) + config = ConfigParser() + config.read([_TEST_CONF_FILE]) + return config + except Exception as exc: + raise AssertionError('local configuration invalid or missing: %s' % (str(exc),)) + + +def _assert_local_config_global_values(config): + config_global_values = dict(config.items('GLOBAL')) + + for path in ('mig_path', 'certs_path', 'state_path'): + path_value = config_global_values.get(path) + if not is_path_within(path_value, start=_LOCAL_MIG_BASE): + raise AssertionError('local config contains bad path: %s=%s' % (path, path_value)) + + return config_global_values + + +_WSGI_BIN = os.path.join(MIG_BASE, 'mig/wsgi-bin') + +def _import_migwsgi(): + sys.path.append(_WSGI_BIN) + migwsgi = importlib.import_module('migwsgi') + sys.path.pop(-1) + return migwsgi + + +migwsgi = _import_migwsgi() + + +class MigSharedConfiguration(MigTestCase): + def test_xxx(self): + config = _assert_local_config() + config_global_values = _assert_local_config_global_values(config) + + def fake_start_response(status, headers, exc=None): + fake_start_response.calls.append((status, headers, exc)) + fake_start_response.calls = [] + + def fake_set_environ(value): + fake_set_environ.value = value + fake_set_environ.value = None + + wsgi_environ = create_wsgi_environ( + _TEST_CONF_FILE, + env_http_host='localhost', + env_path_info='/' + ) + + test_output_returner = create_output_returner(b'') + + yielder = migwsgi._application(wsgi_environ, fake_start_response, + _format_output=test_output_returner, + _set_environ=fake_set_environ, + _wrap_wsgi_errors=noop, + _config_file=_TEST_CONF_FILE, + _skip_log=True, + ) + chunks = list(yielder) + + self.assertGreater(len(chunks), 0) + + + +if __name__ == '__main__': + testmain() From d9323754f78c941d3732ebfd4edb10b7e9656af1 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Thu, 1 Aug 2024 11:57:31 +0200 Subject: [PATCH 04/34] wip --- mig/wsgi-bin/migwsgi.py | 46 +++++++++++++++++++----------- tests/test_mig_wsgi-bin_migwsgi.py | 12 ++++++-- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index 34d132a93..fae32b5fa 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -26,6 +26,7 @@ # import cgi +import codecs import importlib import os import sys @@ -35,6 +36,7 @@ from mig.shared.bailout import bailout_helper, crash_helper, compact_string from mig.shared.base import requested_backend, allow_script, \ is_default_str_coding, force_default_str_coding_rec +from mig.shared.compat import PY2 from mig.shared.defaults import download_block_size, default_fs_coding from mig.shared.conf import get_configuration_object from mig.shared.objecttypes import get_object_type_info @@ -42,6 +44,18 @@ from mig.shared.safeinput import valid_backend_name, html_escape, InputException from mig.shared.scriptinput import fieldstorage_to_dict +if PY2: + pass +else: + def _ensure_wsgi_chunk(chunk): + return codecs.encode(chunk, 'utf8') + + +def _import_backend(backend): + import_path = 'mig.shared.functionality.%s' % backend + module_handle = importlib.import_module(import_path) + return module_handle.main + def object_type_info(object_type): """Lookup object type""" @@ -49,8 +63,7 @@ def object_type_info(object_type): return get_object_type_info(object_type) -def stub(configuration, client_id, import_path, backend, user_arguments_dict, - environ): +def stub(configuration, client_id, user_arguments_dict, environ, _retrieve_handler): """Run backend on behalf of client_id with supplied user_arguments_dict. I.e. import main from import_path and execute it with supplied arguments. """ @@ -61,6 +74,7 @@ def stub(configuration, client_id, import_path, backend, user_arguments_dict, before_time = time.time() output_objects = [] + backend = 'UNKNOWN' main = dummy_main # _logger.debug("stub for backend %r" % backend) @@ -69,6 +83,7 @@ def stub(configuration, client_id, import_path, backend, user_arguments_dict, # NEVER print/output it verbatim before it is validated below. try: + backend = requested_backend(environ, fallback=default_page) valid_backend_name(backend) except InputException as iex: _logger.error("%s refused to import invalid backend %r (%s): %s" % @@ -81,15 +96,14 @@ def stub(configuration, client_id, import_path, backend, user_arguments_dict, {'object_type': 'link', 'text': 'Go to default interface', 'destination': configuration.site_landing_page} ]) - return (output_objects, returnvalues.CLIENT_ERROR) + return backend, (output_objects, returnvalues.CLIENT_ERROR) try: # Import main from backend module # _logger.debug("import main from %r" % import_path) # NOTE: dynamic module loading to find corresponding main function - module_handle = importlib.import_module(import_path) - main = module_handle.main + main = _retrieve_handler(backend) except Exception as err: _logger.error("%s could not import %r (%s): %s" % (_addr, backend, import_path, err)) @@ -100,7 +114,7 @@ def stub(configuration, client_id, import_path, backend, user_arguments_dict, {'object_type': 'link', 'text': 'Go to default interface', 'destination': configuration.site_landing_page} ]) - return (output_objects, returnvalues.SYSTEM_ERROR) + return backend, (output_objects, returnvalues.SYSTEM_ERROR) # _logger.debug("imported main %s" % main) @@ -115,7 +129,7 @@ def stub(configuration, client_id, import_path, backend, user_arguments_dict, output_objects.append( {'object_type': 'error_text', 'text': 'User input is not on expected format!'}) - return (output_objects, returnvalues.INVALID_ARGUMENT) + return backend, (output_objects, returnvalues.INVALID_ARGUMENT) try: (output_objects, (ret_code, ret_msg)) = main(client_id, @@ -125,7 +139,7 @@ def stub(configuration, client_id, import_path, backend, user_arguments_dict, _logger.error("%s script crashed:\n%s" % (_addr, traceback.format_exc())) crash_helper(configuration, backend, output_objects) - return (output_objects, returnvalues.ERROR) + return backend, (output_objects, returnvalues.ERROR) (val_ret, val_msg) = validate(output_objects) if not val_ret: @@ -138,7 +152,7 @@ def stub(configuration, client_id, import_path, backend, user_arguments_dict, after_time = time.time() output_objects.append({'object_type': 'timing_info', 'text': "done in %.3fs" % (after_time - before_time)}) - return (output_objects, (ret_code, ret_msg)) + return backend, (output_objects, (ret_code, ret_msg)) def wrap_wsgi_errors(environ, configuration, max_line_len=100): @@ -198,7 +212,7 @@ def _set_os_environ(value): return _application(environ, start_response, _format_output=format_output, _set_environ=_set_os_environ, _wrap_wsgi_errors=wrap_wsgi_errors) -def _application(environ, start_response, _format_output, _set_environ, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False): +def _application(environ, start_response, _format_output, _set_environ, _retrieve_handler=_import_backend, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False): # NOTE: pass app environ including apache and query args on to sub handlers # through the usual 'os.environ' channel expected in functionality @@ -305,7 +319,6 @@ def _application(environ, start_response, _format_output, _set_environ, _wrap_ws default_page = configuration.site_landing_page script_name = requested_backend(environ, fallback=default_page, strip_ext=False) - backend = requested_backend(environ, fallback=default_page) # _logger.debug('DEBUG: wsgi found backend %s and script %s' % # (backend, script_name)) fieldstorage = cgi.FieldStorage(fp=environ['wsgi.input'], @@ -314,13 +327,12 @@ def _application(environ, start_response, _format_output, _set_environ, _wrap_ws if 'output_format' in user_arguments_dict: output_format = user_arguments_dict['output_format'][0] - module_path = 'mig.shared.functionality.%s' % backend (allow, msg) = allow_script(configuration, script_name, client_id) if allow: # _logger.debug("wsgi handling script: %s" % script_name) - (output_objs, ret_val) = stub(configuration, client_id, - module_path, backend, - user_arguments_dict, environ) + backend, (output_objs, ret_val) = stub(configuration, client_id, + user_arguments_dict, environ, + _retrieve_handler) else: _logger.warning("wsgi handling refused script:%s" % script_name) (output_objs, ret_val) = reject_main(client_id, @@ -370,7 +382,7 @@ def _application(environ, start_response, _format_output, _set_environ, _wrap_ws output_objs.append(wsgi_entry) _logger.debug("call format %r output to %s" % (backend, output_format)) - output = _format_output(configuration, backend, ret_code, ret_msg, + output = format_output(configuration, backend, ret_code, ret_msg, output_objs, output_format) # _logger.debug("formatted %s output to %s" % (backend, output_format)) # _logger.debug("output:\n%s" % [output]) @@ -429,7 +441,7 @@ def _application(environ, start_response, _format_output, _set_environ, _wrap_ws # (backend, i+1, chunk_parts)) # end index may be after end of content - but no problem part = output[i*download_block_size:(i+1)*download_block_size] - yield part + yield _ensure_wsgi_chunk(part) if chunk_parts > 1: _logger.info("WSGI %s finished yielding all %d output parts" % (backend, chunk_parts)) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index cf75693cc..ec21b1722 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -40,8 +40,9 @@ def test_format_output(*args): return arranged return test_format_output -def create_wsgi_environ(config_file, env_http_host=None, env_path_info=None): +def create_wsgi_environ(config_file, wsgi_input=None, env_http_host=None, env_path_info=None): environ = {} + environ['wsgi.input'] = () environ['MIG_CONF'] = config_file environ['HTTP_HOST'] = env_http_host environ['PATH_INFO'] = env_path_info @@ -106,6 +107,9 @@ def test_xxx(self): config = _assert_local_config() config_global_values = _assert_local_config_global_values(config) + def fake_handler(*args): + pass + def fake_start_response(status, headers, exc=None): fake_start_response.calls.append((status, headers, exc)) fake_start_response.calls = [] @@ -120,11 +124,12 @@ def fake_set_environ(value): env_path_info='/' ) - test_output_returner = create_output_returner(b'') + test_output_returner = create_output_returner('HELLO WORLD') yielder = migwsgi._application(wsgi_environ, fake_start_response, _format_output=test_output_returner, _set_environ=fake_set_environ, + _retrieve_handler=lambda _: fake_handler, _wrap_wsgi_errors=noop, _config_file=_TEST_CONF_FILE, _skip_log=True, @@ -132,7 +137,8 @@ def fake_set_environ(value): chunks = list(yielder) self.assertGreater(len(chunks), 0) - + import codecs + print(codecs.decode(chunks[0], 'utf8')) if __name__ == '__main__': From c718999986754e0a4c350487d41ddfeab79d8f7d Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Fri, 2 Aug 2024 09:32:15 +0200 Subject: [PATCH 05/34] fixup --- mig/wsgi-bin/migwsgi.py | 5 +++-- tests/test_mig_wsgi-bin_migwsgi.py | 5 ++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index fae32b5fa..fb656a6ec 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -83,6 +83,7 @@ def stub(configuration, client_id, user_arguments_dict, environ, _retrieve_handl # NEVER print/output it verbatim before it is validated below. try: + default_page = configuration.site_landing_page # TODO: avoid doing this work a second time backend = requested_backend(environ, fallback=default_page) valid_backend_name(backend) except InputException as iex: @@ -210,9 +211,9 @@ def application(environ, start_response): def _set_os_environ(value): os.environ = value - return _application(environ, start_response, _format_output=format_output, _set_environ=_set_os_environ, _wrap_wsgi_errors=wrap_wsgi_errors) + return _application(environ, start_response, _set_environ=_set_os_environ, _wrap_wsgi_errors=wrap_wsgi_errors) -def _application(environ, start_response, _format_output, _set_environ, _retrieve_handler=_import_backend, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False): +def _application(environ, start_response, _set_environ, _retrieve_handler=_import_backend, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False): # NOTE: pass app environ including apache and query args on to sub handlers # through the usual 'os.environ' channel expected in functionality diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index ec21b1722..57b1a40a3 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -33,7 +33,7 @@ import sys from tests.support import MIG_BASE, MigTestCase, testmain - +import mig.shared.returnvalues as returnvalues def create_output_returner(arranged=None): def test_format_output(*args): @@ -108,7 +108,7 @@ def test_xxx(self): config_global_values = _assert_local_config_global_values(config) def fake_handler(*args): - pass + return [], returnvalues.OK def fake_start_response(status, headers, exc=None): fake_start_response.calls.append((status, headers, exc)) @@ -127,7 +127,6 @@ def fake_set_environ(value): test_output_returner = create_output_returner('HELLO WORLD') yielder = migwsgi._application(wsgi_environ, fake_start_response, - _format_output=test_output_returner, _set_environ=fake_set_environ, _retrieve_handler=lambda _: fake_handler, _wrap_wsgi_errors=noop, From 1030c2ae64425df099fb5f06544ff9da14a71e2e Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Fri, 2 Aug 2024 11:21:45 +0200 Subject: [PATCH 06/34] fixup --- mig/wsgi-bin/migwsgi.py | 9 +-- tests/test_mig_wsgi-bin_migwsgi.py | 108 ++++++++++++++++++++++------- 2 files changed, 88 insertions(+), 29 deletions(-) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index fb656a6ec..6c3920139 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -45,7 +45,8 @@ from mig.shared.scriptinput import fieldstorage_to_dict if PY2: - pass + def _ensure_wsgi_chunk(chunk): + return chunk else: def _ensure_wsgi_chunk(chunk): return codecs.encode(chunk, 'utf8') @@ -213,7 +214,7 @@ def _set_os_environ(value): return _application(environ, start_response, _set_environ=_set_os_environ, _wrap_wsgi_errors=wrap_wsgi_errors) -def _application(environ, start_response, _set_environ, _retrieve_handler=_import_backend, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False): +def _application(environ, start_response, _set_environ, _format_output=format_output, _retrieve_handler=_import_backend, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False): # NOTE: pass app environ including apache and query args on to sub handlers # through the usual 'os.environ' channel expected in functionality @@ -383,7 +384,7 @@ def _application(environ, start_response, _set_environ, _retrieve_handler=_impor output_objs.append(wsgi_entry) _logger.debug("call format %r output to %s" % (backend, output_format)) - output = format_output(configuration, backend, ret_code, ret_msg, + output = _format_output(configuration, backend, ret_code, ret_msg, output_objs, output_format) # _logger.debug("formatted %s output to %s" % (backend, output_format)) # _logger.debug("output:\n%s" % [output]) @@ -392,7 +393,7 @@ def _application(environ, start_response, _set_environ, _retrieve_handler=_impor _logger.error( "Formatted output is NOT on default str coding: %s" % [output[:100]]) err_mark = '__****__' - output = format_output(configuration, backend, ret_code, ret_msg, + output = _format_output(configuration, backend, ret_code, ret_msg, force_default_str_coding_rec( output_objs, highlight=err_mark), output_format) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 57b1a40a3..c70fc814d 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -26,6 +26,7 @@ """Unit tests for the MiG WSGI glue""" +import codecs from configparser import ConfigParser import importlib import os @@ -33,26 +34,9 @@ import sys from tests.support import MIG_BASE, MigTestCase, testmain +from mig.shared.output import format_output import mig.shared.returnvalues as returnvalues -def create_output_returner(arranged=None): - def test_format_output(*args): - return arranged - return test_format_output - -def create_wsgi_environ(config_file, wsgi_input=None, env_http_host=None, env_path_info=None): - environ = {} - environ['wsgi.input'] = () - environ['MIG_CONF'] = config_file - environ['HTTP_HOST'] = env_http_host - environ['PATH_INFO'] = env_path_info - environ['SCRIPT_URI'] = ''.join(('http://', environ['HTTP_HOST'], environ['PATH_INFO'])) - return environ - - -def noop(*args): - pass - from tests.support import PY2, is_path_within from mig.shared.base import client_id_dir, client_dir_id, get_short_id, \ @@ -90,19 +74,92 @@ def _assert_local_config_global_values(config): return config_global_values -_WSGI_BIN = os.path.join(MIG_BASE, 'mig/wsgi-bin') - def _import_migwsgi(): - sys.path.append(_WSGI_BIN) + sys.path.append(os.path.join(MIG_BASE, 'mig/wsgi-bin')) migwsgi = importlib.import_module('migwsgi') sys.path.pop(-1) return migwsgi +migwsgi = _import_migwsgi() -migwsgi = _import_migwsgi() +def create_instrumented_format_output(arranged): + def instrumented_format_output( + configuration, + backend, + ret_val, + ret_msg, + out_obj, + outputformat, + ): + # record the call args + call_args_out_obj = list(out_obj) # capture the original before altering it + call_args = (configuration, backend, ret_val, ret_msg, call_args_out_obj, outputformat,) + instrumented_format_output.calls.append({ 'args': call_args }) + + + # FIXME: the following is a workaround for a bug that exists between the WSGI wrapper + # and the output formatter - specifically, the latter adds default header and + # title if start does not exist, but the former ensures that start always exists + # meaning that a default response under WSGI is missing half the HTML. + start_obj_idx = next((i for i, obj in enumerate(out_obj) if obj['object_type'] == 'start')) + insertion_idx = start_obj_idx + + insertion_idx += 1 + out_obj.insert(insertion_idx, { + 'object_type': 'title', + 'text': arranged, + 'meta': '', + 'style': {}, + 'script': {}, + }) + + # FIXME: format_output() will write the header _before_ the preamble unless there some + # other non-special output object prior to it. + # insertion_idx += 1 + # out_obj.insert(insertion_idx, { + # 'object_type': '__FORCEPREAMBLE__', + # }) + + insertion_idx += 1 + out_obj.insert(insertion_idx, { + 'object_type': 'header', + 'text': arranged + }) + + return format_output( + configuration, + backend, + ret_val, + ret_msg, + out_obj, + outputformat, + ) + instrumented_format_output.calls = [] + return instrumented_format_output + + +def create_wsgi_environ(config_file, wsgi_input=None, env_http_host=None, env_path_info=None): + environ = {} + environ['wsgi.input'] = () + environ['MIG_CONF'] = config_file + environ['HTTP_HOST'] = env_http_host + environ['PATH_INFO'] = env_path_info + environ['SCRIPT_URI'] = ''.join(('http://', environ['HTTP_HOST'], environ['PATH_INFO'])) + return environ + + +def noop(*args): + pass class MigSharedConfiguration(MigTestCase): + def assertHtmlBasics(self, value): + assert isinstance(value, type(u"")) + assert value.startswith("') + maybe_document_end = value[end_html_tag_idx:].rstrip() + self.assertEqual(maybe_document_end, '') + def test_xxx(self): config = _assert_local_config() config_global_values = _assert_local_config_global_values(config) @@ -124,9 +181,10 @@ def fake_set_environ(value): env_path_info='/' ) - test_output_returner = create_output_returner('HELLO WORLD') + instrumented_format_output = create_instrumented_format_output('HELLO WORLD') yielder = migwsgi._application(wsgi_environ, fake_start_response, + _format_output=instrumented_format_output, _set_environ=fake_set_environ, _retrieve_handler=lambda _: fake_handler, _wrap_wsgi_errors=noop, @@ -136,8 +194,8 @@ def fake_set_environ(value): chunks = list(yielder) self.assertGreater(len(chunks), 0) - import codecs - print(codecs.decode(chunks[0], 'utf8')) + value = codecs.decode(chunks[0], 'utf8') + self.assertHtmlBasics(value) if __name__ == '__main__': From 30520dc97ab6b20ceab52312eea3f6f0587feb07 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Mon, 5 Aug 2024 11:17:46 +0200 Subject: [PATCH 07/34] updare and relocate a comment --- tests/test_mig_wsgi-bin_migwsgi.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index c70fc814d..27d7e79ad 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -104,6 +104,9 @@ def instrumented_format_output( start_obj_idx = next((i for i, obj in enumerate(out_obj) if obj['object_type'] == 'start')) insertion_idx = start_obj_idx + # FIXME: format_output() is sensitive to ordering and MUST see a title object _before_ + # anything else otherwise the preamble ends up written above the header and thus + # an invalid HTML page is served. insertion_idx += 1 out_obj.insert(insertion_idx, { 'object_type': 'title', @@ -113,13 +116,6 @@ def instrumented_format_output( 'script': {}, }) - # FIXME: format_output() will write the header _before_ the preamble unless there some - # other non-special output object prior to it. - # insertion_idx += 1 - # out_obj.insert(insertion_idx, { - # 'object_type': '__FORCEPREAMBLE__', - # }) - insertion_idx += 1 out_obj.insert(insertion_idx, { 'object_type': 'header', From 614062125bfbd5d8e178f80f5435af968b79259f Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Mon, 5 Aug 2024 11:18:37 +0200 Subject: [PATCH 08/34] start tightening up the code --- tests/test_mig_wsgi-bin_migwsgi.py | 47 +++++++++++++++++++----------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 27d7e79ad..776405fff 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -82,8 +82,25 @@ def _import_migwsgi(): migwsgi = _import_migwsgi() +def _is_return_value(return_value): + defined_return_values = returnvalues.__dict__.values() + return return_value in defined_return_values + + +def create_instrumented_retrieve_handler(output_objects=None, return_value=None): + if not output_objects: + output_objects = [] + + assert isinstance(output_objects, list) + assert _is_return_value(return_value), "return value must be present in returnvalues" + + def _instrumented_retrieve_handler(*args): + return [], return_value + return _instrumented_retrieve_handler + + def create_instrumented_format_output(arranged): - def instrumented_format_output( + def _instrumented_format_output( configuration, backend, ret_val, @@ -94,8 +111,7 @@ def instrumented_format_output( # record the call args call_args_out_obj = list(out_obj) # capture the original before altering it call_args = (configuration, backend, ret_val, ret_msg, call_args_out_obj, outputformat,) - instrumented_format_output.calls.append({ 'args': call_args }) - + _instrumented_format_output.calls.append({ 'args': call_args }) # FIXME: the following is a workaround for a bug that exists between the WSGI wrapper # and the output formatter - specifically, the latter adds default header and @@ -130,16 +146,16 @@ def instrumented_format_output( out_obj, outputformat, ) - instrumented_format_output.calls = [] - return instrumented_format_output + _instrumented_format_output.calls = [] + return _instrumented_format_output -def create_wsgi_environ(config_file, wsgi_input=None, env_http_host=None, env_path_info=None): +def create_wsgi_environ(config_file, wsgi_variables): environ = {} environ['wsgi.input'] = () environ['MIG_CONF'] = config_file - environ['HTTP_HOST'] = env_http_host - environ['PATH_INFO'] = env_path_info + environ['HTTP_HOST'] = wsgi_variables.get('http_host') + environ['PATH_INFO'] = wsgi_variables.get('path_info') environ['SCRIPT_URI'] = ''.join(('http://', environ['HTTP_HOST'], environ['PATH_INFO'])) return environ @@ -160,9 +176,6 @@ def test_xxx(self): config = _assert_local_config() config_global_values = _assert_local_config_global_values(config) - def fake_handler(*args): - return [], returnvalues.OK - def fake_start_response(status, headers, exc=None): fake_start_response.calls.append((status, headers, exc)) fake_start_response.calls = [] @@ -171,18 +184,18 @@ def fake_set_environ(value): fake_set_environ.value = value fake_set_environ.value = None - wsgi_environ = create_wsgi_environ( - _TEST_CONF_FILE, - env_http_host='localhost', - env_path_info='/' - ) + wsgi_environ = create_wsgi_environ(_TEST_CONF_FILE, wsgi_variables=dict( + http_host='localhost', + path_info='/', + )) instrumented_format_output = create_instrumented_format_output('HELLO WORLD') + instrumented_retrieve_handler = create_instrumented_retrieve_handler(None, returnvalues.OK) yielder = migwsgi._application(wsgi_environ, fake_start_response, _format_output=instrumented_format_output, + _retrieve_handler=instrumented_retrieve_handler, _set_environ=fake_set_environ, - _retrieve_handler=lambda _: fake_handler, _wrap_wsgi_errors=noop, _config_file=_TEST_CONF_FILE, _skip_log=True, From b6eb4c38ac153dcb9cfd457fa2f03dff867e5c28 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Mon, 5 Aug 2024 11:20:16 +0200 Subject: [PATCH 09/34] shift things around a little --- tests/test_mig_wsgi-bin_migwsgi.py | 41 +++++++++++++++--------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 776405fff..02290c315 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -50,6 +50,15 @@ _TEST_CONF_SYMLINK = os.path.join(MIG_BASE, "envhelp/output/testconfs") +# workaround for migwsgi being placed witin a non-module directory +def _import_migwsgi(): + sys.path.append(os.path.join(MIG_BASE, 'mig/wsgi-bin')) + migwsgi = importlib.import_module('migwsgi') + sys.path.pop(-1) + return migwsgi +migwsgi = _import_migwsgi() + + def _assert_local_config(): try: link_stat = os.lstat(_TEST_CONF_SYMLINK) @@ -74,31 +83,11 @@ def _assert_local_config_global_values(config): return config_global_values -def _import_migwsgi(): - sys.path.append(os.path.join(MIG_BASE, 'mig/wsgi-bin')) - migwsgi = importlib.import_module('migwsgi') - sys.path.pop(-1) - return migwsgi -migwsgi = _import_migwsgi() - - def _is_return_value(return_value): defined_return_values = returnvalues.__dict__.values() return return_value in defined_return_values -def create_instrumented_retrieve_handler(output_objects=None, return_value=None): - if not output_objects: - output_objects = [] - - assert isinstance(output_objects, list) - assert _is_return_value(return_value), "return value must be present in returnvalues" - - def _instrumented_retrieve_handler(*args): - return [], return_value - return _instrumented_retrieve_handler - - def create_instrumented_format_output(arranged): def _instrumented_format_output( configuration, @@ -150,6 +139,18 @@ def _instrumented_format_output( return _instrumented_format_output +def create_instrumented_retrieve_handler(output_objects=None, return_value=None): + if not output_objects: + output_objects = [] + + assert isinstance(output_objects, list) + assert _is_return_value(return_value), "return value must be present in returnvalues" + + def _instrumented_retrieve_handler(*args): + return [], return_value + return _instrumented_retrieve_handler + + def create_wsgi_environ(config_file, wsgi_variables): environ = {} environ['wsgi.input'] = () From 55ed288820047e40e5723901fa032871a2c0541a Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Mon, 5 Aug 2024 11:54:49 +0200 Subject: [PATCH 10/34] work to make it readable with a nod towards further tests --- tests/test_mig_wsgi-bin_migwsgi.py | 56 ++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 02290c315..71c1f4e68 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -88,6 +88,14 @@ def _is_return_value(return_value): return return_value in defined_return_values +def _unpack_result(application_result): + chunks = list(application_result) + assert len(chunks) > 0, "invocation returned no output" + complete_value = b''.join(chunks) + decoded_value = codecs.decode(complete_value, 'utf8') + return decoded_value + + def create_instrumented_format_output(arranged): def _instrumented_format_output( configuration, @@ -147,7 +155,10 @@ def create_instrumented_retrieve_handler(output_objects=None, return_value=None) assert _is_return_value(return_value), "return value must be present in returnvalues" def _instrumented_retrieve_handler(*args): + _instrumented_retrieve_handler.calls.append(tuple(args)) return [], return_value + _instrumented_retrieve_handler.calls = [] + return _instrumented_retrieve_handler @@ -165,15 +176,23 @@ def noop(*args): pass -class MigSharedConfiguration(MigTestCase): - def assertHtmlBasics(self, value): +class MigWsgi_binMigwsgi(MigTestCase): + def assertInstrumentation(self): + def was_called(fake): + assert hasattr(fake, 'calls') + return len(fake.calls) > 0 + + self.assertTrue(was_called(self.instrumented_format_output)) + self.assertTrue(was_called(self.instrumented_retrieve_handler)) + + def assertIsValidHtmlDocument(self, value): assert isinstance(value, type(u"")) assert value.startswith("') maybe_document_end = value[end_html_tag_idx:].rstrip() self.assertEqual(maybe_document_end, '') - def test_xxx(self): + def before_each(self): config = _assert_local_config() config_global_values = _assert_local_config_global_values(config) @@ -182,30 +201,37 @@ def fake_start_response(status, headers, exc=None): fake_start_response.calls = [] def fake_set_environ(value): - fake_set_environ.value = value - fake_set_environ.value = None + fake_set_environ.calls.append((value)) + fake_set_environ.calls = [] - wsgi_environ = create_wsgi_environ(_TEST_CONF_FILE, wsgi_variables=dict( + fake_wsgi_environ = create_wsgi_environ(_TEST_CONF_FILE, wsgi_variables=dict( http_host='localhost', path_info='/', )) - instrumented_format_output = create_instrumented_format_output('HELLO WORLD') - instrumented_retrieve_handler = create_instrumented_retrieve_handler(None, returnvalues.OK) + self.instrumented_format_output = create_instrumented_format_output('HELLO WORLD') + self.instrumented_retrieve_handler = create_instrumented_retrieve_handler(None, returnvalues.OK) - yielder = migwsgi._application(wsgi_environ, fake_start_response, - _format_output=instrumented_format_output, - _retrieve_handler=instrumented_retrieve_handler, + self.application_args = (fake_wsgi_environ, fake_start_response,) + self.application_kwargs = dict( + _format_output=self.instrumented_format_output, + _retrieve_handler=self.instrumented_retrieve_handler, _set_environ=fake_set_environ, + ) + + def test_creates_valid_html_page_for_return_value_ok(self): + application_result = migwsgi._application( + *self.application_args, _wrap_wsgi_errors=noop, _config_file=_TEST_CONF_FILE, _skip_log=True, + **self.application_kwargs ) - chunks = list(yielder) - self.assertGreater(len(chunks), 0) - value = codecs.decode(chunks[0], 'utf8') - self.assertHtmlBasics(value) + output = _unpack_result(application_result) + + self.assertInstrumentation() + self.assertIsValidHtmlDocument(output) if __name__ == '__main__': From c4ffa9f55a4c51745c4a9be484aaad7dfc0c1ba9 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Mon, 5 Aug 2024 14:19:48 +0200 Subject: [PATCH 11/34] assert the response status --- tests/test_mig_wsgi-bin_migwsgi.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 71c1f4e68..65b517704 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -185,6 +185,17 @@ def was_called(fake): self.assertTrue(was_called(self.instrumented_format_output)) self.assertTrue(was_called(self.instrumented_retrieve_handler)) + def assertResponseStatus(self, expected_status_code): + def called_once(fake): + assert hasattr(fake, 'calls') + return len(fake.calls) == 1 + + self.assertTrue(called_once(self.fake_start_response)) + thecall = self.fake_start_response.calls[0] + wsgi_status = thecall[0] + actual_status_code = int(wsgi_status[0:3]) + self.assertEqual(actual_status_code, expected_status_code) + def assertIsValidHtmlDocument(self, value): assert isinstance(value, type(u"")) assert value.startswith(" Date: Mon, 5 Aug 2024 15:25:26 +0200 Subject: [PATCH 12/34] allow programming the response --- tests/test_mig_wsgi-bin_migwsgi.py | 34 ++++++++++++++++++------------ 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 65b517704..0c4009337 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -88,7 +88,7 @@ def _is_return_value(return_value): return return_value in defined_return_values -def _unpack_result(application_result): +def _trigger_and_unpack_result(application_result): chunks = list(application_result) assert len(chunks) > 0, "invocation returned no output" complete_value = b''.join(chunks) @@ -147,17 +147,17 @@ def _instrumented_format_output( return _instrumented_format_output -def create_instrumented_retrieve_handler(output_objects=None, return_value=None): - if not output_objects: - output_objects = [] - - assert isinstance(output_objects, list) - assert _is_return_value(return_value), "return value must be present in returnvalues" - +def create_instrumented_retrieve_handler(): def _instrumented_retrieve_handler(*args): _instrumented_retrieve_handler.calls.append(tuple(args)) - return [], return_value + return _instrumented_retrieve_handler.returning or ([], returnvalues.ERROR) + def _program(output_objects=None, return_value=None): + assert _is_return_value(return_value), "return value must be present in returnvalues" + assert isinstance(output_objects, list) + _instrumented_retrieve_handler.returning = (output_objects, return_value) _instrumented_retrieve_handler.calls = [] + _instrumented_retrieve_handler.returning = None + _instrumented_retrieve_handler.program = _program return _instrumented_retrieve_handler @@ -178,12 +178,14 @@ def noop(*args): class MigWsgi_binMigwsgi(MigTestCase): def assertInstrumentation(self): + self.assertIsNotNone(self.instrumented_retrieve_handler.returning, "no response programmed") + def was_called(fake): assert hasattr(fake, 'calls') return len(fake.calls) > 0 - self.assertTrue(was_called(self.instrumented_format_output)) - self.assertTrue(was_called(self.instrumented_retrieve_handler)) + self.assertTrue(was_called(self.instrumented_format_output), "no output generated") + self.assertTrue(was_called(self.instrumented_retrieve_handler), "no output generated") def assertResponseStatus(self, expected_status_code): def called_once(fake): @@ -223,7 +225,7 @@ def fake_set_environ(value): )) self.instrumented_format_output = create_instrumented_format_output('HELLO WORLD') - self.instrumented_retrieve_handler = create_instrumented_retrieve_handler(None, returnvalues.OK) + self.instrumented_retrieve_handler = create_instrumented_retrieve_handler() self.application_args = (fake_wsgi_environ, fake_start_response,) self.application_kwargs = dict( @@ -233,6 +235,8 @@ def fake_set_environ(value): ) def test_return_value_ok_returns_status_200(self): + self.instrumented_retrieve_handler.program([], returnvalues.OK) + application_result = migwsgi._application( *self.application_args, _wrap_wsgi_errors=noop, @@ -241,12 +245,14 @@ def test_return_value_ok_returns_status_200(self): **self.application_kwargs ) - output = _unpack_result(application_result) + _trigger_and_unpack_result(application_result) self.assertInstrumentation() self.assertResponseStatus(200) def test_return_value_ok_creates_valid_html_page(self): + self.instrumented_retrieve_handler.program([], returnvalues.OK) + application_result = migwsgi._application( *self.application_args, _wrap_wsgi_errors=noop, @@ -255,7 +261,7 @@ def test_return_value_ok_creates_valid_html_page(self): **self.application_kwargs ) - output = _unpack_result(application_result) + output = _trigger_and_unpack_result(application_result) self.assertInstrumentation() self.assertIsValidHtmlDocument(output) From b20490ff5273d25b78ba1967c852e09cf364845f Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Mon, 5 Aug 2024 15:42:27 +0200 Subject: [PATCH 13/34] repair previous --- tests/test_mig_wsgi-bin_migwsgi.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 0c4009337..8342eeef9 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -148,16 +148,23 @@ def _instrumented_format_output( def create_instrumented_retrieve_handler(): - def _instrumented_retrieve_handler(*args): - _instrumented_retrieve_handler.calls.append(tuple(args)) - return _instrumented_retrieve_handler.returning or ([], returnvalues.ERROR) - def _program(output_objects=None, return_value=None): + def _simulated_action(*args): + return _simulated_action.returning or ([], returnvalues.ERROR) + _simulated_action.calls = [] + _simulated_action.returning = None + + def _program_response(output_objects=None, return_value=None): assert _is_return_value(return_value), "return value must be present in returnvalues" assert isinstance(output_objects, list) - _instrumented_retrieve_handler.returning = (output_objects, return_value) + _simulated_action.returning = (output_objects, return_value) + + def _instrumented_retrieve_handler(*args): + _instrumented_retrieve_handler.calls.append(tuple(args)) + return _simulated_action _instrumented_retrieve_handler.calls = [] - _instrumented_retrieve_handler.returning = None - _instrumented_retrieve_handler.program = _program + + _instrumented_retrieve_handler.program = _program_response + _instrumented_retrieve_handler.simulated = _simulated_action return _instrumented_retrieve_handler @@ -178,7 +185,8 @@ def noop(*args): class MigWsgi_binMigwsgi(MigTestCase): def assertInstrumentation(self): - self.assertIsNotNone(self.instrumented_retrieve_handler.returning, "no response programmed") + simulated_action = self.instrumented_retrieve_handler.simulated + self.assertIsNotNone(simulated_action.returning, "no response programmed") def was_called(fake): assert hasattr(fake, 'calls') From 8ce5e03d91b6b0d0d09b5e0a0ba936514bb4b902 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Mon, 5 Aug 2024 16:33:03 +0200 Subject: [PATCH 14/34] assert that a programmed title ends up in the page --- tests/test_mig_wsgi-bin_migwsgi.py | 49 +++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 8342eeef9..628666bd9 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -96,7 +96,7 @@ def _trigger_and_unpack_result(application_result): return decoded_value -def create_instrumented_format_output(arranged): +def create_instrumented_format_output(): def _instrumented_format_output( configuration, backend, @@ -123,7 +123,7 @@ def _instrumented_format_output( insertion_idx += 1 out_obj.insert(insertion_idx, { 'object_type': 'title', - 'text': arranged, + 'text': _instrumented_format_output.values['title_text'], 'meta': '', 'style': {}, 'script': {}, @@ -132,7 +132,7 @@ def _instrumented_format_output( insertion_idx += 1 out_obj.insert(insertion_idx, { 'object_type': 'header', - 'text': arranged + 'text': _instrumented_format_output.values['header_text'] }) return format_output( @@ -144,6 +144,16 @@ def _instrumented_format_output( outputformat, ) _instrumented_format_output.calls = [] + _instrumented_format_output.values = dict( + title_text='', + header_text='', + ) + + def _program_values(**kwargs): + _instrumented_format_output.values.update(kwargs) + + _instrumented_format_output.set_values = _program_values + return _instrumented_format_output @@ -206,6 +216,18 @@ def called_once(fake): actual_status_code = int(wsgi_status[0:3]) self.assertEqual(actual_status_code, expected_status_code) + def assertHtmlElementTextContent(self, output, tag_name, expected_text, trim_newlines=True): + # TODO: this is a definitively stop-gap way of finding a tag within the HTML + # and is used purely to keep this initial change to a reasonable size. + tag_open = ''.join(['<', tag_name, '>']) + tag_open_index = output.index(tag_open) + tag_close = ''.join(['']) + tag_close_index = output.index(tag_close) + actual_text = output[tag_open_index+len(tag_open):tag_close_index] + if trim_newlines: + actual_text = actual_text.strip('\n') + self.assertEqual(actual_text, expected_text) + def assertIsValidHtmlDocument(self, value): assert isinstance(value, type(u"")) assert value.startswith(" Date: Wed, 14 Aug 2024 15:54:19 +0200 Subject: [PATCH 15/34] line naming up with other recent work in grid_openid --- mig/wsgi-bin/migwsgi.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index 6c3920139..f29a0faf0 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -44,11 +44,12 @@ from mig.shared.safeinput import valid_backend_name, html_escape, InputException from mig.shared.scriptinput import fieldstorage_to_dict + if PY2: - def _ensure_wsgi_chunk(chunk): + def _ensure_encoded_string(chunk): return chunk else: - def _ensure_wsgi_chunk(chunk): + def _ensure_encoded_string(chunk): return codecs.encode(chunk, 'utf8') @@ -443,7 +444,7 @@ def _application(environ, start_response, _set_environ, _format_output=format_ou # (backend, i+1, chunk_parts)) # end index may be after end of content - but no problem part = output[i*download_block_size:(i+1)*download_block_size] - yield _ensure_wsgi_chunk(part) + yield _ensure_encoded_string(part) if chunk_parts > 1: _logger.info("WSGI %s finished yielding all %d output parts" % (backend, chunk_parts)) From b4933a037f10812dd4dbaf6f6436135ee1f6a414 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Fri, 16 Aug 2024 13:07:20 +0200 Subject: [PATCH 16/34] fixup --- mig/shared/base.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/mig/shared/base.py b/mig/shared/base.py index 64f12b370..316c1cb11 100644 --- a/mig/shared/base.py +++ b/mig/shared/base.py @@ -516,8 +516,9 @@ def force_utf8_rec(input_obj, highlight=''): if isinstance(input_obj, dict): return {force_utf8_rec(i, highlight): force_utf8_rec(j, highlight) for (i, j) in input_obj.items()} - elif isinstance(input_obj, list): - return [force_utf8_rec(i, highlight) for i in input_obj] + elif isinstance(input_obj, (list, tuple)): + thetype = type(input_obj) + return thetype(force_utf8_rec(i, highlight) for i in input_obj) elif is_unicode(input_obj): return force_utf8(input_obj, highlight) else: @@ -544,8 +545,9 @@ def force_unicode_rec(input_obj, highlight=''): if isinstance(input_obj, dict): return {force_unicode_rec(i, highlight): force_unicode_rec(j, highlight) for (i, j) in input_obj.items()} - elif isinstance(input_obj, list): - return [force_unicode_rec(i, highlight) for i in input_obj] + elif isinstance(input_obj, (list, tuple)): + thetype = type(input_obj) + return thetype(force_utf8_rec(i, highlight) for i in input_obj) elif not is_unicode(input_obj): return force_unicode(input_obj, highlight) else: From 0ba4f50d34ed45e76d2bcf5af97f0e7ea4888c0d Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Mon, 23 Sep 2024 15:19:16 +0200 Subject: [PATCH 17/34] fixup --- mig/wsgi-bin/migwsgi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index f29a0faf0..cdd30c01f 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -89,8 +89,8 @@ def stub(configuration, client_id, user_arguments_dict, environ, _retrieve_handl backend = requested_backend(environ, fallback=default_page) valid_backend_name(backend) except InputException as iex: - _logger.error("%s refused to import invalid backend %r (%s): %s" % - (_addr, backend, import_path, iex)) + _logger.error("%s refused to import invalid backend %r: %s" % + (_addr, backend, iex)) bailout_helper(configuration, backend, output_objects, header_text='User Error') output_objects.extend([ From 087208318875e1d3902f3ae8ef1e3e541d472908 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Mon, 23 Sep 2024 20:46:49 +0200 Subject: [PATCH 18/34] fixup --- mig/wsgi-bin/migwsgi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index cdd30c01f..5314a710a 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -108,8 +108,8 @@ def stub(configuration, client_id, user_arguments_dict, environ, _retrieve_handl # NOTE: dynamic module loading to find corresponding main function main = _retrieve_handler(backend) except Exception as err: - _logger.error("%s could not import %r (%s): %s" % - (_addr, backend, import_path, err)) + _logger.error("%s could not import %r: %s" % + (_addr, backend, err)) bailout_helper(configuration, backend, output_objects) output_objects.extend([ {'object_type': 'error_text', 'text': From b1e38401c10fca90d0b668534ad25e7b1da675c7 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Mon, 23 Sep 2024 20:51:51 +0200 Subject: [PATCH 19/34] fixup --- mig/wsgi-bin/migwsgi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index 5314a710a..f5b2f3788 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -124,9 +124,9 @@ def stub(configuration, client_id, user_arguments_dict, environ, _retrieve_handl # Now backend value is validated to be safe for output if not isinstance(user_arguments_dict, dict): - _logger.error("%s invalid user args %s for %s" % (_addr, + _logger.error("%s invalid user args %s for backend %r" % (_addr, user_arguments_dict, - import_path)) + backend)) bailout_helper(configuration, backend, output_objects, header_text='Input Error') output_objects.append( From 366a42c46ca50abb2821c5f7462908d3ceab73f8 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Tue, 24 Sep 2024 15:44:04 +0200 Subject: [PATCH 20/34] split the testing infrastructure across multiple files --- tests/support/__init__.py | 3 ++ tests/support/htmlsupp.py | 71 ++++++++++++++++++++++++++++++ tests/support/wsgisupp.py | 63 ++++++++++++++++++++++++++ tests/test_mig_wsgi-bin_migwsgi.py | 54 +++-------------------- 4 files changed, 143 insertions(+), 48 deletions(-) create mode 100644 tests/support/htmlsupp.py create mode 100644 tests/support/wsgisupp.py diff --git a/tests/support/__init__.py b/tests/support/__init__.py index b271645e8..5e4cdf221 100644 --- a/tests/support/__init__.py +++ b/tests/support/__init__.py @@ -82,8 +82,11 @@ from tests.support.assertover import AssertOver from tests.support.configsupp import FakeConfiguration +from tests.support.htmlsupp import HtmlAssertMixin from tests.support.loggersupp import FakeLogger from tests.support.serversupp import make_wrapped_server +from tests.support.wsgisupp import create_wsgi_environ, \ + ServerAssertMixin, FakeStartResponse # Basic global logging configuration for testing diff --git a/tests/support/htmlsupp.py b/tests/support/htmlsupp.py new file mode 100644 index 000000000..8f6fc63c2 --- /dev/null +++ b/tests/support/htmlsupp.py @@ -0,0 +1,71 @@ +#!/usr/bin/python +# -*- coding: utf-8 -*- +# +# --- BEGIN_HEADER --- +# +# htmlsupp - test support library for HTML +# Copyright (C) 2003-2024 The MiG Project by the Science HPC Center at UCPH +# +# This file is part of MiG. +# +# MiG is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# MiG is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# -- END_HEADER --- +# + +"""Test support library for HTML.""" + + +class HtmlAssertMixin: + """Custom assertions for HTML containing strings.""" + + def assertHtmlElementTextContent(self, value, tag_name, expected_text, trim_newlines=True): + """Locate the first occurrence of a tag within an HTML input string + and assert that any contained string equals what was supplied. + """ + + self.assertIsValidHtmlDocument(value) + + # TODO: this is a definitively stop-gap way of finding a tag within the HTML + # and is used purely to keep this initial change to a reasonable size. + + tag_open = ''.join(['<', tag_name, '>']) + tag_open_index = value.index(tag_open) + tag_open_index_after = tag_open_index + len(tag_open) + + tag_close = ''.join(['']) + tag_close_index = value.index(tag_close, tag_open_index_after) + + actual_text = value[tag_open_index_after:tag_close_index] + if trim_newlines: + actual_text = actual_text.strip('\n') + self.assertEqual(actual_text, expected_text) + + def assertIsValidHtmlDocument(self, value): + """Check that the input string contains a valid HTML document. + """ + + assert isinstance(value, type(u"")) + + error = None + try: + assert value.startswith("') + maybe_document_end = value[end_html_tag_idx:].rstrip() + self.assertEqual(maybe_document_end, '') + except Exception as exc: + error = exc + if error: + raise AssertionError("failed to verify input string as HTML: %s", str(exc)) diff --git a/tests/support/wsgisupp.py b/tests/support/wsgisupp.py new file mode 100644 index 000000000..6cf177a27 --- /dev/null +++ b/tests/support/wsgisupp.py @@ -0,0 +1,63 @@ +#!/usr/bin/python +# -*- coding: utf-8 -*- +# +# --- BEGIN_HEADER --- +# +# htmlsupp - test support library for WSGI +# Copyright (C) 2003-2024 The MiG Project by the Science HPC Center at UCPH +# +# This file is part of MiG. +# +# MiG is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# MiG is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# -- END_HEADER --- +# + +"""Test support library for WSGI.""" + + +def create_wsgi_environ(config_file, wsgi_variables): + environ = {} + environ['wsgi.input'] = () + environ['MIG_CONF'] = config_file + environ['HTTP_HOST'] = wsgi_variables.get('http_host') + environ['PATH_INFO'] = wsgi_variables.get('path_info') + environ['SCRIPT_URI'] = ''.join(('http://', environ['HTTP_HOST'], environ['PATH_INFO'])) + return environ + + +class FakeStartResponse: + def __init__(self): + self.calls = [] + + def __call__(self, status, headers, exc=None): + self.calls.append((status, headers, exc)) + + +class ServerAssertMixin: + """Custom assertions for verifying server code executed under test.""" + + def assertWsgiResponseStatus(self, fake_start_response, expected_status_code): + assert isinstance(fake_start_response, FakeStartResponse) + + def called_once(fake): + assert hasattr(fake, 'calls') + return len(fake.calls) == 1 + + self.assertTrue(called_once(fake_start_response)) + thecall = fake_start_response.calls[0] + wsgi_status = thecall[0] + actual_status_code = int(wsgi_status[0:3]) + self.assertEqual(actual_status_code, expected_status_code) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 628666bd9..f1e6e7a37 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -38,7 +38,8 @@ import mig.shared.returnvalues as returnvalues -from tests.support import PY2, is_path_within +from tests.support import PY2, is_path_within, \ + create_wsgi_environ, ServerAssertMixin, FakeStartResponse, HtmlAssertMixin from mig.shared.base import client_id_dir, client_dir_id, get_short_id, \ invisible_path, allow_script, brief_list @@ -179,21 +180,11 @@ def _instrumented_retrieve_handler(*args): return _instrumented_retrieve_handler -def create_wsgi_environ(config_file, wsgi_variables): - environ = {} - environ['wsgi.input'] = () - environ['MIG_CONF'] = config_file - environ['HTTP_HOST'] = wsgi_variables.get('http_host') - environ['PATH_INFO'] = wsgi_variables.get('path_info') - environ['SCRIPT_URI'] = ''.join(('http://', environ['HTTP_HOST'], environ['PATH_INFO'])) - return environ - - def noop(*args): pass -class MigWsgi_binMigwsgi(MigTestCase): +class MigWsgi_binMigwsgi(MigTestCase, ServerAssertMixin, HtmlAssertMixin): def assertInstrumentation(self): simulated_action = self.instrumented_retrieve_handler.simulated self.assertIsNotNone(simulated_action.returning, "no response programmed") @@ -205,44 +196,11 @@ def was_called(fake): self.assertTrue(was_called(self.instrumented_format_output), "no output generated") self.assertTrue(was_called(self.instrumented_retrieve_handler), "no output generated") - def assertResponseStatus(self, expected_status_code): - def called_once(fake): - assert hasattr(fake, 'calls') - return len(fake.calls) == 1 - - self.assertTrue(called_once(self.fake_start_response)) - thecall = self.fake_start_response.calls[0] - wsgi_status = thecall[0] - actual_status_code = int(wsgi_status[0:3]) - self.assertEqual(actual_status_code, expected_status_code) - - def assertHtmlElementTextContent(self, output, tag_name, expected_text, trim_newlines=True): - # TODO: this is a definitively stop-gap way of finding a tag within the HTML - # and is used purely to keep this initial change to a reasonable size. - tag_open = ''.join(['<', tag_name, '>']) - tag_open_index = output.index(tag_open) - tag_close = ''.join(['']) - tag_close_index = output.index(tag_close) - actual_text = output[tag_open_index+len(tag_open):tag_close_index] - if trim_newlines: - actual_text = actual_text.strip('\n') - self.assertEqual(actual_text, expected_text) - - def assertIsValidHtmlDocument(self, value): - assert isinstance(value, type(u"")) - assert value.startswith("') - maybe_document_end = value[end_html_tag_idx:].rstrip() - self.assertEqual(maybe_document_end, '') - def before_each(self): config = _assert_local_config() config_global_values = _assert_local_config_global_values(config) - def fake_start_response(status, headers, exc=None): - fake_start_response.calls.append((status, headers, exc)) - fake_start_response.calls = [] - self.fake_start_response = fake_start_response + self.fake_start_response = FakeStartResponse() def fake_set_environ(value): fake_set_environ.calls.append((value)) @@ -257,7 +215,7 @@ def fake_set_environ(value): self.instrumented_format_output = create_instrumented_format_output() self.instrumented_retrieve_handler = create_instrumented_retrieve_handler() - self.application_args = (fake_wsgi_environ, fake_start_response,) + self.application_args = (fake_wsgi_environ, self.fake_start_response,) self.application_kwargs = dict( _format_output=self.instrumented_format_output, _retrieve_handler=self.instrumented_retrieve_handler, @@ -278,7 +236,7 @@ def test_return_value_ok_returns_status_200(self): _trigger_and_unpack_result(application_result) self.assertInstrumentation() - self.assertResponseStatus(200) + self.assertWsgiResponseStatus(self.fake_start_response, 200) def test_return_value_ok_returns_valid_html_page(self): self.instrumented_retrieve_handler.program([], returnvalues.OK) From 9e554fff887cd703377e60d7b4e8c1b53e31f18a Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Tue, 24 Sep 2024 15:44:22 +0200 Subject: [PATCH 21/34] collect common default kwargs --- tests/test_mig_wsgi-bin_migwsgi.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index f1e6e7a37..fd5479906 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -217,6 +217,9 @@ def fake_set_environ(value): self.application_args = (fake_wsgi_environ, self.fake_start_response,) self.application_kwargs = dict( + _wrap_wsgi_errors=noop, + _config_file=_TEST_CONF_FILE, + _skip_log=True, _format_output=self.instrumented_format_output, _retrieve_handler=self.instrumented_retrieve_handler, _set_environ=fake_set_environ, @@ -227,9 +230,6 @@ def test_return_value_ok_returns_status_200(self): application_result = migwsgi._application( *self.application_args, - _wrap_wsgi_errors=noop, - _config_file=_TEST_CONF_FILE, - _skip_log=True, **self.application_kwargs ) @@ -243,9 +243,6 @@ def test_return_value_ok_returns_valid_html_page(self): application_result = migwsgi._application( *self.application_args, - _wrap_wsgi_errors=noop, - _config_file=_TEST_CONF_FILE, - _skip_log=True, **self.application_kwargs ) @@ -260,9 +257,6 @@ def test_return_value_ok_returns_expected_title(self): application_result = migwsgi._application( *self.application_args, - _wrap_wsgi_errors=noop, - _config_file=_TEST_CONF_FILE, - _skip_log=True, **self.application_kwargs ) From 9149a5bfd670c2c360249821d03c5c8cdbe2931e Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Tue, 24 Sep 2024 15:55:28 +0200 Subject: [PATCH 22/34] use noop for set environ --- tests/test_mig_wsgi-bin_migwsgi.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index fd5479906..dd1e379ac 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -202,11 +202,6 @@ def before_each(self): self.fake_start_response = FakeStartResponse() - def fake_set_environ(value): - fake_set_environ.calls.append((value)) - fake_set_environ.calls = [] - self.fake_set_environ = fake_set_environ - fake_wsgi_environ = create_wsgi_environ(_TEST_CONF_FILE, wsgi_variables=dict( http_host='localhost', path_info='/', @@ -222,7 +217,7 @@ def fake_set_environ(value): _skip_log=True, _format_output=self.instrumented_format_output, _retrieve_handler=self.instrumented_retrieve_handler, - _set_environ=fake_set_environ, + _set_environ=noop, ) def test_return_value_ok_returns_status_200(self): From 93cafe364e587c913123c2d2e5020c76b173be34 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Tue, 24 Sep 2024 16:00:11 +0200 Subject: [PATCH 23/34] make the generic WSGI handling setup code more uniform --- tests/support/__init__.py | 2 +- tests/support/wsgisupp.py | 4 ++++ tests/test_mig_wsgi-bin_migwsgi.py | 12 +++++++----- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/support/__init__.py b/tests/support/__init__.py index 5e4cdf221..1e0e49751 100644 --- a/tests/support/__init__.py +++ b/tests/support/__init__.py @@ -86,7 +86,7 @@ from tests.support.loggersupp import FakeLogger from tests.support.serversupp import make_wrapped_server from tests.support.wsgisupp import create_wsgi_environ, \ - ServerAssertMixin, FakeStartResponse + create_wsgi_start_response, ServerAssertMixin # Basic global logging configuration for testing diff --git a/tests/support/wsgisupp.py b/tests/support/wsgisupp.py index 6cf177a27..320b04456 100644 --- a/tests/support/wsgisupp.py +++ b/tests/support/wsgisupp.py @@ -46,6 +46,10 @@ def __call__(self, status, headers, exc=None): self.calls.append((status, headers, exc)) +def create_wsgi_start_response(): + return FakeStartResponse() + + class ServerAssertMixin: """Custom assertions for verifying server code executed under test.""" diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index dd1e379ac..41d26f070 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -39,7 +39,8 @@ from tests.support import PY2, is_path_within, \ - create_wsgi_environ, ServerAssertMixin, FakeStartResponse, HtmlAssertMixin + create_wsgi_environ, create_wsgi_start_response, \ + ServerAssertMixin, HtmlAssertMixin from mig.shared.base import client_id_dir, client_dir_id, get_short_id, \ invisible_path, allow_script, brief_list @@ -200,17 +201,18 @@ def before_each(self): config = _assert_local_config() config_global_values = _assert_local_config_global_values(config) - self.fake_start_response = FakeStartResponse() - - fake_wsgi_environ = create_wsgi_environ(_TEST_CONF_FILE, wsgi_variables=dict( + # generic WSGI setup + self.fake_wsgi_environ = create_wsgi_environ(_TEST_CONF_FILE, wsgi_variables=dict( http_host='localhost', path_info='/', )) + self.fake_start_response = create_wsgi_start_response() + # MiG WSGI wrapper specific setup self.instrumented_format_output = create_instrumented_format_output() self.instrumented_retrieve_handler = create_instrumented_retrieve_handler() - self.application_args = (fake_wsgi_environ, self.fake_start_response,) + self.application_args = (self.fake_wsgi_environ, self.fake_start_response,) self.application_kwargs = dict( _wrap_wsgi_errors=noop, _config_file=_TEST_CONF_FILE, From 47071730160d7d21e76e2de2ead79892feed1c04 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 25 Sep 2024 13:47:02 +0200 Subject: [PATCH 24/34] bring over improvements to hmtlsupp from another branch --- tests/support/htmlsupp.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/tests/support/htmlsupp.py b/tests/support/htmlsupp.py index 8f6fc63c2..61fcadbee 100644 --- a/tests/support/htmlsupp.py +++ b/tests/support/htmlsupp.py @@ -31,9 +31,9 @@ class HtmlAssertMixin: """Custom assertions for HTML containing strings.""" - def assertHtmlElementTextContent(self, value, tag_name, expected_text, trim_newlines=True): - """Locate the first occurrence of a tag within an HTML input string - and assert that any contained string equals what was supplied. + def assertHtmlElement(self, value, tag_name): + """Check that an occurrence of the specifid tag within an HTML input + string can be found. Returns the textual content of the first match. """ self.assertIsValidHtmlDocument(value) @@ -48,7 +48,19 @@ def assertHtmlElementTextContent(self, value, tag_name, expected_text, trim_newl tag_close = ''.join(['']) tag_close_index = value.index(tag_close, tag_open_index_after) - actual_text = value[tag_open_index_after:tag_close_index] + return value[tag_open_index_after:tag_close_index] + + def assertHtmlElementTextContent(self, value, tag_name, expected_text, trim_newlines=True): + """Check there is an occurrence of a tag within an HTML input string + and check the text it encloses equals exactly the expecatation. + """ + + self.assertIsValidHtmlDocument(value) + + # TODO: this is a definitively stop-gap way of finding a tag within the HTML + # and is used purely to keep this initial change to a reasonable size. + + actual_text = self.assertHtmlElement(value, tag_name) if trim_newlines: actual_text = actual_text.strip('\n') self.assertEqual(actual_text, expected_text) @@ -57,15 +69,16 @@ def assertIsValidHtmlDocument(self, value): """Check that the input string contains a valid HTML document. """ - assert isinstance(value, type(u"")) + assert isinstance(value, type(u"")), "input string was not utf8" error = None try: - assert value.startswith("') maybe_document_end = value[end_html_tag_idx:].rstrip() - self.assertEqual(maybe_document_end, '') + assert maybe_document_end == '', "no valid document closer" except Exception as exc: error = exc if error: - raise AssertionError("failed to verify input string as HTML: %s", str(exc)) + raise AssertionError("failed to verify input string as HTML: %s", str(error)) From 5ffe7925cede6d125674b4769e0c6e4336094458 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 2 Oct 2024 18:37:03 +0200 Subject: [PATCH 25/34] simplify --- mig/wsgi-bin/migwsgi.py | 9 +++++--- tests/support/wsgisupp.py | 4 ++-- tests/test_mig_wsgi-bin_migwsgi.py | 35 ++++++++---------------------- 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index f5b2f3788..5e5afba7c 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -213,9 +213,10 @@ def application(environ, start_response): def _set_os_environ(value): os.environ = value - return _application(environ, start_response, _set_environ=_set_os_environ, _wrap_wsgi_errors=wrap_wsgi_errors) + return _application(None, environ, start_response, _set_environ=_set_os_environ, _wrap_wsgi_errors=wrap_wsgi_errors) -def _application(environ, start_response, _set_environ, _format_output=format_output, _retrieve_handler=_import_backend, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False): + +def _application(configuration, environ, start_response, _set_environ, _format_output=format_output, _retrieve_handler=_import_backend, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False): # NOTE: pass app environ including apache and query args on to sub handlers # through the usual 'os.environ' channel expected in functionality @@ -266,7 +267,9 @@ def _application(environ, start_response, _set_environ, _format_output=format_ou if sys.version_info[0] < 3: sys.stdout = sys.stderr - configuration = get_configuration_object(_config_file, _skip_log) + if configuration is None: + configuration = get_configuration_object(_config_file, _skip_log) + _logger = configuration.logger # NOTE: replace default wsgi errors to apache error log with our own logs diff --git a/tests/support/wsgisupp.py b/tests/support/wsgisupp.py index 320b04456..b3bdb0672 100644 --- a/tests/support/wsgisupp.py +++ b/tests/support/wsgisupp.py @@ -28,10 +28,10 @@ """Test support library for WSGI.""" -def create_wsgi_environ(config_file, wsgi_variables): +def create_wsgi_environ(configuration, wsgi_variables): environ = {} environ['wsgi.input'] = () - environ['MIG_CONF'] = config_file + environ['MIG_CONF'] = configuration.config_file environ['HTTP_HOST'] = wsgi_variables.get('http_host') environ['PATH_INFO'] = wsgi_variables.get('path_info') environ['SCRIPT_URI'] = ''.join(('http://', environ['HTTP_HOST'], environ['PATH_INFO'])) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 41d26f070..099bbd954 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -61,29 +61,12 @@ def _import_migwsgi(): migwsgi = _import_migwsgi() -def _assert_local_config(): - try: - link_stat = os.lstat(_TEST_CONF_SYMLINK) - assert stat.S_ISLNK(link_stat.st_mode) - configdir_stat = os.stat(_TEST_CONF_DIR) - assert stat.S_ISDIR(configdir_stat.st_mode) - config = ConfigParser() - config.read([_TEST_CONF_FILE]) - return config - except Exception as exc: - raise AssertionError('local configuration invalid or missing: %s' % (str(exc),)) - - -def _assert_local_config_global_values(config): - config_global_values = dict(config.items('GLOBAL')) - - for path in ('mig_path', 'certs_path', 'state_path'): - path_value = config_global_values.get(path) +def _assert_local_config_global_values(configuration): + for config_key in ('mig_path', 'certs_path', 'state_path'): + path_value = getattr(configuration, config_key) if not is_path_within(path_value, start=_LOCAL_MIG_BASE): raise AssertionError('local config contains bad path: %s=%s' % (path, path_value)) - return config_global_values - def _is_return_value(return_value): defined_return_values = returnvalues.__dict__.values() @@ -197,12 +180,14 @@ def was_called(fake): self.assertTrue(was_called(self.instrumented_format_output), "no output generated") self.assertTrue(was_called(self.instrumented_retrieve_handler), "no output generated") + def _provide_configuration(self): + return 'testconfig' + def before_each(self): - config = _assert_local_config() - config_global_values = _assert_local_config_global_values(config) + _assert_local_config_global_values(self.configuration) # generic WSGI setup - self.fake_wsgi_environ = create_wsgi_environ(_TEST_CONF_FILE, wsgi_variables=dict( + self.fake_wsgi_environ = create_wsgi_environ(self.configuration, wsgi_variables=dict( http_host='localhost', path_info='/', )) @@ -212,11 +197,9 @@ def before_each(self): self.instrumented_format_output = create_instrumented_format_output() self.instrumented_retrieve_handler = create_instrumented_retrieve_handler() - self.application_args = (self.fake_wsgi_environ, self.fake_start_response,) + self.application_args = (self.configuration, self.fake_wsgi_environ, self.fake_start_response,) self.application_kwargs = dict( _wrap_wsgi_errors=noop, - _config_file=_TEST_CONF_FILE, - _skip_log=True, _format_output=self.instrumented_format_output, _retrieve_handler=self.instrumented_retrieve_handler, _set_environ=noop, From f9859bdcf5b609d23369bb9105d3ca2c162656a1 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 2 Oct 2024 18:39:48 +0200 Subject: [PATCH 26/34] fixup --- tests/test_mig_wsgi-bin_migwsgi.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 099bbd954..bb1ab1677 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -61,13 +61,6 @@ def _import_migwsgi(): migwsgi = _import_migwsgi() -def _assert_local_config_global_values(configuration): - for config_key in ('mig_path', 'certs_path', 'state_path'): - path_value = getattr(configuration, config_key) - if not is_path_within(path_value, start=_LOCAL_MIG_BASE): - raise AssertionError('local config contains bad path: %s=%s' % (path, path_value)) - - def _is_return_value(return_value): defined_return_values = returnvalues.__dict__.values() return return_value in defined_return_values @@ -184,8 +177,6 @@ def _provide_configuration(self): return 'testconfig' def before_each(self): - _assert_local_config_global_values(self.configuration) - # generic WSGI setup self.fake_wsgi_environ = create_wsgi_environ(self.configuration, wsgi_variables=dict( http_host='localhost', From e9444d40a0ac19dc860dc02cbf1163e747029fec Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Tue, 24 Sep 2024 12:37:44 +0200 Subject: [PATCH 27/34] Make responding with binary data work under PY3. The code as previously written would unconditionally encode parts as though they contained text - this went unnoticed on PY2 because strings and bytes are one and the same thing but blew up on PY3 where a string is explicitly of type unicode while a binary file would be raw bytes. Explicitly check the output_format and if instructed to serve a file do so without touching the chunks of file content bytes being yielded. --- mig/wsgi-bin/migwsgi.py | 9 ++-- tests/data/loading.gif | 1 + tests/support/__init__.py | 2 +- tests/support/suppconst.py | 1 + tests/test_mig_wsgi-bin_migwsgi.py | 70 ++++++++++++++++++++++++++++-- 5 files changed, 76 insertions(+), 7 deletions(-) create mode 120000 tests/data/loading.gif diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index 5e5afba7c..8a8e10fd7 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -216,7 +216,7 @@ def _set_os_environ(value): return _application(None, environ, start_response, _set_environ=_set_os_environ, _wrap_wsgi_errors=wrap_wsgi_errors) -def _application(configuration, environ, start_response, _set_environ, _format_output=format_output, _retrieve_handler=_import_backend, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False): +def _application(configuration, environ, start_response, _set_environ, _fieldstorage_to_dict=fieldstorage_to_dict, _format_output=format_output, _retrieve_handler=_import_backend, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False): # NOTE: pass app environ including apache and query args on to sub handlers # through the usual 'os.environ' channel expected in functionality @@ -329,7 +329,7 @@ def _application(configuration, environ, start_response, _set_environ, _format_o # (backend, script_name)) fieldstorage = cgi.FieldStorage(fp=environ['wsgi.input'], environ=environ) - user_arguments_dict = fieldstorage_to_dict(fieldstorage) + user_arguments_dict = _fieldstorage_to_dict(fieldstorage) if 'output_format' in user_arguments_dict: output_format = user_arguments_dict['output_format'][0] @@ -447,7 +447,10 @@ def _application(configuration, environ, start_response, _set_environ, _format_o # (backend, i+1, chunk_parts)) # end index may be after end of content - but no problem part = output[i*download_block_size:(i+1)*download_block_size] - yield _ensure_encoded_string(part) + if output_format == 'file': + yield part + else: + yield _ensure_encoded_string(part) if chunk_parts > 1: _logger.info("WSGI %s finished yielding all %d output parts" % (backend, chunk_parts)) diff --git a/tests/data/loading.gif b/tests/data/loading.gif new file mode 120000 index 000000000..2ef2c9b77 --- /dev/null +++ b/tests/data/loading.gif @@ -0,0 +1 @@ +../../mig/images/loading.gif \ No newline at end of file diff --git a/tests/support/__init__.py b/tests/support/__init__.py index 1e0e49751..a1b54babb 100644 --- a/tests/support/__init__.py +++ b/tests/support/__init__.py @@ -42,7 +42,7 @@ from tests.support.configsupp import FakeConfiguration from tests.support.suppconst import MIG_BASE, TEST_BASE, TEST_FIXTURE_DIR, \ - TEST_OUTPUT_DIR + TEST_OUTPUT_DIR, TEST_DATA_DIR PY2 = (sys.version_info[0] == 2) diff --git a/tests/support/suppconst.py b/tests/support/suppconst.py index fcf401290..15912e933 100644 --- a/tests/support/suppconst.py +++ b/tests/support/suppconst.py @@ -31,6 +31,7 @@ # Use abspath for __file__ on Py2 _SUPPORT_DIR = os.path.dirname(os.path.abspath(__file__)) TEST_BASE = os.path.normpath(os.path.join(_SUPPORT_DIR, "..")) +TEST_DATA_DIR = os.path.join(TEST_BASE, "data") TEST_FIXTURE_DIR = os.path.join(TEST_BASE, "fixture") TEST_OUTPUT_DIR = os.path.join(TEST_BASE, "output") MIG_BASE = os.path.realpath(os.path.join(TEST_BASE, "..")) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index bb1ab1677..8fba8899d 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -32,8 +32,9 @@ import os import stat import sys +import unittest -from tests.support import MIG_BASE, MigTestCase, testmain +from tests.support import MIG_BASE, TEST_BASE, TEST_DATA_DIR, MigTestCase, testmain from mig.shared.output import format_output import mig.shared.returnvalues as returnvalues @@ -66,14 +67,35 @@ def _is_return_value(return_value): return return_value in defined_return_values -def _trigger_and_unpack_result(application_result): +def _trigger_and_unpack_result(application_result, result_kind='textual'): + assert result_kind in ('textual', 'binary') + chunks = list(application_result) assert len(chunks) > 0, "invocation returned no output" complete_value = b''.join(chunks) - decoded_value = codecs.decode(complete_value, 'utf8') + if result_kind == 'binary': + decoded_value = complete_value + else: + decoded_value = codecs.decode(complete_value, 'utf8') return decoded_value +def create_instrumented_fieldstorage_to_dict(): + def _instrumented_fieldstorage_to_dict(fieldstorage): + return _instrumented_fieldstorage_to_dict._result + + _instrumented_fieldstorage_to_dict._result = { + 'output_format': ('html',) + } + + def set_result(result): + _instrumented_fieldstorage_to_dict._result = result + + _instrumented_fieldstorage_to_dict.set_result = set_result + + return _instrumented_fieldstorage_to_dict + + def create_instrumented_format_output(): def _instrumented_format_output( configuration, @@ -88,6 +110,16 @@ def _instrumented_format_output( call_args = (configuration, backend, ret_val, ret_msg, call_args_out_obj, outputformat,) _instrumented_format_output.calls.append({ 'args': call_args }) + if _instrumented_format_output._file: + return format_output( + configuration, + backend, + ret_val, + ret_msg, + out_obj, + outputformat, + ) + # FIXME: the following is a workaround for a bug that exists between the WSGI wrapper # and the output formatter - specifically, the latter adds default header and # title if start does not exist, but the former ensures that start always exists @@ -122,11 +154,18 @@ def _instrumented_format_output( outputformat, ) _instrumented_format_output.calls = [] + _instrumented_format_output._file = False _instrumented_format_output.values = dict( title_text='', header_text='', ) + + def _set_file(is_enabled): + _instrumented_format_output._file = is_enabled + + setattr(_instrumented_format_output, 'set_file', _set_file) + def _program_values(**kwargs): _instrumented_format_output.values.update(kwargs) @@ -185,6 +224,7 @@ def before_each(self): self.fake_start_response = create_wsgi_start_response() # MiG WSGI wrapper specific setup + self.instrumented_fieldstorage_to_dict = create_instrumented_fieldstorage_to_dict() self.instrumented_format_output = create_instrumented_format_output() self.instrumented_retrieve_handler = create_instrumented_retrieve_handler() @@ -192,6 +232,7 @@ def before_each(self): self.application_kwargs = dict( _wrap_wsgi_errors=noop, _format_output=self.instrumented_format_output, + _fieldstorage_to_dict=self.instrumented_fieldstorage_to_dict, _retrieve_handler=self.instrumented_retrieve_handler, _set_environ=noop, ) @@ -236,6 +277,29 @@ def test_return_value_ok_returns_expected_title(self): self.assertInstrumentation() self.assertHtmlElementTextContent(output, 'title', 'TEST', trim_newlines=True) + def test_return_value_ok_serving_a_binary_file(self): + test_binary_file = os.path.join(TEST_DATA_DIR, 'loading.gif') + with open(test_binary_file, 'rb') as f: + test_binary_data = f.read() + + self.instrumented_fieldstorage_to_dict.set_result({ + 'output_format': ('file',) + }) + self.instrumented_format_output.set_file(True) + + file_obj = { 'object_type': 'binary', 'data': test_binary_data } + self.instrumented_retrieve_handler.program([file_obj], returnvalues.OK) + + application_result = migwsgi._application( + *self.application_args, + **self.application_kwargs + ) + + output = _trigger_and_unpack_result(application_result, 'binary') + + self.assertInstrumentation() + self.assertEqual(output, test_binary_data) + if __name__ == '__main__': testmain() From f9d018bc5fd2433c7a51551ff4bf712787f93880 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 25 Sep 2024 15:58:20 +0200 Subject: [PATCH 28/34] WIP - cat paths --- mig/wsgi-bin/migwsgi.py | 23 +++++++++++++++++------ tests/support/__init__.py | 6 +++++- tests/test_mig_wsgi-bin_migwsgi.py | 24 ++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index 8a8e10fd7..acace301c 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -360,7 +360,7 @@ def _application(configuration, environ, start_response, _set_environ, _fieldsto if 'json' == output_format: default_content = 'application/json' - elif 'file' == output_format: + elif 'file' == output_format or 'chunked' == output_format: default_content = 'application/octet-stream' elif 'html' != output_format: default_content = 'text/plain' @@ -410,17 +410,22 @@ def _application(configuration, environ, start_response, _set_environ, _fieldsto if output is None: _logger.error("WSGI %s output formatting failed" % output_format) output = 'Error: output could not be correctly delivered!' + output_format = 'html' - content_length = len(output) - if not 'Content-Length' in dict(response_headers): - # _logger.debug("WSGI adding explicit content length %s" % content_length) - response_headers.append(('Content-Length', "%d" % content_length)) + if output_format == 'chunked': + response_headers.append(('Transfer-Encoding', 'chunked')) + else: + content_length = len(output) + if not 'Content-Length' in dict(response_headers): + # adding explicit content length + response_headers.append(('Content-Length', "%d" % content_length)) _logger.debug("send %r response as %s to %s" % (backend, output_format, client_id)) # NOTE: send response to client but don't crash e.g. on closed connection try: - start_response(status, response_headers) + x = start_response(status, response_headers) + pass except IOError as ioe: _logger.warning("WSGI %s for %s could not deliver output: %s" % (backend, client_id, ioe)) @@ -428,6 +433,12 @@ def _application(configuration, environ, start_response, _set_environ, _fieldsto _logger.error("WSGI %s for %s crashed during response: %s" % (backend, client_id, exc)) + if output_format == 'chunked': + file_obj = next((x for x in output_objs if x['object_type'] == 'file_abspath'), None) + with open(file_obj['abspath'], 'rb') as fh_to_send: + os.sendfile(None, fh_to_send) + + # serve response data with a known content type try: # NOTE: we consistently hit download error for archive files reaching ~2GB # with showfreezefile.py on wsgi but the same on cgi does NOT suffer diff --git a/tests/support/__init__.py b/tests/support/__init__.py index a1b54babb..2445b2ac8 100644 --- a/tests/support/__init__.py +++ b/tests/support/__init__.py @@ -343,12 +343,16 @@ def temppath(relative_path, test_case, ensure_dir=False, skip_clean=False): """Get absolute temp path for relative_path""" assert isinstance(test_case, MigTestCase) tmp_path = os.path.join(TEST_OUTPUT_DIR, relative_path) + return _temppath(tmp_path, test_case, ensure_dir=ensure_dir, skip_clean=skip_clean) + + +def _temppath(tmp_path, test_case, ensure_dir=False, skip_clean=False): if ensure_dir: try: os.mkdir(tmp_path) except FileExistsError: raise AssertionError( - "ABORT: use of unclean output path: %s" % relative_path) + "ABORT: use of unclean output path: %s" % tmp_path) if not skip_clean: test_case._cleanup_paths.add(tmp_path) return tmp_path diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 8fba8899d..6fdff7148 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -300,6 +300,30 @@ def test_return_value_ok_serving_a_binary_file(self): self.assertInstrumentation() self.assertEqual(output, test_binary_data) + def test_sendfile(self): + test_binary_file = os.path.join(TEST_DATA_DIR, 'loading.gif') + + self.instrumented_fieldstorage_to_dict.set_result({ + 'output_format': ('chunked',) + }) + self.instrumented_format_output.set_file(True) + + output_obj = { + 'object_type': 'file_abspath', + 'abspath': test_binary_file + } + self.instrumented_retrieve_handler.program([output_obj], returnvalues.OK) + + application_result = migwsgi._application( + *self.application_args, + **self.application_kwargs + ) + + output = _trigger_and_unpack_result(application_result, 'binary') + + self.assertInstrumentation() + self.assertEqual(output, test_binary_data) + if __name__ == '__main__': testmain() From 3c212e1e15ab5a946a62352dba3dd2e4fcaca522 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 2 Oct 2024 15:53:32 +0200 Subject: [PATCH 29/34] adapt --- mig/wsgi-bin/migwsgi.py | 30 +++++++++++++++++------------- tests/test_mig_wsgi-bin_migwsgi.py | 14 ++++++++++---- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index acace301c..c5e467b00 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -412,20 +412,29 @@ def _application(configuration, environ, start_response, _set_environ, _fieldsto output = 'Error: output could not be correctly delivered!' output_format = 'html' - if output_format == 'chunked': + if output_format == 'serve': response_headers.append(('Transfer-Encoding', 'chunked')) - else: - content_length = len(output) - if not 'Content-Length' in dict(response_headers): - # adding explicit content length - response_headers.append(('Content-Length', "%d" % content_length)) + + start_response(status, response_headers) + + serve_obj = next((x for x in output_objs if x['object_type'] == 'serve_paths'), None) + + for path in serve_obj['paths']: + with open(path, 'rb') as fh_for_path: + yield fh_for_path.read() + + return + + content_length = len(output) + if not 'Content-Length' in dict(response_headers): + # adding explicit content length + response_headers.append(('Content-Length', "%d" % content_length)) _logger.debug("send %r response as %s to %s" % (backend, output_format, client_id)) # NOTE: send response to client but don't crash e.g. on closed connection try: - x = start_response(status, response_headers) - pass + start_response(status, response_headers) except IOError as ioe: _logger.warning("WSGI %s for %s could not deliver output: %s" % (backend, client_id, ioe)) @@ -433,11 +442,6 @@ def _application(configuration, environ, start_response, _set_environ, _fieldsto _logger.error("WSGI %s for %s crashed during response: %s" % (backend, client_id, exc)) - if output_format == 'chunked': - file_obj = next((x for x in output_objs if x['object_type'] == 'file_abspath'), None) - with open(file_obj['abspath'], 'rb') as fh_to_send: - os.sendfile(None, fh_to_send) - # serve response data with a known content type try: # NOTE: we consistently hit download error for archive files reaching ~2GB diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 6fdff7148..8620349ed 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -237,6 +237,7 @@ def before_each(self): _set_environ=noop, ) + @unittest.skip("TEMP") def test_return_value_ok_returns_status_200(self): self.instrumented_retrieve_handler.program([], returnvalues.OK) @@ -250,6 +251,7 @@ def test_return_value_ok_returns_status_200(self): self.assertInstrumentation() self.assertWsgiResponseStatus(self.fake_start_response, 200) + @unittest.skip("TEMP") def test_return_value_ok_returns_valid_html_page(self): self.instrumented_retrieve_handler.program([], returnvalues.OK) @@ -263,6 +265,7 @@ def test_return_value_ok_returns_valid_html_page(self): self.assertInstrumentation() self.assertIsValidHtmlDocument(output) + @unittest.skip("TEMP") def test_return_value_ok_returns_expected_title(self): self.instrumented_format_output.set_values(title_text='TEST') self.instrumented_retrieve_handler.program([], returnvalues.OK) @@ -277,6 +280,7 @@ def test_return_value_ok_returns_expected_title(self): self.assertInstrumentation() self.assertHtmlElementTextContent(output, 'title', 'TEST', trim_newlines=True) + @unittest.skip("TEMP") def test_return_value_ok_serving_a_binary_file(self): test_binary_file = os.path.join(TEST_DATA_DIR, 'loading.gif') with open(test_binary_file, 'rb') as f: @@ -300,17 +304,19 @@ def test_return_value_ok_serving_a_binary_file(self): self.assertInstrumentation() self.assertEqual(output, test_binary_data) - def test_sendfile(self): + def test_serve_paths_signle_file(self): test_binary_file = os.path.join(TEST_DATA_DIR, 'loading.gif') + with open(test_binary_file, 'rb') as fh_test_file: + test_binary_data = fh_test_file.read() self.instrumented_fieldstorage_to_dict.set_result({ - 'output_format': ('chunked',) + 'output_format': ('serve',) }) self.instrumented_format_output.set_file(True) output_obj = { - 'object_type': 'file_abspath', - 'abspath': test_binary_file + 'object_type': 'serve_paths', + 'paths': [test_binary_file] } self.instrumented_retrieve_handler.program([output_obj], returnvalues.OK) From a8207df7c84b705163954bc0d160534cc5218dbc Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 2 Oct 2024 18:13:05 +0200 Subject: [PATCH 30/34] limit --- mig/shared/configuration.py | 2 ++ mig/shared/returnvalues.py | 4 +++ mig/wsgi-bin/migwsgi.py | 19 ++++++++++++-- tests/test_mig_wsgi-bin_migwsgi.py | 41 ++++++++++++++++++++++++++++-- 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/mig/shared/configuration.py b/mig/shared/configuration.py index bbf969ed5..ed8a486a3 100644 --- a/mig/shared/configuration.py +++ b/mig/shared/configuration.py @@ -222,6 +222,7 @@ def fix_missing(config_file, verbose=True): 'trac_id_field': 'email', 'migserver_http_url': 'http://%%(server_fqdn)s', 'migserver_https_url': '', + 'migserver_server_maxsize': float('inf'), 'myfiles_py_location': '', 'mig_server_id': '%s.0' % fqdn, 'empty_job_name': 'no_suitable_job-', @@ -596,6 +597,7 @@ class Configuration: migserver_https_mig_oidc_url = '' migserver_https_ext_oidc_url = '' migserver_https_sid_url = '' + migserver_server_maxsize = float('inf') sleep_period_for_empty_jobs = '' min_seconds_between_live_update_requests = 0 cputime_for_empty_jobs = 0 diff --git a/mig/shared/returnvalues.py b/mig/shared/returnvalues.py index 8c9d98363..f0ddcbe98 100644 --- a/mig/shared/returnvalues.py +++ b/mig/shared/returnvalues.py @@ -46,3 +46,7 @@ USER_NOT_CREATED = (201, 'USER_NOT_CREATED') OUTPUT_VALIDATION_ERROR = (202, 'The output the server ' + 'has generated could not be validated') + +# REQUEST ERRORS + +REJECTED_ERROR = (422, 'REJECTED') diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index c5e467b00..72c9dc25b 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -59,6 +59,10 @@ def _import_backend(backend): return module_handle.main +def _returnvalue_to_status(returnvalue): + return ' '.join((str(item) for item in returnvalue)) + + def object_type_info(object_type): """Lookup object type""" @@ -415,10 +419,21 @@ def _application(configuration, environ, start_response, _set_environ, _fieldsto if output_format == 'serve': response_headers.append(('Transfer-Encoding', 'chunked')) - start_response(status, response_headers) - serve_obj = next((x for x in output_objs if x['object_type'] == 'serve_paths'), None) + serve_paths_stat_results = (os.stat(path) for path in serve_obj['paths']) + serve_paths_total_bytes = sum(st.st_size for st in serve_paths_stat_results) + + serve_maxsize = configuration.migserver_server_maxsize + + if serve_maxsize != float('inf') and serve_paths_total_bytes > serve_maxsize: + status = _returnvalue_to_status(returnvalues.REJECTED_ERROR) + start_response(status, {}) + return b'' + + # we are all good to respond.. do so + start_response(status, response_headers) + for path in serve_obj['paths']: with open(path, 'rb') as fh_for_path: yield fh_for_path.read() diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 8620349ed..1d111c53e 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -68,9 +68,14 @@ def _is_return_value(return_value): def _trigger_and_unpack_result(application_result, result_kind='textual'): - assert result_kind in ('textual', 'binary') + assert result_kind in ('textual', 'binary', 'none') chunks = list(application_result) + + if result_kind == 'none': + assert len(chunks) == 0, "invocation returned output but none expected" + return None + assert len(chunks) > 0, "invocation returned no output" complete_value = b''.join(chunks) if result_kind == 'binary': @@ -304,11 +309,14 @@ def test_return_value_ok_serving_a_binary_file(self): self.assertInstrumentation() self.assertEqual(output, test_binary_data) - def test_serve_paths_signle_file(self): + def test_serve_paths_signle_file_at_limit(self): test_binary_file = os.path.join(TEST_DATA_DIR, 'loading.gif') + test_binary_file_size = os.stat(test_binary_file).st_size with open(test_binary_file, 'rb') as fh_test_file: test_binary_data = fh_test_file.read() + self.configuration.migserver_server_maxsize = test_binary_file_size + self.instrumented_fieldstorage_to_dict.set_result({ 'output_format': ('serve',) }) @@ -330,6 +338,35 @@ def test_serve_paths_signle_file(self): self.assertInstrumentation() self.assertEqual(output, test_binary_data) + def test_serve_paths_signle_file_over_limit(self): + test_binary_file = os.path.join(TEST_DATA_DIR, 'loading.gif') + test_binary_file_size = os.stat(test_binary_file).st_size + with open(test_binary_file, 'rb') as fh_test_file: + test_binary_data = fh_test_file.read() + + self.configuration.migserver_server_maxsize = test_binary_file_size - 1 + + self.instrumented_fieldstorage_to_dict.set_result({ + 'output_format': ('serve',) + }) + self.instrumented_format_output.set_file(True) + + output_obj = { + 'object_type': 'serve_paths', + 'paths': [test_binary_file] + } + self.instrumented_retrieve_handler.program([output_obj], returnvalues.OK) + + application_result = migwsgi._application( + *self.application_args, + **self.application_kwargs + ) + + _trigger_and_unpack_result(application_result, 'none') + + self.assertInstrumentation() + self.assertWsgiResponseStatus(self.fake_start_response, 422) + if __name__ == '__main__': testmain() From ddc3787b2b09e665f1f38ec5a0eeb0baa97c13cb Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 2 Oct 2024 18:47:46 +0200 Subject: [PATCH 31/34] fixup --- tests/test_mig_wsgi-bin_migwsgi.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index 1d111c53e..c1faaafd9 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -32,7 +32,6 @@ import os import stat import sys -import unittest from tests.support import MIG_BASE, TEST_BASE, TEST_DATA_DIR, MigTestCase, testmain from mig.shared.output import format_output @@ -242,7 +241,6 @@ def before_each(self): _set_environ=noop, ) - @unittest.skip("TEMP") def test_return_value_ok_returns_status_200(self): self.instrumented_retrieve_handler.program([], returnvalues.OK) @@ -256,7 +254,6 @@ def test_return_value_ok_returns_status_200(self): self.assertInstrumentation() self.assertWsgiResponseStatus(self.fake_start_response, 200) - @unittest.skip("TEMP") def test_return_value_ok_returns_valid_html_page(self): self.instrumented_retrieve_handler.program([], returnvalues.OK) @@ -270,7 +267,6 @@ def test_return_value_ok_returns_valid_html_page(self): self.assertInstrumentation() self.assertIsValidHtmlDocument(output) - @unittest.skip("TEMP") def test_return_value_ok_returns_expected_title(self): self.instrumented_format_output.set_values(title_text='TEST') self.instrumented_retrieve_handler.program([], returnvalues.OK) @@ -285,7 +281,6 @@ def test_return_value_ok_returns_expected_title(self): self.assertInstrumentation() self.assertHtmlElementTextContent(output, 'title', 'TEST', trim_newlines=True) - @unittest.skip("TEMP") def test_return_value_ok_serving_a_binary_file(self): test_binary_file = os.path.join(TEST_DATA_DIR, 'loading.gif') with open(test_binary_file, 'rb') as f: From 88afad8daee60cc3a53903156616eca6264d6eb7 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 2 Oct 2024 19:09:11 +0200 Subject: [PATCH 32/34] abandon ship on float('inf') and use -1 for limit --- mig/shared/configuration.py | 4 ++-- mig/wsgi-bin/migwsgi.py | 2 +- tests/fixture/mig_shared_configuration--new.json | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/mig/shared/configuration.py b/mig/shared/configuration.py index ed8a486a3..e77103a2c 100644 --- a/mig/shared/configuration.py +++ b/mig/shared/configuration.py @@ -222,7 +222,7 @@ def fix_missing(config_file, verbose=True): 'trac_id_field': 'email', 'migserver_http_url': 'http://%%(server_fqdn)s', 'migserver_https_url': '', - 'migserver_server_maxsize': float('inf'), + 'migserver_server_maxsize': -1, 'myfiles_py_location': '', 'mig_server_id': '%s.0' % fqdn, 'empty_job_name': 'no_suitable_job-', @@ -597,7 +597,7 @@ class Configuration: migserver_https_mig_oidc_url = '' migserver_https_ext_oidc_url = '' migserver_https_sid_url = '' - migserver_server_maxsize = float('inf') + migserver_server_maxsize = -1 sleep_period_for_empty_jobs = '' min_seconds_between_live_update_requests = 0 cputime_for_empty_jobs = 0 diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index 72c9dc25b..29915faac 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -426,7 +426,7 @@ def _application(configuration, environ, start_response, _set_environ, _fieldsto serve_maxsize = configuration.migserver_server_maxsize - if serve_maxsize != float('inf') and serve_paths_total_bytes > serve_maxsize: + if serve_maxsize > -1 and serve_paths_total_bytes > serve_maxsize: status = _returnvalue_to_status(returnvalues.REJECTED_ERROR) start_response(status, {}) return b'' diff --git a/tests/fixture/mig_shared_configuration--new.json b/tests/fixture/mig_shared_configuration--new.json index b73c4d842..9b61988a7 100644 --- a/tests/fixture/mig_shared_configuration--new.json +++ b/tests/fixture/mig_shared_configuration--new.json @@ -121,6 +121,7 @@ "migserver_https_url": "", "migserver_public_alias_url": "", "migserver_public_url": "", + "migserver_server_maxsize": -1, "min_seconds_between_live_update_requests": 0, "mrsl_files_dir": "", "myfiles_py_location": "", From fea96794626be1fd05baf7665efc645e78f20aca Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 2 Oct 2024 20:56:56 +0200 Subject: [PATCH 33/34] carve out as a function --- mig/wsgi-bin/migwsgi.py | 43 ++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index 29915faac..d589f6d4a 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -69,6 +69,27 @@ def object_type_info(object_type): return get_object_type_info(object_type) +def serve_paths(configuration, serve_obj, start_response): + serve_maxsize = configuration.migserver_server_maxsize + + serve_paths_stat_results = (os.stat(path) for path in serve_obj['paths']) + serve_paths_total_bytes = sum(st.st_size for st in serve_paths_stat_results) + + if serve_maxsize > -1 and serve_paths_total_bytes > serve_maxsize: + start_response(_returnvalue_to_status(returnvalues.REJECTED_ERROR), {}) + return b'' + + # we are all good to respond.. do so + start_response(_returnvalue_to_status(returnvalues.OK), { + 'Content-Type': 'application/octet-stream', + 'Transfer-Encoding': 'chunked', + }) + + for path in serve_obj['paths']: + with open(path, 'rb') as path_handle: + yield path_handle.read() + + def stub(configuration, client_id, user_arguments_dict, environ, _retrieve_handler): """Run backend on behalf of client_id with supplied user_arguments_dict. I.e. import main from import_path and execute it with supplied arguments. @@ -417,27 +438,9 @@ def _application(configuration, environ, start_response, _set_environ, _fieldsto output_format = 'html' if output_format == 'serve': - response_headers.append(('Transfer-Encoding', 'chunked')) - serve_obj = next((x for x in output_objs if x['object_type'] == 'serve_paths'), None) - - serve_paths_stat_results = (os.stat(path) for path in serve_obj['paths']) - serve_paths_total_bytes = sum(st.st_size for st in serve_paths_stat_results) - - serve_maxsize = configuration.migserver_server_maxsize - - if serve_maxsize > -1 and serve_paths_total_bytes > serve_maxsize: - status = _returnvalue_to_status(returnvalues.REJECTED_ERROR) - start_response(status, {}) - return b'' - - # we are all good to respond.. do so - start_response(status, response_headers) - - for path in serve_obj['paths']: - with open(path, 'rb') as fh_for_path: - yield fh_for_path.read() - + for piece in serve_paths(configuration, serve_obj, start_response): + yield piece return content_length = len(output) From c1246c90be1563579b01b49bb952404d44e27295 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Thu, 3 Oct 2024 11:13:05 +0200 Subject: [PATCH 34/34] tweak + lint --- mig/wsgi-bin/migwsgi.py | 11 ++++++----- tests/test_mig_wsgi-bin_migwsgi.py | 9 +++------ 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index d589f6d4a..11387f669 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -69,15 +69,16 @@ def object_type_info(object_type): return get_object_type_info(object_type) -def serve_paths(configuration, serve_obj, start_response): +def serve_paths(configuration, paths, start_response): serve_maxsize = configuration.migserver_server_maxsize - serve_paths_stat_results = (os.stat(path) for path in serve_obj['paths']) + serve_paths_stat_results = (os.stat(path) for path in paths) serve_paths_total_bytes = sum(st.st_size for st in serve_paths_stat_results) if serve_maxsize > -1 and serve_paths_total_bytes > serve_maxsize: start_response(_returnvalue_to_status(returnvalues.REJECTED_ERROR), {}) - return b'' + yield b'' + return # we are all good to respond.. do so start_response(_returnvalue_to_status(returnvalues.OK), { @@ -85,7 +86,7 @@ def serve_paths(configuration, serve_obj, start_response): 'Transfer-Encoding': 'chunked', }) - for path in serve_obj['paths']: + for path in paths: with open(path, 'rb') as path_handle: yield path_handle.read() @@ -439,7 +440,7 @@ def _application(configuration, environ, start_response, _set_environ, _fieldsto if output_format == 'serve': serve_obj = next((x for x in output_objs if x['object_type'] == 'serve_paths'), None) - for piece in serve_paths(configuration, serve_obj, start_response): + for piece in serve_paths(configuration, serve_obj['paths'], start_response): yield piece return diff --git a/tests/test_mig_wsgi-bin_migwsgi.py b/tests/test_mig_wsgi-bin_migwsgi.py index c1faaafd9..39129324b 100644 --- a/tests/test_mig_wsgi-bin_migwsgi.py +++ b/tests/test_mig_wsgi-bin_migwsgi.py @@ -67,14 +67,10 @@ def _is_return_value(return_value): def _trigger_and_unpack_result(application_result, result_kind='textual'): - assert result_kind in ('textual', 'binary', 'none') + assert result_kind in ('textual', 'binary') chunks = list(application_result) - if result_kind == 'none': - assert len(chunks) == 0, "invocation returned output but none expected" - return None - assert len(chunks) > 0, "invocation returned no output" complete_value = b''.join(chunks) if result_kind == 'binary': @@ -357,9 +353,10 @@ def test_serve_paths_signle_file_over_limit(self): **self.application_kwargs ) - _trigger_and_unpack_result(application_result, 'none') + output = _trigger_and_unpack_result(application_result, 'binary') self.assertInstrumentation() + self.assertEqual(output, b'') self.assertWsgiResponseStatus(self.fake_start_response, 422)