diff --git a/admin/sql/clear_tables.sql b/admin/sql/clear_tables.sql new file mode 100644 index 000000000..582ed6d09 --- /dev/null +++ b/admin/sql/clear_tables.sql @@ -0,0 +1,13 @@ +DELETE FROM comment_revision; +DELETE FROM comment; +DELETE FROM vote; +DELETE FROM spam_report; +DELETE FROM revision; +DELETE FROM oauth_grant; +DELETE FROM oauth_token; +DELETE FROM oauth_client; +DELETE FROM moderation_log; +DELETE FROM review; +DELETE FROM "user"; +DELETE FROM license; +DELETE FROM avg_rating; diff --git a/critiquebrainz/data/testing.py b/critiquebrainz/data/testing.py index 027e57d3c..b4d7c742a 100644 --- a/critiquebrainz/data/testing.py +++ b/critiquebrainz/data/testing.py @@ -2,7 +2,7 @@ from flask_testing import TestCase -from critiquebrainz.data.utils import create_all, drop_tables, drop_types +from critiquebrainz.data.utils import create_all, drop_tables, drop_types, clear_tables from critiquebrainz.frontend import create_app @@ -21,6 +21,4 @@ def tearDown(self): @staticmethod def reset_db(): - drop_tables() - drop_types() - create_all() + clear_tables() diff --git a/critiquebrainz/data/utils.py b/critiquebrainz/data/utils.py index 4d36a8104..1924baad1 100644 --- a/critiquebrainz/data/utils.py +++ b/critiquebrainz/data/utils.py @@ -30,6 +30,10 @@ def drop_types(): db.run_sql_script(os.path.join(ADMIN_SQL_DIR, 'drop_types.sql')) +def clear_tables(): + db.run_sql_script(os.path.join(ADMIN_SQL_DIR, 'clear_tables.sql')) + + def explode_db_uri(uri): """Extracts database connection info from the URI. diff --git a/critiquebrainz/db/users.py b/critiquebrainz/db/users.py index f6e7a9996..e0e2dc315 100644 --- a/critiquebrainz/db/users.py +++ b/critiquebrainz/db/users.py @@ -4,7 +4,6 @@ import sqlalchemy from critiquebrainz import db -from critiquebrainz.db import revision as db_revision USER_GET_COLUMNS = [ @@ -305,6 +304,7 @@ def list_users(limit=None, offset=0): result = connection.execute(sqlalchemy.text(""" SELECT {columns} FROM "user" + ORDER BY musicbrainz_row_id LIMIT :limit OFFSET :offset """.format(columns=','.join(USER_GET_COLUMNS))), { @@ -357,6 +357,7 @@ def has_voted(user_id, review_id): Returns: (bool): True if has voted else False. """ + from critiquebrainz.db import revision as db_revision last_revision = db_revision.get(review_id, limit=1)[0] with db.engine.connect() as connection: result = connection.execute(sqlalchemy.text(""" diff --git a/critiquebrainz/frontend/external/__init__.py b/critiquebrainz/frontend/external/__init__.py index e401f7319..5ea2d1124 100644 --- a/critiquebrainz/frontend/external/__init__.py +++ b/critiquebrainz/frontend/external/__init__.py @@ -10,6 +10,7 @@ from critiquebrainz.frontend.external.entities import get_entity_by_id, get_multiple_entities + class MBDataAccess(object): """A data access object which switches between database get methods or development versions This is useful because we won't show a review if we cannot find its metadata in the @@ -51,6 +52,7 @@ def get_multiple_entities(self): ctx.get_multiple_entities = self.get_multiple_entities_method return ctx.get_multiple_entities + mbstore = MBDataAccess() diff --git a/critiquebrainz/frontend/testing.py b/critiquebrainz/frontend/testing.py index a91a9688c..e0f482880 100644 --- a/critiquebrainz/frontend/testing.py +++ b/critiquebrainz/frontend/testing.py @@ -1,34 +1,17 @@ import os -from critiquebrainz.ws.oauth import oauth - -from flask_testing import TestCase -from critiquebrainz.data.utils import create_all, drop_tables, drop_types from critiquebrainz.frontend import create_app +from critiquebrainz.testing import ServerTestCase +from critiquebrainz.ws.oauth import oauth -class FrontendTestCase(TestCase): +class FrontendTestCase(ServerTestCase): - def create_app(self): - app = create_app(config_path=os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..', 'test_config.py')) + @classmethod + def create_app(cls): + app = create_app( + config_path=os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..', 'test_config.py') + ) oauth.init_app(app) + app.config['TESTING'] = True return app - - def setUp(self): - self.reset_db() - # TODO(roman): Add stuff form fixtures. - - def tearDown(self): - pass - - @staticmethod - def reset_db(): - drop_tables() - drop_types() - create_all() - - def temporary_login(self, user): - """Based on: http://stackoverflow.com/a/16238537.""" - with self.client.session_transaction() as session: - session['_user_id'] = user.id - session['_fresh'] = True diff --git a/critiquebrainz/frontend/views/review.py b/critiquebrainz/frontend/views/review.py index 6440376b4..60fa0aa70 100644 --- a/critiquebrainz/frontend/views/review.py +++ b/critiquebrainz/frontend/views/review.py @@ -1,6 +1,5 @@ from math import ceil -from brainzutils.musicbrainz_db.exceptions import NoDataFoundException from flask import Blueprint, render_template, request, redirect, url_for, jsonify from flask_babel import gettext, get_locale, lazy_gettext from flask_login import login_required, current_user diff --git a/critiquebrainz/frontend/views/test/test_artist.py b/critiquebrainz/frontend/views/test/test_artist.py index 794192baf..c293d96b7 100644 --- a/critiquebrainz/frontend/views/test/test_artist.py +++ b/critiquebrainz/frontend/views/test/test_artist.py @@ -1,9 +1,9 @@ from unittest import mock -from critiquebrainz.db.user import User from critiquebrainz.frontend.testing import FrontendTestCase from critiquebrainz.db import users as db_users + def return_release_groups(*, artist_id, release_types=None, limit=None, offset=None): # pylint: disable=unused-argument if release_types == ['ep']: @@ -30,6 +30,7 @@ def return_release_groups(*, artist_id, release_types=None, limit=None, offset=N class ArtistViewsTestCase(FrontendTestCase): def setUp(self): + from critiquebrainz.db.user import User super(ArtistViewsTestCase, self).setUp() self.reviewer = User(db_users.get_or_create( 1, "Reviewer", new_user_data={"display_name": u"Reviewer"} diff --git a/critiquebrainz/frontend/views/test/test_oauth.py b/critiquebrainz/frontend/views/test/test_oauth.py index 998d47d0f..e12e91be8 100644 --- a/critiquebrainz/frontend/views/test/test_oauth.py +++ b/critiquebrainz/frontend/views/test/test_oauth.py @@ -9,6 +9,7 @@ class OauthTestCase(FrontendTestCase): def setUp(self): + super().setUp() from critiquebrainz.db.user import User self.user = User(db_users.get_or_create(2, "9371e5c7-5995-4471-a5a9-33481f897f9c", new_user_data={ "display_name": u"User", diff --git a/critiquebrainz/frontend/views/test/test_review.py b/critiquebrainz/frontend/views/test/test_review.py index 3c793b6bd..c7ab6abc0 100644 --- a/critiquebrainz/frontend/views/test/test_review.py +++ b/critiquebrainz/frontend/views/test/test_review.py @@ -2,7 +2,7 @@ from unittest.mock import patch from urllib.parse import urlparse -from flask import current_app, url_for +from flask import url_for import critiquebrainz.db.license as db_license import critiquebrainz.db.review as db_review diff --git a/critiquebrainz/frontend/views/test/test_user.py b/critiquebrainz/frontend/views/test/test_user.py index 901eb6310..91452b9df 100644 --- a/critiquebrainz/frontend/views/test/test_user.py +++ b/critiquebrainz/frontend/views/test/test_user.py @@ -12,7 +12,9 @@ def setUp(self): self.user = User(db_users.get_or_create(1, "Tester", new_user_data={ "display_name": u"Tester", })) - self.hacker = User(db_users.create(musicbrainz_row_id = 2, display_name = u"Hacker")) + self.hacker = User(db_users.get_or_create(2, "Hacker", new_user_data={ + "display_name": u"Hacker", + })) self.admin = User(db_users.get_or_create(3, "Admin", new_user_data={ "display_name": u"Admin", })) diff --git a/critiquebrainz/testing.py b/critiquebrainz/testing.py new file mode 100644 index 000000000..d1b6a5225 --- /dev/null +++ b/critiquebrainz/testing.py @@ -0,0 +1,153 @@ +import unittest +from urllib import parse + +from flask import template_rendered, message_flashed, g + +from critiquebrainz.data.utils import create_all, drop_tables, drop_types, clear_tables + + +class ServerTestCase(unittest.TestCase): + """ TestCase for Flask App tests, most of the code here has been borrowed from flask_testing.TestCase + (https://github.com/jarus/flask-testing/blob/5107691011fa891835c01547e73e991c484fa07f/flask_testing/utils.py#L118). + A key difference is that flask_testing's TestCase creates the flask app for each test method whereas + our ServerTestCase creates it once per class. flask_testing's test case allows also for custom Response mixins + and other stuff to be compatible across flask versions. Since, we don't need that the implementation can be + simplified. + + Notably, the implementation of the setUpClass, setUp, tearDown, tearDownClass, assertMessageFlashed, + assertTemplateUsed has been adapted to use class level variables. The implementation of assertRedirects has been + modified to fix concerning absolute and relative urls in Location header. + """ + + def reset_db(self): + clear_tables() + + def temporary_login(self, user): + # flask-login stores the logged in user in the global g which lasts for the entire duration of a test + # (a test request context is pushed in setUp and popped in teardown). therefore, we need to remove + # the field manually for if multiple logins are needed in a test. + if hasattr(g, "_login_user"): + del g._login_user + with self.client.session_transaction() as session: + session['_user_id'] = user.id + session['_fresh'] = True + + @classmethod + def setUpClass(cls): + cls.app = cls.create_app() + cls.client = cls.app.test_client() + + template_rendered.connect(cls._set_template) + message_flashed.connect(cls._add_flash_message) + + def setUp(self) -> None: + self._ctx = self.app.test_request_context() + self._ctx.push() + + ServerTestCase.template = None + ServerTestCase.flashed_messages = [] + + self.reset_db() + + @classmethod + def _add_flash_message(cls, app, message, category): + ServerTestCase.flashed_messages.append((message, category)) + + @classmethod + def _set_template(cls, app, template, context): + ServerTestCase.template = (template, context) + + def tearDown(self): + self._ctx.pop() + del self._ctx + + @classmethod + def tearDownClass(cls): + template_rendered.disconnect(cls._set_template) + message_flashed.disconnect(cls._add_flash_message) + del cls.client + del cls.app + + def assertMessageFlashed(self, message, category='message'): + """ + Checks if a given message was flashed. + Only works if your version of Flask has message_flashed + signal support (0.10+) and blinker is installed. + :param message: expected message + :param category: expected message category + """ + for _message, _category in ServerTestCase.flashed_messages: + if _message == message and _category == category: + return True + + raise AssertionError("Message '%s' in category '%s' wasn't flashed" % (message, category)) + + def assertTemplateUsed(self, name): + """ + Checks if a given template is used in the request. + Only works if your version of Flask has signals + support (0.6+) and blinker is installed. + If the template engine used is not Jinja2, provide + ``tmpl_name_attribute`` with a value of its `Template` + class attribute name which contains the provided ``name`` value. + :versionadded: 0.2 + :param name: template name + """ + if ServerTestCase.template is None: + self.fail("No template used") + used_template = ServerTestCase.template[0].name + self.assertEqual(used_template, name, f"Template {name} not used. Template used: {used_template}") + + def get_context_variable(self, name): + if ServerTestCase.template is None: + self.fail("No template used") + context = ServerTestCase.template[1] + if name in context: + return context[name] + raise ValueError() + + def assertContext(self, name, value, message=None): + try: + self.assertEqual(self.get_context_variable(name), value, message) + except ValueError: + self.fail(message or "Context variable does not exist: %s" % name) + + assert_context = assertContext + + def assertRedirects(self, response, location, message=None, permanent=False): + if permanent: + valid_status_codes = (301, 308) + else: + valid_status_codes = (301, 302, 303, 305, 307, 308) + + valid_status_code_str = ', '.join(str(code) for code in valid_status_codes) + not_redirect = f"HTTP Status {valid_status_code_str} expected but got {response.status_code}" + + self.assertIn(response.status_code, valid_status_codes, message or not_redirect) + + response_location = parse.unquote(response.location) + location_mismatch = f"Expected redirect location {location} but got {response_location}" + self.assertTrue(response_location.endswith(location), message or location_mismatch) + + def assertStatus(self, response, status_code, message=None): + message = message or 'HTTP Status %s expected but got %s' \ + % (status_code, response.status_code) + self.assertEqual(response.status_code, status_code, message) + + def assert200(self, response, message=None): + self.assertStatus(response, 200, message) + + def assert400(self, response, message=None): + self.assertStatus(response, 400, message) + + def assert401(self, response, message=None): + self.assertStatus(response, 401, message) + + def assert403(self, response, message=None): + self.assertStatus(response, 403, message) + + def assert404(self, response, message=None): + self.assertStatus(response, 404, message) + + def assert500(self, response, message=None): + self.assertStatus(response, 500, message) \ No newline at end of file diff --git a/critiquebrainz/ws/__init__.py b/critiquebrainz/ws/__init__.py index a91ca10eb..1d1843543 100644 --- a/critiquebrainz/ws/__init__.py +++ b/critiquebrainz/ws/__init__.py @@ -86,6 +86,9 @@ def create_app(debug=None, config_path=None): from critiquebrainz.frontend import babel babel.init_app(app, domain='cb_webservice') + from critiquebrainz.frontend.external import mbstore + mbstore.init_app(app) + # OAuth from critiquebrainz.ws.oauth import oauth oauth.init_app(app) diff --git a/critiquebrainz/ws/testing.py b/critiquebrainz/ws/testing.py index 96b7045f6..adfff932e 100644 --- a/critiquebrainz/ws/testing.py +++ b/critiquebrainz/ws/testing.py @@ -1,34 +1,22 @@ import os -from flask_testing import TestCase - import critiquebrainz.db.oauth_client as db_oauth_client import critiquebrainz.db.users as db_users -from critiquebrainz.data.utils import create_all, drop_tables, drop_types +from critiquebrainz.testing import ServerTestCase from critiquebrainz.ws import create_app from critiquebrainz.ws.oauth import oauth -class WebServiceTestCase(TestCase): +class WebServiceTestCase(ServerTestCase): - def create_app(self): - app = create_app(config_path=os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..', 'test_config.py')) + @classmethod + def create_app(cls): + app = create_app( + config_path=os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..', 'test_config.py') + ) oauth.init_app(app) return app - def setUp(self): - self.reset_db() - # TODO(roman): Add stuff form fixtures. - - def tearDown(self): - pass - - @staticmethod - def reset_db(): - drop_tables() - drop_types() - create_all() - @staticmethod def create_dummy_client(user): db_oauth_client.create( diff --git a/requirements.txt b/requirements.txt index 968aa2a50..622058f3c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,6 @@ click==8.1.3 Flask-Babel==2.0.0 Flask-Login==0.6.0 Flask-SQLAlchemy==2.5.1 -Flask-Testing==0.8.1 Flask-WTF==1.0.1 Markdown==3.3.6 bleach==5.0.1