heckyel/mediagoblin
0
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix some unit tests and bugs

Browse Source 
This fixes a lot of the issues with the LocalUser changes that were
merged recently. There was a problem where the attributes of LocalUser
were not being eagerly loaded and because the Session was detached an
exception was being raised when they were accessed.

This also fixes some typo's which were introduced.

Finally this adds a temporary fix for a potential SQLAlchemy bug, this
is a bug where doing:

    User.query.filter(LocalUser.username == "some_username").first()

does NOT yeild a user with the username "some_username" but all users
on the site. The temp fix is to just query the LocalUser, this should
be resolved when bug is confirmed and fixed upstream.
This commit is contained in:
Jessica Tallon
2015-08-24 18:28:41 +02:00
parent e9bb5879f7
commit b4997540dc
25 changed files with 87 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
mediagoblin/tests/__init__.py
View File

@@ -44,7 +44,7 @@ class MGClientTestCase:
fixture_add_user(username, **options)
def user(self, username):
return User.query.filter(LocalUser.username==username).first()
return LocalUser.query.filter(LocalUser.username==username).first()
def _do_request(self, url, *context_keys, **kwargs):
template.clear_test_template_context()

8
mediagoblin/tests/test_auth.py
View File

@@ -98,8 +98,8 @@ def test_register_views(test_app):
assert 'mediagoblin/user_pages/user_nonactive.html' in template.TEMPLATE_TEST_CONTEXT
## Make sure user is in place
new_user = mg_globals.database.User.query.filter(
LocalUser.usrname==u'angrygirl'
new_user = mg_globals.database.LocalUser.query.filter(
LocalUser.username==u'angrygirl'
).first()
assert new_user
@@ -138,7 +138,7 @@ def test_register_views(test_app):
# assert context['verification_successful'] == True
# TODO: Would be good to test messages here when we can do so...
new_user = mg_globals.database.User.query.filter(
new_user = mg_globals.database.LocalUser.query.filter(
LocalUser.username==u'angrygirl'
).first()
assert new_user
@@ -151,7 +151,7 @@ def test_register_views(test_app):
'mediagoblin/user_pages/user.html']
# assert context['verification_successful'] == True
# TODO: Would be good to test messages here when we can do so...
new_user = mg_globals.database.User.query.filter(
new_user = mg_globals.database.LocalUser.query.filter(
LocalUser.username==u'angrygirl'
).first()
assert new_user

4
mediagoblin/tests/test_basic_auth.py
View File

@@ -88,7 +88,7 @@ def test_change_password(test_app):
assert urlparse.urlsplit(res.location)[2] == '/edit/account/'
# test_user has to be fetched again in order to have the current values
test_user = User.query.filter(LocalUser.username==u'chris').first()
test_user = LocalUser.query.filter(LocalUser.username==u'chris').first()
assert auth_tools.bcrypt_check_password('123456', test_user.pw_hash)
# test that the password cannot be changed if the given
@@ -100,5 +100,5 @@ def test_change_password(test_app):
'new_password': '098765',
})
test_user = User.query.filter(LocalUser.username==u'chris').first()
test_user = LocalUser.query.filter(LocalUser.username==u'chris').first()
assert not auth_tools.bcrypt_check_password('098765', test_user.pw_hash)

12
mediagoblin/tests/test_edit.py
View File

