From d693f6bd867871ba084f0da0ff8ba5a667174fc7 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Mon, 26 Mar 2012 11:14:11 -0500 Subject: [PATCH 1/8] SQL based tests and refactored Celery setup stuff - Changed config files of test configs to use SQL - Updated celery initialization tools, factored them to be able to use the "big instance" application stuff --- mediagoblin/gmg_commands/dbupdate.py | 8 +++- mediagoblin/init/celery/__init__.py | 55 +++++++++++++++++++------- mediagoblin/tests/test_mgoblin_app.ini | 6 ++- mediagoblin/tests/tools.py | 26 ++++++------ 4 files changed, 62 insertions(+), 33 deletions(-) diff --git a/mediagoblin/gmg_commands/dbupdate.py b/mediagoblin/gmg_commands/dbupdate.py index 27698170..5415b997 100644 --- a/mediagoblin/gmg_commands/dbupdate.py +++ b/mediagoblin/gmg_commands/dbupdate.py @@ -65,14 +65,13 @@ def gather_database_data(media_types): return managed_dbdata -def dbupdate(args): +def run_dbupdate(app_config): """ Initialize or migrate the database as specified by the config file. Will also initialize or migrate all extensions (media types, and in the future, plugins) """ - globa_config, app_config = setup_global_and_app_config(args.conf_file) # Gather information from all media managers / projects dbdatas = gather_database_data(app_config['media_types']) @@ -87,3 +86,8 @@ def dbupdate(args): for dbdata in dbdatas: migration_manager = dbdata.make_migration_manager(Session()) migration_manager.init_or_migrate() + + +def dbupdate(args): + global_config, app_config = setup_global_and_app_config(args.conf_file) + run_dbupdate(app_config) diff --git a/mediagoblin/init/celery/__init__.py b/mediagoblin/init/celery/__init__.py index 67b020c3..fc595ea7 100644 --- a/mediagoblin/init/celery/__init__.py +++ b/mediagoblin/init/celery/__init__.py @@ -17,28 +17,18 @@ import os import sys +from celery import Celery + MANDATORY_CELERY_IMPORTS = ['mediagoblin.processing.task'] DEFAULT_SETTINGS_MODULE = 'mediagoblin.init.celery.dummy_settings_module' -def setup_celery_from_config(app_config, global_config, - settings_module=DEFAULT_SETTINGS_MODULE, - force_celery_always_eager=False, - set_environ=True): +def get_celery_settings_dict(app_config, global_config, + force_celery_always_eager=False): """ - Take a mediagoblin app config and try to set up a celery settings - module from this. - - Args: - - app_config: the application config section - - global_config: the entire ConfigObj loaded config, all sections - - settings_module: the module to populate, as a string - - force_celery_always_eager: whether or not to force celery into - always eager mode; good for development and small installs - - set_environ: if set, this will CELERY_CONFIG_MODULE to the - settings_module + Get a celery settings dictionary from reading the config """ if 'celery' in global_config: celery_conf = global_config['celery'] @@ -61,6 +51,41 @@ def setup_celery_from_config(app_config, global_config, celery_settings['CELERY_ALWAYS_EAGER'] = True celery_settings['CELERY_EAGER_PROPAGATES_EXCEPTIONS'] = True + return celery_settings + + +def setup_celery_app(app_config, global_config, + settings_module=DEFAULT_SETTINGS_MODULE, + force_celery_always_eager=False): + """ + Setup celery without using terrible setup-celery-module hacks. + """ + celery_settings = get_celery_settings_dict( + app_config, global_config, force_celery_always_eager) + celery_app = Celery() + celery_app.config_from_object(celery_settings) + + +def setup_celery_from_config(app_config, global_config, + settings_module=DEFAULT_SETTINGS_MODULE, + force_celery_always_eager=False, + set_environ=True): + """ + Take a mediagoblin app config and try to set up a celery settings + module from this. + + Args: + - app_config: the application config section + - global_config: the entire ConfigObj loaded config, all sections + - settings_module: the module to populate, as a string + - force_celery_always_eager: whether or not to force celery into + always eager mode; good for development and small installs + - set_environ: if set, this will CELERY_CONFIG_MODULE to the + settings_module + """ + celery_settings = get_celery_settings_dict( + app_config, global_config, force_celery_always_eager) + __import__(settings_module) this_module = sys.modules[settings_module] diff --git a/mediagoblin/tests/test_mgoblin_app.ini b/mediagoblin/tests/test_mgoblin_app.ini index 01bf0972..fd610ecc 100644 --- a/mediagoblin/tests/test_mgoblin_app.ini +++ b/mediagoblin/tests/test_mgoblin_app.ini @@ -2,7 +2,9 @@ direct_remote_path = /test_static/ email_sender_address = "notice@mediagoblin.example.org" email_debug_mode = true -db_name = __mediagoblin_tests__ + +# Use an in-memory database +sql_engine = "sqlite:///%(here)s/test_user_dev/mediagoblin.db" # tag parsing tags_max_length = 50 @@ -27,3 +29,5 @@ lock_dir = %(here)s/test_user_dev/beaker/cache/lock [celery] CELERY_ALWAYS_EAGER = true +CELERY_RESULT_DBURI = "sqlite:///%(here)s/test_user_dev/celery.db" +BROKER_HOST = "sqlite:///%(here)s/test_user_dev/kombu.db" diff --git a/mediagoblin/tests/tools.py b/mediagoblin/tests/tools.py index 7cf355b0..a99173e8 100644 --- a/mediagoblin/tests/tools.py +++ b/mediagoblin/tests/tools.py @@ -26,8 +26,11 @@ from mediagoblin.tools import testing from mediagoblin.init.config import read_mediagoblin_config from mediagoblin.decorators import _make_safe from mediagoblin.db.open import setup_connection_and_db_from_config +from mediagoblin.db.sql.base import Session from mediagoblin.meddleware import BaseMeddleware from mediagoblin.auth.lib import bcrypt_gen_password_hash +from mediagoblin.gmg_commands.dbupdate import run_dbupdate +from mediagoblin.init.celery import setup_celery_app MEDIAGOBLIN_TEST_DB_NAME = u'__mediagoblin_tests__' @@ -125,26 +128,19 @@ def get_test_app(dump_old_app=True): global_config, validation_result = read_mediagoblin_config(TEST_APP_CONFIG) app_config = global_config['mediagoblin'] - # Wipe database - # @@: For now we're dropping collections, but we could also just - # collection.remove() ? - connection, db = setup_connection_and_db_from_config(app_config) - assert db.name == MEDIAGOBLIN_TEST_DB_NAME - - collections_to_wipe = [ - collection - for collection in db.collection_names() - if not collection.startswith('system.')] - - for collection in collections_to_wipe: - db.drop_collection(collection) - - # TODO: Drop and recreate indexes + # Run database setup/migrations + run_dbupdate(app_config) # setup app and return test_app = loadapp( 'config:' + TEST_SERVER_CONFIG) + Session.rollback() + Session.remove() + + # Re-setup celery + setup_celery_app(app_config, global_config) + # Insert the TestingMeddleware, which can do some # sanity checks on every request/response. # Doing it this way is probably not the cleanest way. From 8e96bcbc78b453993250466e69bf156a240f8c6a Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Mon, 26 Mar 2012 11:31:09 -0500 Subject: [PATCH 2/8] Suggest we move to an in-memory database ;) --- mediagoblin/tests/test_mgoblin_app.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mediagoblin/tests/test_mgoblin_app.ini b/mediagoblin/tests/test_mgoblin_app.ini index fd610ecc..3b979ff7 100644 --- a/mediagoblin/tests/test_mgoblin_app.ini +++ b/mediagoblin/tests/test_mgoblin_app.ini @@ -3,7 +3,7 @@ direct_remote_path = /test_static/ email_sender_address = "notice@mediagoblin.example.org" email_debug_mode = true -# Use an in-memory database +# TODO: Switch to using an in-memory database sql_engine = "sqlite:///%(here)s/test_user_dev/mediagoblin.db" # tag parsing From 8ea37380bdab6ed19177059de0ad270ad20d485a Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Mon, 26 Mar 2012 11:50:36 -0500 Subject: [PATCH 3/8] Remove the user_dev directory on tests ending (We used to remove the mongo db on tests ending...) --- mediagoblin/tests/__init__.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/mediagoblin/tests/__init__.py b/mediagoblin/tests/__init__.py index 15a5add0..4e84914a 100644 --- a/mediagoblin/tests/__init__.py +++ b/mediagoblin/tests/__init__.py @@ -14,10 +14,12 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -from mediagoblin import mg_globals +import os +import shutil +from mediagoblin import mg_globals from mediagoblin.tests.tools import ( - MEDIAGOBLIN_TEST_DB_NAME, suicide_if_bad_celery_environ) + TEST_USER_DEV, suicide_if_bad_celery_environ) def setup_package(): @@ -25,8 +27,6 @@ def setup_package(): def teardown_package(): - if ((mg_globals.db_connection - and mg_globals.database.name == MEDIAGOBLIN_TEST_DB_NAME)): - print "Killing db ..." - mg_globals.db_connection.drop_database(MEDIAGOBLIN_TEST_DB_NAME) - print "... done" + # Remove and reinstall user_dev directories + if os.path.exists(TEST_USER_DEV): + shutil.rmtree(TEST_USER_DEV) From 38877794e73ab9c23e11bf5a22263c1d1d5b083c Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Mon, 26 Mar 2012 11:59:34 -0500 Subject: [PATCH 4/8] TestSubission's tag check stuff passing now --- mediagoblin/tests/test_submission.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mediagoblin/tests/test_submission.py b/mediagoblin/tests/test_submission.py index 1f56779e..788dfacf 100644 --- a/mediagoblin/tests/test_submission.py +++ b/mediagoblin/tests/test_submission.py @@ -140,9 +140,11 @@ class TestSubmission: context = template.TEMPLATE_TEST_CONTEXT['mediagoblin/user_pages/user.html'] request = context['request'] media = request.db.MediaEntry.find({'title': 'Balanced Goblin'})[0] - assert_equal(media.tags, - [{'name': u'yin', 'slug': u'yin'}, - {'name': u'yang', 'slug': u'yang'}]) + assert media.tags[0]['name'] == u'yin' + assert media.tags[0]['slug'] == u'yin' + + assert media.tags[1]['name'] == u'yang' + assert media.tags[1]['slug'] == u'yang' # Test tags that are too long # --------------- From 37ef4c66b1bc5841b68bf25f06feb7d863ff89f5 Mon Sep 17 00:00:00 2001 From: Elrond Date: Sun, 26 Feb 2012 15:15:10 +0100 Subject: [PATCH 5/8] Reload and detach the test user. The code often needs to know some fields of the test user even after doing some sql and stuff. The solultion is to reload it and properly detach it from its Session. That way all its fields are available and the whole thing is not connected to a session. It feels like a normal object. --- mediagoblin/tests/tools.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/mediagoblin/tests/tools.py b/mediagoblin/tests/tools.py index a99173e8..5b4e3746 100644 --- a/mediagoblin/tests/tools.py +++ b/mediagoblin/tests/tools.py @@ -212,4 +212,11 @@ def fixture_add_user(username = u'chris', password = 'toast', test_user.save() + # Reload + test_user = mg_globals.database.User.find_one({'username': username}) + + # ... and detach from session: + from mediagoblin.db.sql.base import Session + Session.expunge(test_user) + return test_user From f4162cb640646cb8f67809e0f70585c603452ff2 Mon Sep 17 00:00:00 2001 From: Elrond Date: Sun, 26 Feb 2012 15:14:50 +0100 Subject: [PATCH 6/8] Reload the user for current values. This might not be needed, but it helped at one point. --- mediagoblin/tests/test_auth.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mediagoblin/tests/test_auth.py b/mediagoblin/tests/test_auth.py index 3a33c66c..8f988af3 100644 --- a/mediagoblin/tests/test_auth.py +++ b/mediagoblin/tests/test_auth.py @@ -269,6 +269,7 @@ def test_register_views(test_app): ## Try using an expired token to change password, shouldn't work template.clear_test_template_context() + new_user = mg_globals.database.User.find_one({'username': 'happygirl'}) real_token_expiration = new_user.fp_token_expire new_user.fp_token_expire = datetime.datetime.now() new_user.save() From ce29c140ed90fe0e19ed1b68ca8f7cf27aa03e8e Mon Sep 17 00:00:00 2001 From: Elrond Date: Sun, 1 Apr 2012 21:46:36 +0200 Subject: [PATCH 7/8] Finally enable SQL for everybody! This switches the whole source code over to use sql instead of mongodb. It's a pretty easy change, but changes nearly the complete way things work. Hopefully everythong works! --- .gitignore | 1 - mediagoblin/db/sql_switch.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 mediagoblin/db/sql_switch.py diff --git a/.gitignore b/.gitignore index a15a5697..6875d78a 100644 --- a/.gitignore +++ b/.gitignore @@ -20,7 +20,6 @@ /celery.db /kombu.db /server-log.txt -/mediagoblin/db/sql_switch.py # Tests /mediagoblin/tests/user_dev/ diff --git a/mediagoblin/db/sql_switch.py b/mediagoblin/db/sql_switch.py new file mode 100644 index 00000000..571adbdb --- /dev/null +++ b/mediagoblin/db/sql_switch.py @@ -0,0 +1 @@ +use_sql = True From bc27a100fc05cea72c47d8ae446454d347d0a0ff Mon Sep 17 00:00:00 2001 From: Elrond Date: Sun, 1 Apr 2012 22:02:06 +0200 Subject: [PATCH 8/8] Fix unit tests for sql: cache media_id. Attributes of SQLAlchemy objects get "lost". So "cache" them locally in the code. This is really the simple explanation for some scarry sqlalchemy details. --- mediagoblin/tests/test_submission.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mediagoblin/tests/test_submission.py b/mediagoblin/tests/test_submission.py index 788dfacf..9b503f4f 100644 --- a/mediagoblin/tests/test_submission.py +++ b/mediagoblin/tests/test_submission.py @@ -183,8 +183,9 @@ class TestSubmission: assert_true(media) # Add a comment, so we can test for its deletion later. + media_id = media.id get_comments = lambda: list( - request.db.MediaComment.find({'media_entry': media._id})) + request.db.MediaComment.find({'media_entry': media_id})) assert_false(get_comments()) response = self.test_app.post( request.urlgen('mediagoblin.user_pages.media_post_comment', @@ -200,7 +201,7 @@ class TestSubmission: request.urlgen('mediagoblin.user_pages.media_confirm_delete', # No work: user=media.uploader().username, user=self.test_user.username, - media=media._id), + media=media_id), # no value means no confirm {})