From 7382944df0dafd533ae4db557ee50dd623d123a9 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 11 Jun 2014 15:16:05 -0500 Subject: [PATCH] Introduce unit(-ish) tests for routes. Allows testing both how URLs are going to map to controllers and actions as well as testing what paths will be produced by url_for. Requires some refactoring of Galaxy framework classes and buildapp to allow stubbing out actual instantiation of controllers, etc.... --- lib/galaxy/web/framework/__init__.py | 12 +++- lib/galaxy/webapps/galaxy/buildapp.py | 60 ++++++++++---------- test/unit/test_routes.py | 81 +++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 31 deletions(-) create mode 100644 test/unit/test_routes.py diff --git a/lib/galaxy/web/framework/__init__.py b/lib/galaxy/web/framework/__init__.py index 3c282927f2e..51ce453eb7f 100644 --- a/lib/galaxy/web/framework/__init__.py +++ b/lib/galaxy/web/framework/__init__.py @@ -454,7 +454,8 @@ class WebApplication( base.WebApplication ): for key in dir( module ): T = getattr( module, key ) if inspect.isclass( T ) and T is not BaseUIController and issubclass( T, BaseUIController ): - self.add_ui_controller( name, T( app ) ) + controller = self._instantiate_controller( T, app ) + self.add_ui_controller( name, controller ) def add_api_controllers( self, package_name, app ): """ @@ -480,7 +481,14 @@ class WebApplication( base.WebApplication ): if inspect.isclass( T ) and not key.startswith("Base") and issubclass( T, BaseAPIController ): # By default use module_name, but allow controller to override name controller_name = getattr( T, "controller_name", name ) - self.add_api_controller( controller_name, T( app ) ) + controller = self._instantiate_controller( T, app ) + self.add_api_controller( controller_name, controller ) + + def _instantiate_controller( self, T, app ): + """ Extension point, allow apps to contstruct controllers differently, + really just used to stub out actual controllers for routes testing. + """ + return T( app ) class GalaxyWebTransaction( base.DefaultWebTransaction ): diff --git a/lib/galaxy/webapps/galaxy/buildapp.py b/lib/galaxy/webapps/galaxy/buildapp.py index 5e2ce4be9e4..cf0c45dd926 100644 --- a/lib/galaxy/webapps/galaxy/buildapp.py +++ b/lib/galaxy/webapps/galaxy/buildapp.py @@ -70,10 +70,38 @@ def app_factory( global_conf, **kwargs ): webapp.add_route( '/u/:username/v/:slug', controller='visualization', action='display_by_username_and_slug' ) webapp.add_route( '/search', controller='search', action='index' ) - # ================ - # ===== API ===== - # ================ + # TODO: Refactor above routes into external method to allow testing in + # isolation as well. + populate_api_routes( webapp, app ) + # ==== Done + # Indicate that all configuration settings have been provided + webapp.finalize_config() + + # Wrap the webapp in some useful middleware + if kwargs.get( 'middleware', True ): + webapp = wrap_in_middleware( webapp, global_conf, **kwargs ) + if asbool( kwargs.get( 'static_enabled', True ) ): + webapp = wrap_in_static( webapp, global_conf, plugin_frameworks=[ app.visualizations_registry ], **kwargs ) + #webapp = wrap_in_static( webapp, global_conf, plugin_frameworks=None, **kwargs ) + if asbool(kwargs.get('pack_scripts', False)): + pack_scripts() + # Close any pooled database connections before forking + try: + galaxy.model.mapping.metadata.engine.connection_provider._pool.dispose() + except: + pass + # Close any pooled database connections before forking + try: + galaxy.model.tool_shed_install.mapping.metadata.engine.connection_provider._pool.dispose() + except: + pass + + # Return + return webapp + + +def populate_api_routes( webapp, app ): webapp.add_api_controllers( 'galaxy.webapps.galaxy.api', app ) valid_history_contents_types = [ @@ -350,32 +378,6 @@ def app_factory( global_conf, **kwargs ): controller="metrics", action="create", conditions=dict( method=["POST"] ) ) - # ==== Done - # Indicate that all configuration settings have been provided - webapp.finalize_config() - - # Wrap the webapp in some useful middleware - if kwargs.get( 'middleware', True ): - webapp = wrap_in_middleware( webapp, global_conf, **kwargs ) - if asbool( kwargs.get( 'static_enabled', True ) ): - webapp = wrap_in_static( webapp, global_conf, plugin_frameworks=[ app.visualizations_registry ], **kwargs ) - #webapp = wrap_in_static( webapp, global_conf, plugin_frameworks=None, **kwargs ) - if asbool(kwargs.get('pack_scripts', False)): - pack_scripts() - # Close any pooled database connections before forking - try: - galaxy.model.mapping.metadata.engine.connection_provider._pool.dispose() - except: - pass - # Close any pooled database connections before forking - try: - galaxy.model.tool_shed_install.mapping.metadata.engine.connection_provider._pool.dispose() - except: - pass - - # Return - return webapp - def pack_scripts(): from glob import glob from subprocess import call diff --git a/test/unit/test_routes.py b/test/unit/test_routes.py new file mode 100644 index 00000000000..196dada2597 --- /dev/null +++ b/test/unit/test_routes.py @@ -0,0 +1,81 @@ +#import routes + +# Unused but adds some stuff to routes.Mapper. +from galaxy.web.framework import WebApplication +from galaxy.webapps.galaxy import buildapp as galaxy_buildapp + +from galaxy.web import url_for +from galaxy.util.bunch import Bunch + + +class TestWebapp( WebApplication ): + + def _instantiate_controller( self, type, app ): + # Stub out all actual controllers - just want to test routes. + return object() + + def assert_maps( self, url, method="GET", **parts ): + map_result = self.mapper.match( url, environ={"REQUEST_METHOD": method } ) + for key, expected_value in parts.iteritems(): + actual_value = map_result[ key ] + if actual_value != expected_value: + message = "Problem mapping route [%s], part %s expected value [%s] but obtained [%s]" + raise AssertionError(message % ( url, key, expected_value, actual_value ) ) + + +def test_galaxy_routes( ): + test_config = Bunch( template_path="/tmp", template_cache="/tmp" ) + app = Bunch( config=test_config, security=object(), trace_logger=None ) + test_webapp = TestWebapp( app ) + + galaxy_buildapp.populate_api_routes( test_webapp, app ) + + assert_url_is( url_for( "api_key_retrieval" ), "/api/authenticate/baseauth" ) + + # Test previously problematic tool ids with slashes. + test_webapp.assert_maps( + "/api/tools/testtoolshed.g2.bx.psu.edu/devteam/tool1", + controller="tools", + id="testtoolshed.g2.bx.psu.edu/devteam/tool1" + ) + + test_webapp.assert_maps( + "/api/datatypes/sniffers", + controller="datatypes", + action="sniffers" + ) + + test_webapp.assert_maps( + "/api/histories/123/contents/456", + controller="history_contents", + action="show" + ) + + test_webapp.assert_maps( + "/api/histories/123/contents/456", + method="PUT", + controller="history_contents", + action="update", + ) + + # Test differeniating datasets and datasets collections + # in history contents. + test_webapp.assert_maps( + "/api/histories/123/contents/datasets/456", + method="PUT", + controller="history_contents", + action="update", + type="dataset" + ) + + test_webapp.assert_maps( + "/api/histories/123/contents/dataset_collections/456", + method="PUT", + controller="history_contents", + action="update", + type="dataset_collection" + ) + + +def assert_url_is( actual, expected ): + assert actual == expected, "Expected URL [%s] but obtained [%s]" % ( expected, actual )