@@ -44,12 +44,12 @@ class TestUserEdit(object):
self.login(test_app)
# Make sure user exists
assert User.query.filter(LocalUser.username=u'chris').first()
assert LocalUser.query.filter(LocalUser.username==u'chris').first()
res = test_app.post('/edit/account/delete/', {'confirmed': 'y'})
# Make sure user has been deleted
assert User.query.filter(LocalUser.username==u'chris').first() == None
assert LocalUser.query.filter(LocalUser.username==u'chris').first() == None
#TODO: make sure all corresponding items comments etc have been
# deleted too. Perhaps in submission test?
@@ -79,7 +79,7 @@ class TestUserEdit(object):
'bio': u'I love toast!',
'url': u'http://dustycloud.org/'})
test_user = User.query.filter(LocalUser.username==u'chris').first()
test_user = LocalUser.query.filter(LocalUser.username==u'chris').first()
assert test_user.bio == u'I love toast!'
assert test_user.url == u'http://dustycloud.org/'
@@ -159,10 +159,10 @@ class TestUserEdit(object):
assert urlparse.urlsplit(res.location)[2] == '/'
# Email shouldn't be saved
email_in_db = mg_globals.database.User.query.filter(
email_in_db = mg_globals.database.LocalUser.query.filter(
LocalUser.email=='new@example.com'
).first()
email = User.query.filter(LocalUser.username=='chris').first().email
email = LocalUser.query.filter(LocalUser.username=='chris').first().email
assert email_in_db is None
assert email == 'chris@example.com'
@@ -173,7 +173,7 @@ class TestUserEdit(object):
res.follow()
# New email saved?
email = User.query.filter(LocalUser.username=='chris').first().email
email = LocalUser.query.filter(LocalUser.username=='chris').first().email
assert email == 'new@example.com'
# test changing the url inproperly

2
mediagoblin/tests/test_ldap.py
View File

@@ -115,7 +115,7 @@ def test_ldap_plugin(ldap_plugin_app):
ldap_plugin_app.get('/auth/logout/')
# Get user and detach from session
test_user = mg_globals.database.User.query.filter(
test_user = mg_globals.database.LocalUser.query.filter(
LocalUser.username==u'chris'
).first()
Session.expunge(test_user)

4
mediagoblin/tests/test_modelmethods.py
View File

@@ -168,9 +168,9 @@ class TestUserHasPrivilege:
privileges=[u'admin',u'moderator',u'active'])
fixture_add_user(u'aeva',
privileges=[u'moderator',u'active'])
self.natalie_user = User.query.filter(
self.natalie_user = LocalUser.query.filter(
LocalUser.username==u'natalie').first()
self.aeva_user = User.query.filter(
self.aeva_user = LocalUser.query.filter(
LocalUser.username==u'aeva').first()
def test_privilege_added_correctly(self, test_app):

6
mediagoblin/tests/test_moderation.py
View File

@@ -47,9 +47,9 @@ class TestModerationViews:
self.query_for_users()
def query_for_users(self):
self.admin_user = User.query.filter(LocalUser.username==u'admin').first()
self.mod_user = User.query.filter(LocalUser.username==u'moderator').first()
self.user = User.query.filter(LocalUser.username==u'regular').first()
self.admin_user = LocalUser.query.filter(LocalUser.username==u'admin').first()
self.mod_user = LocalUser.query.filter(LocalUser.username==u'moderator').first()
self.user = LocalUser.query.filter(LocalUser.username==u'regular').first()
def do_post(self, data, *context_keys, **kwargs):
url = kwargs.pop('url', '/submit/')

2
mediagoblin/tests/test_openid.py
View File

@@ -192,7 +192,7 @@ class TestOpenIDPlugin(object):
openid_plugin_app.get('/auth/logout')
# Get user and detach from session
test_user = mg_globals.database.User.query.filter(
test_user = mg_globals.database.LocalUser.query.filter(
LocalUser.username==u'chris'
).first()
Session.expunge(test_user)

8
mediagoblin/tests/test_persona.py
View File

@@ -27,8 +27,8 @@ import six.moves.urllib.parse as urlparse
pytest.importorskip("requests")
from mediagoblin import mg_globals
from mediagoblin.db.base import Session, LocalUser
from mediagoblin.db.models import Privilege
from mediagoblin.db.base import Session
from mediagoblin.db.models import Privilege, LocalUser
from mediagoblin.tests.tools import get_app
from mediagoblin.tools import template
@@ -117,14 +117,14 @@ class TestPersonaPlugin(object):
persona_plugin_app.get('/auth/logout/')
# Get user and detach from session
test_user = mg_globals.database.User.query.filter(
test_user = mg_globals.database.LocalUser.query.filter(
LocalUser.username==u'chris'
).first()
active_privilege = Privilege.query.filter(
Privilege.privilege_name==u'active').first()
test_user.all_privileges.append(active_privilege)
test_user.save()
test_user = mg_globals.database.User.query.filter(
test_user = mg_globals.database.LocalUser.query.filter(
LocalUser.username==u'chris'
).first()
Session.expunge(test_user)

6
mediagoblin/tests/test_privileges.py
View File

@@ -64,9 +64,9 @@ class TestPrivilegeFunctionality:
return response, context_data
def query_for_users(self):
self.admin_user = User.query.filter(LocalUser.username==u'alex').first()
self.mod_user = User.query.filter(LocalUser.username==u'meow').first()
self.user = User.query.filter(LocalUser.username==u'natalie').first()
self.admin_user = LocalUser.query.filter(LocalUser.username==u'alex').first()
self.mod_user = LocalUser.query.filter(LocalUser.username==u'meow').first()
self.user = LocalUser.query.filter(LocalUser.username==u'natalie').first()
def testUserBanned(self):
self.login(u'natalie')

6
mediagoblin/tests/test_reporting.py
View File

@@ -20,7 +20,7 @@ import six
from mediagoblin.tools import template
from mediagoblin.tests.tools import (fixture_add_user, fixture_media_entry,
fixture_add_comment, fixture_add_comment_report)
from mediagoblin.db.models import (MediaReport, CommentReport, User, LocalUser
from mediagoblin.db.models import (MediaReport, CommentReport, User, LocalUser,
MediaComment)
@@ -56,8 +56,8 @@ class TestReportFiling:
return response, context_data
def query_for_users(self):
return (User.query.filter(LocalUser.username==u'allie').first(),
User.query.filter(LocalUser.username==u'natalie').first())
return (LocalUser.query.filter(LocalUser.username==u'allie').first(),
LocalUser.query.filter(LocalUser.username==u'natalie').first())
def testMediaReports(self):
self.login(u'allie')

2
mediagoblin/tests/test_submission.py
View File

@@ -72,7 +72,7 @@ class TestSubmission:
#### totally stupid.
#### Also if we found a way to make this run it should be a
#### property.
return User.query.filter(LocalUser.username==u'chris').first()
return LocalUser.query.filter(LocalUser.username==u'chris').first()
def login(self):
self.test_app.post(

11
mediagoblin/tests/tools.py
View File

@@ -176,7 +176,7 @@ def assert_db_meets_expected(db, expected):
def fixture_add_user(username=u'chris', password=u'toast',
privileges=[], wants_comment_notification=True):
# Reuse existing user or create a new one
test_user = User.query.filter(LocalUser.username==username).first()
test_user = LocalUser.query.filter(LocalUser.username==username).first()
if test_user is None:
test_user = LocalUser()
test_user.username = username
@@ -190,8 +190,11 @@ def fixture_add_user(username=u'chris', password=u'toast',
test_user.all_privileges.append(query.one())
test_user.save()
# Reload
test_user = User.query.filter(LocalUser.username==username).first()
# Reload - The `with_polymorphic` needs to be there to eagerly load
# the attributes on the LocalUser as this can't be done post detachment.
user_query = LocalUser.query.with_polymorphic(LocalUser)
test_user = user_query.filter(LocalUser.username==username).first()
# ... and detach from session:
Session.expunge(test_user)
@@ -201,7 +204,7 @@ def fixture_add_user(username=u'chris', password=u'toast',
def fixture_comment_subscription(entry, notify=True, send_email=None):
if send_email is None:
uploader = User.query.filter_by(id=entry.uploader).first()
uploader = LocalUser.query.filter_by(id=entry.uploader).first()
send_email = uploader.wants_comment_notification
cs = CommentSubscription(