From cea51d2699020ad9bc0002a1cef2de1870edd163 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Sat, 20 Jan 2024 19:10:46 +0530 Subject: [PATCH 01/13] change requirements --- requirements/test.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements/test.txt b/requirements/test.txt index dbc324ab500..462494c3c09 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -5,10 +5,10 @@ fake-factory==0.5.7 # pyup: ignore Needs an update coverage==4.5.1 mock==2.0.0 mixer==6.0.1 -pytest==3.7.1 # pyup: < 4.0.0 +pytest==6.2.5 # pyup: < 4.0.0 pytest-cov==2.5.1 -pytest-django==3.3.3 +pytest-django==3.6.0 pytest-env==0.6.2 pytest-pythonpath==0.7.2 -attrs==19.1.0 +attrs==19.2.0 parameterized==0.8.1 From b97b650b9ffbfef7e057e4b660465469a59abe6d Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 27 Jun 2024 15:38:53 -0700 Subject: [PATCH 02/13] Update pytest plugins. Remove unused pyup locks. --- requirements/test.txt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/requirements/test.txt b/requirements/test.txt index 462494c3c09..2f9cee0cdf5 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -1,13 +1,13 @@ # These are for testing beautifulsoup4==4.8.2 -factory-boy==2.7.0 # pyup: ignore - Needs an update, called 'faker' now -fake-factory==0.5.7 # pyup: ignore Needs an update -coverage==4.5.1 +factory-boy==2.7.0 +fake-factory==0.5.7 +coverage==6.2 mock==2.0.0 mixer==6.0.1 -pytest==6.2.5 # pyup: < 4.0.0 -pytest-cov==2.5.1 -pytest-django==3.6.0 +pytest-cov==3.0.0 +pytest==6.2.5 +pytest-django==4.5.2 pytest-env==0.6.2 pytest-pythonpath==0.7.2 attrs==19.2.0 From 02a8f76f872464b75bf7971721c2c999104a552e Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 27 Jun 2024 15:39:09 -0700 Subject: [PATCH 03/13] Change updated skipif for pytest. --- kolibri/core/discovery/test/test_network_broadcast.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kolibri/core/discovery/test/test_network_broadcast.py b/kolibri/core/discovery/test/test_network_broadcast.py index 21ec1b6a19d..af150f6555f 100644 --- a/kolibri/core/discovery/test/test_network_broadcast.py +++ b/kolibri/core/discovery/test/test_network_broadcast.py @@ -242,7 +242,7 @@ def test_is_broadcasting(self): self.broadcast.zeroconf = self.zeroconf self.assertTrue(self.broadcast.is_broadcasting) - @pytest.mark.skipIf(ZEROCONF_NEEDS_UPDATE, "Needs updated Zeroconf") + @pytest.mark.skipif(ZEROCONF_NEEDS_UPDATE, "Needs updated Zeroconf") def test_addresses(self): self.assertEqual(set(), self.broadcast.addresses) self.broadcast.zeroconf = self.zeroconf @@ -279,7 +279,7 @@ def test_update_broadcast__instance(self, mock_renew): self.assertEqual("abc-1", self.broadcast.instance.zeroconf_id) mock_renew.assert_called_once() - @pytest.mark.skipIf(ZEROCONF_NEEDS_UPDATE, "Needs updated Zeroconf") + @pytest.mark.skipif(ZEROCONF_NEEDS_UPDATE, "Needs updated Zeroconf") def test_update_broadcast__interfaces(self): new_interfaces = [MOCK_INTERFACE_IP] self.assertNotEqual(new_interfaces, self.broadcast.interfaces) From 21229c8c1bc2a3a9c09bb857ffad45938dade0bd Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 27 Jun 2024 15:39:43 -0700 Subject: [PATCH 04/13] Clean up db backups after tests to prevent issues. --- kolibri/core/deviceadmin/tests/test_dbrestore.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/kolibri/core/deviceadmin/tests/test_dbrestore.py b/kolibri/core/deviceadmin/tests/test_dbrestore.py index 92eb712f30e..c0f583d0b50 100644 --- a/kolibri/core/deviceadmin/tests/test_dbrestore.py +++ b/kolibri/core/deviceadmin/tests/test_dbrestore.py @@ -109,6 +109,11 @@ def test_fail_on_unknown_file(): get_dtm_from_backup_name("this-file-has-no-time") +def _clear_backups(folder): + for f in os.listdir(folder): + os.remove(os.path.join(folder, f)) + + @pytest.mark.django_db @pytest.mark.filterwarnings("ignore:Overriding setting DATABASES") def test_restore_from_latest(): @@ -146,6 +151,7 @@ def test_restore_from_latest(): assert ( Facility.objects.filter(name="test latest", kind=FACILITY).count() == 1 ) + _clear_backups(default_backup_folder()) @pytest.mark.django_db @@ -176,6 +182,7 @@ def test_restore_from_file_to_memory(): call_command("dbrestore", backup) # Test that the user has been restored! assert Facility.objects.filter(name="test file", kind=FACILITY).count() == 1 + _clear_backups(dest_folder) @pytest.mark.django_db @@ -212,6 +219,7 @@ def test_restore_from_file_to_file(): call_command("dbrestore", backup) # Test that the user has been restored! assert Facility.objects.filter(name="test file", kind=FACILITY).count() == 1 + _clear_backups(dest_folder) def test_search_latest(): From 125edae5964bb061fafeaae412e3da73f5362ec2 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 1 Jul 2024 11:55:28 -0700 Subject: [PATCH 05/13] Fix skipIf syntax. --- kolibri/core/discovery/test/test_network_broadcast.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kolibri/core/discovery/test/test_network_broadcast.py b/kolibri/core/discovery/test/test_network_broadcast.py index af150f6555f..b2c20fc40ea 100644 --- a/kolibri/core/discovery/test/test_network_broadcast.py +++ b/kolibri/core/discovery/test/test_network_broadcast.py @@ -242,7 +242,7 @@ def test_is_broadcasting(self): self.broadcast.zeroconf = self.zeroconf self.assertTrue(self.broadcast.is_broadcasting) - @pytest.mark.skipif(ZEROCONF_NEEDS_UPDATE, "Needs updated Zeroconf") + @pytest.mark.skipif(ZEROCONF_NEEDS_UPDATE, reason="Needs updated Zeroconf") def test_addresses(self): self.assertEqual(set(), self.broadcast.addresses) self.broadcast.zeroconf = self.zeroconf @@ -279,7 +279,7 @@ def test_update_broadcast__instance(self, mock_renew): self.assertEqual("abc-1", self.broadcast.instance.zeroconf_id) mock_renew.assert_called_once() - @pytest.mark.skipif(ZEROCONF_NEEDS_UPDATE, "Needs updated Zeroconf") + @pytest.mark.skipif(ZEROCONF_NEEDS_UPDATE, reason="Needs updated Zeroconf") def test_update_broadcast__interfaces(self): new_interfaces = [MOCK_INTERFACE_IP] self.assertNotEqual(new_interfaces, self.broadcast.interfaces) From 4ee0a2673d8fbb2d9593fd7ac7e1bb1d2065ffb9 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 1 Jul 2024 11:58:31 -0700 Subject: [PATCH 06/13] Remove pytest patch now that we've upgraded. --- test/patch_pytest.py | 58 ------------------------------------------ test/pytest_3.10.patch | 38 --------------------------- tox.ini | 1 - 3 files changed, 97 deletions(-) delete mode 100644 test/patch_pytest.py delete mode 100644 test/pytest_3.10.patch diff --git a/test/patch_pytest.py b/test/patch_pytest.py deleted file mode 100644 index f57e5226690..00000000000 --- a/test/patch_pytest.py +++ /dev/null @@ -1,58 +0,0 @@ -""" -This script backports this Python 3.10 compatibility fix https://github.com/pytest-dev/pytest/pull/8540 -in order to allow pytest to run in Python 3.10 without upgrading to version 6.2.5 which does not support 2.7 -""" -import logging -import os -import subprocess -import sys - -import pytest - -logger = logging.getLogger(__name__) - - -def patch(): - site_packages_dir = os.path.dirname(pytest.__file__) - - patch_file = os.path.join(os.path.dirname(__file__), "pytest_3.10.patch") - - logger.info("Applying patch: " + str(patch_file)) - - # -N: insist this is FORWARD patch, don't reverse apply - # -p1: strip first path component - # -t: batch mode, don't ask questions - patch_command = [ - "patch", - "-N", - "-p1", - "-d", - site_packages_dir, - "-t", - "-i", - patch_file, - ] - logger.info(" ".join(patch_command)) - try: - # Use a dry run to establish whether the patch is already applied. - # If we don't check this, the patch may be partially applied (which is bad!) - subprocess.check_output(patch_command + ["--dry-run"]) - except subprocess.CalledProcessError as e: - if e.returncode == 1: - # Return code 1 means not all hunks could be applied, this usually - # means the patch is already applied. - logger.warning( - "Failed to apply patch (exit code 1), " - "assuming it is already applied: ", - str(patch_file), - ) - else: - raise e - else: - # The dry run worked, so do the real thing - subprocess.check_output(patch_command) - - -if __name__ == "__main__": - if sys.version_info >= (3, 10): - patch() diff --git a/test/pytest_3.10.patch b/test/pytest_3.10.patch deleted file mode 100644 index 4c933e99e65..00000000000 --- a/test/pytest_3.10.patch +++ /dev/null @@ -1,38 +0,0 @@ -diff --git a/_pytest/assertion/rewrite.py b/_pytest/assertion/rewrite.py -index 4f96b9e8c..1aa5b12de 100644 ---- a/_pytest/assertion/rewrite.py -+++ b/_pytest/assertion/rewrite.py -@@ -587,10 +587,6 @@ class AssertionRewriter(ast.NodeVisitor): - return - # Insert some special imports at the top of the module but after any - # docstrings and __future__ imports. -- aliases = [ -- ast.alias(py.builtin.builtins.__name__, "@py_builtins"), -- ast.alias("_pytest.assertion.rewrite", "@pytest_ar"), -- ] - doc = getattr(mod, "docstring", None) - expect_docstring = doc is None - if doc is not None and self.is_rewrite_disabled(doc): -@@ -617,6 +613,22 @@ class AssertionRewriter(ast.NodeVisitor): - pos += 1 - else: - lineno = item.lineno -+ # Now actually insert the special imports. -+ if sys.version_info >= (3, 10): -+ aliases = [ -+ ast.alias("builtins", "@py_builtins", lineno=lineno, col_offset=0), -+ ast.alias( -+ "_pytest.assertion.rewrite", -+ "@pytest_ar", -+ lineno=lineno, -+ col_offset=0, -+ ), -+ ] -+ else: -+ aliases = [ -+ ast.alias("builtins", "@py_builtins"), -+ ast.alias("_pytest.assertion.rewrite", "@pytest_ar"), -+ ] - imports = [ - ast.Import([alias], lineno=lineno, col_offset=0) for alias in aliases - ] diff --git a/tox.ini b/tox.ini index f800e1c76d9..585bde51f51 100644 --- a/tox.ini +++ b/tox.ini @@ -26,7 +26,6 @@ deps = -r{toxinidir}/requirements/cext.txt commands = sh -c 'kolibri manage makemigrations --check' - python test/patch_pytest.py # Run the actual tests python -O -m pytest {posargs:kolibri --cov=kolibri --cov-report= --cov-append --color=no} python -O -m pytest --cov=kolibri --cov-report= --cov-append --color=no -p no:django test From cddc236944af3f0bf987e899942264e4a01f5167 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 17 Jul 2024 07:16:39 -0700 Subject: [PATCH 07/13] Cleanup usage of pytest-django db mark. --- kolibri/core/deviceadmin/tests/test_dbbackup.py | 2 +- kolibri/core/deviceadmin/tests/test_dbrestore.py | 6 +++--- .../core/tasks/test/taskrunner/test_job_running.py | 1 - kolibri/core/tasks/test/taskrunner/test_scheduler.py | 1 - kolibri/core/tasks/test/taskrunner/test_storage.py | 1 - kolibri/core/tasks/test/taskrunner/test_worker.py | 1 - kolibri/core/tasks/test/test_no_connection.py | 4 ---- kolibri/utils/tests/test_cli.py | 2 +- kolibri/utils/tests/test_main.py | 12 ++++++------ 9 files changed, 11 insertions(+), 19 deletions(-) diff --git a/kolibri/core/deviceadmin/tests/test_dbbackup.py b/kolibri/core/deviceadmin/tests/test_dbbackup.py index e62d450c200..2ec18087b79 100644 --- a/kolibri/core/deviceadmin/tests/test_dbbackup.py +++ b/kolibri/core/deviceadmin/tests/test_dbbackup.py @@ -25,7 +25,7 @@ def test_active_kolibri(): gs.assert_called_once() -@pytest.mark.django_db +@pytest.mark.django_db(transaction=True) def test_inactive_kolibri(): """ Tests that if kolibri is inactive, a dump is created diff --git a/kolibri/core/deviceadmin/tests/test_dbrestore.py b/kolibri/core/deviceadmin/tests/test_dbrestore.py index c0f583d0b50..19f75f3aeb4 100644 --- a/kolibri/core/deviceadmin/tests/test_dbrestore.py +++ b/kolibri/core/deviceadmin/tests/test_dbrestore.py @@ -114,7 +114,7 @@ def _clear_backups(folder): os.remove(os.path.join(folder, f)) -@pytest.mark.django_db +@pytest.mark.django_db(transaction=True) @pytest.mark.filterwarnings("ignore:Overriding setting DATABASES") def test_restore_from_latest(): """ @@ -154,7 +154,7 @@ def test_restore_from_latest(): _clear_backups(default_backup_folder()) -@pytest.mark.django_db +@pytest.mark.django_db(transaction=True) @pytest.mark.filterwarnings("ignore:Overriding setting DATABASES") def test_restore_from_file_to_memory(): """ @@ -185,7 +185,7 @@ def test_restore_from_file_to_memory(): _clear_backups(dest_folder) -@pytest.mark.django_db +@pytest.mark.django_db(transaction=True) @pytest.mark.filterwarnings("ignore:Overriding setting DATABASES") def test_restore_from_file_to_file(): """ diff --git a/kolibri/core/tasks/test/taskrunner/test_job_running.py b/kolibri/core/tasks/test/taskrunner/test_job_running.py index 74d9c9a0c47..d07d82baef8 100644 --- a/kolibri/core/tasks/test/taskrunner/test_job_running.py +++ b/kolibri/core/tasks/test/taskrunner/test_job_running.py @@ -162,7 +162,6 @@ def update_progress_cancelable_job(): return -@pytest.mark.django_db class TestJobStorage(object): def test_does_not_enqueue_a_function(self, storage_fixture): try: diff --git a/kolibri/core/tasks/test/taskrunner/test_scheduler.py b/kolibri/core/tasks/test/taskrunner/test_scheduler.py index 39f419e691a..3d4b0156679 100644 --- a/kolibri/core/tasks/test/taskrunner/test_scheduler.py +++ b/kolibri/core/tasks/test/taskrunner/test_scheduler.py @@ -19,7 +19,6 @@ def job_storage(): s.clear(force=True) -@pytest.mark.django_db class TestScheduler(object): @pytest.fixture def job(self): diff --git a/kolibri/core/tasks/test/taskrunner/test_storage.py b/kolibri/core/tasks/test/taskrunner/test_storage.py index d93acbc7188..873ab439b0e 100644 --- a/kolibri/core/tasks/test/taskrunner/test_storage.py +++ b/kolibri/core/tasks/test/taskrunner/test_storage.py @@ -48,7 +48,6 @@ def simplejob(func): return Job(func) -@pytest.mark.django_db class TestBackend: def test_can_enqueue_single_job(self, defaultbackend, simplejob, func): job_id = defaultbackend.enqueue_job(simplejob, QUEUE) diff --git a/kolibri/core/tasks/test/taskrunner/test_worker.py b/kolibri/core/tasks/test/taskrunner/test_worker.py index 9a361d39088..d43f155e22b 100644 --- a/kolibri/core/tasks/test/taskrunner/test_worker.py +++ b/kolibri/core/tasks/test/taskrunner/test_worker.py @@ -108,7 +108,6 @@ def delete_future(): assert job2.state == "COMPLETED" -@pytest.mark.django_db class TestWorker: def test_enqueue_job_runs_job(self, worker): job = Job(id, args=(9,)) diff --git a/kolibri/core/tasks/test/test_no_connection.py b/kolibri/core/tasks/test/test_no_connection.py index c7cc176200a..0edddee1292 100644 --- a/kolibri/core/tasks/test/test_no_connection.py +++ b/kolibri/core/tasks/test/test_no_connection.py @@ -1,7 +1,3 @@ -import pytest - - -@pytest.mark.django_db def test_importing_job_storage_no_open_connection(): from kolibri.core.tasks.main import job_storage diff --git a/kolibri/utils/tests/test_cli.py b/kolibri/utils/tests/test_cli.py index 3ae726cc027..412b1b08da6 100755 --- a/kolibri/utils/tests/test_cli.py +++ b/kolibri/utils/tests/test_cli.py @@ -142,7 +142,7 @@ def test_plugin_with_no_plugin_class(plugins): assert installed_apps_before == plugins.config["INSTALLED_PLUGINS"] -@pytest.mark.django_db +@pytest.mark.django_db(transaction=True) def test_kolibri_listen_port_env(monkeypatch): """ Starts and stops the server, mocking the actual server.start() diff --git a/kolibri/utils/tests/test_main.py b/kolibri/utils/tests/test_main.py index 4d91fdfcc7c..64c6356611e 100755 --- a/kolibri/utils/tests/test_main.py +++ b/kolibri/utils/tests/test_main.py @@ -17,7 +17,7 @@ # from django.conf import settings -@pytest.mark.django_db +@pytest.mark.django_db(transaction=True) @patch("kolibri.plugins.registry.is_initialized", return_value=False) @patch("kolibri.utils.main._upgrades_after_django_setup") @patch("kolibri.utils.main.get_version", return_value="") @@ -40,7 +40,7 @@ def test_first_run( assert set(plugins.config["INSTALLED_PLUGINS"]) == set(plugins.DEFAULT_PLUGINS) -@pytest.mark.django_db +@pytest.mark.django_db(transaction=True) @patch("kolibri.plugins.registry.is_initialized", return_value=False) @patch("kolibri.utils.main._upgrades_after_django_setup") @patch("kolibri.utils.main.get_version", return_value="0.0.1") @@ -53,7 +53,7 @@ def test_update(update, get_version, upgrades_after_django_setup, is_initialized update.assert_called_once() -@pytest.mark.django_db +@pytest.mark.django_db(transaction=True) @patch("kolibri.plugins.registry.is_initialized", return_value=False) @patch("kolibri.utils.main.get_version", return_value="0.0.1") def test_update_exits_if_running(get_version, is_initialized): @@ -68,7 +68,7 @@ def test_update_exits_if_running(get_version, is_initialized): pass -@pytest.mark.django_db +@pytest.mark.django_db(transaction=True) def test_version_updated(): """ Tests our db backup logic: version_updated gets any change, backup gets only non-dev changes @@ -81,7 +81,7 @@ def test_version_updated(): assert not main.should_back_up("0.10.0-dev0", "0.10.0-dev0") -@pytest.mark.django_db +@pytest.mark.django_db(transaction=True) @unittest.skipIf( True, # TODO: rtibbles - reinstate and fix test @@ -132,7 +132,7 @@ def test_conditional_backup(): assert higher_db_version in backups_after[0] -@pytest.mark.django_db +@pytest.mark.django_db(transaction=True) @patch("kolibri.plugins.registry.is_initialized", return_value=False) @patch("kolibri.utils.main._upgrades_after_django_setup") @patch("kolibri.utils.main.get_version", return_value=kolibri.__version__) From 2deab844a861baadd860601f9c4297d79ac04887 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 17 Jul 2024 07:18:28 -0700 Subject: [PATCH 08/13] Only use transaction test cases when needed by the test. --- kolibri/core/analytics/test/test_ping.py | 4 +- kolibri/core/analytics/test/test_utils.py | 96 ++++++++++--------- .../auth/test/test_morango_integration.py | 2 + .../core/content/test/test_delete_content.py | 8 +- .../discovery/test/test_network_search.py | 4 +- kolibri/core/public/test/test_api.py | 14 +-- kolibri/core/test/test_key_urls.py | 3 +- 7 files changed, 65 insertions(+), 66 deletions(-) diff --git a/kolibri/core/analytics/test/test_ping.py b/kolibri/core/analytics/test/test_ping.py index 1cd87f73ea5..787a7e96fb0 100644 --- a/kolibri/core/analytics/test/test_ping.py +++ b/kolibri/core/analytics/test/test_ping.py @@ -4,7 +4,7 @@ import mock from django.core.management import call_command from django.core.management.base import CommandError -from django.test import TransactionTestCase +from django.test import TestCase from requests.models import Response from .test_utils import BaseDeviceSetupMixin @@ -36,7 +36,7 @@ def json(self): return mocked_requests_post -class PingCommandTestCase(BaseDeviceSetupMixin, TransactionTestCase): +class PingCommandTestCase(BaseDeviceSetupMixin, TestCase): @mock.patch( "kolibri.core.analytics.utils.requests.post", side_effect=mocked_requests_post_wrapper({"id": 17}, 200), diff --git a/kolibri/core/analytics/test/test_utils.py b/kolibri/core/analytics/test/test_utils.py index 714def16fbc..df5d6b11815 100644 --- a/kolibri/core/analytics/test/test_utils.py +++ b/kolibri/core/analytics/test/test_utils.py @@ -7,7 +7,7 @@ import random import uuid -from django.test import TransactionTestCase +from django.test import TestCase from le_utils.constants import content_kinds from kolibri.core.analytics.constants.nutrition_endpoints import PINGBACK @@ -57,8 +57,9 @@ class BaseDeviceSetupMixin(object): min_timestamp = datetime.datetime(2018, 10, 11) max_timestamp = datetime.datetime(2019, 10, 11) - def setUp(self): - super(BaseDeviceSetupMixin, self).setUp() + @classmethod + def setUpTestData(cls): + super(BaseDeviceSetupMixin, cls).setUpTestData() clear_process_cache() # create dummy channel channel_id = uuid.uuid4().hex @@ -68,8 +69,8 @@ def setUp(self): channel_id=channel_id, content_id=uuid.uuid4().hex, ) - self.channel = ChannelMetadata.objects.create( - id=channel_id, name="channel", last_updated=self.min_timestamp, root=root + cls.channel = ChannelMetadata.objects.create( + id=channel_id, name="channel", last_updated=cls.min_timestamp, root=root ) lf = LocalFile.objects.create( id=uuid.uuid4().hex, available=True, file_size=1048576 # 1 MB @@ -81,23 +82,23 @@ def setUp(self): with io.open(data_path, mode="r", encoding="utf-8") as f: users = [data for data in csv.DictReader(f)] - self.facilities = user_data.get_or_create_facilities( - n_facilities=self.n_facilities + cls.facilities = user_data.get_or_create_facilities( + n_facilities=cls.n_facilities ) - self.users = [] + cls.users = [] - for facility in self.facilities: + for facility in cls.facilities: dataset = facility.dataset # create superuser and login session - for i in range(self.n_superusers): + for i in range(cls.n_superusers): superuser = create_superuser( facility=facility, username="superuser{}".format(i) ) facility.add_role(superuser, role_kinds.ADMIN) UserSessionLog.objects.create( user=superuser, - start_timestamp=self.min_timestamp, - last_interaction_timestamp=self.max_timestamp, + start_timestamp=cls.min_timestamp, + last_interaction_timestamp=cls.max_timestamp, ) # create lesson and exam for facility Lesson.objects.create( @@ -114,42 +115,42 @@ def setUp(self): exam_id = uuid.uuid4().hex classrooms = user_data.get_or_create_classrooms( - n_classes=self.n_classes, facility=facility + n_classes=cls.n_classes, facility=facility ) # Get all the user data at once so that it is distinct across classrooms - facility_user_data = random.sample(users, self.n_classes * self.n_users) + facility_user_data = random.sample(users, cls.n_classes * cls.n_users) # create random content id for the session logs - self.content_id = uuid.uuid4().hex + cls.content_id = uuid.uuid4().hex for i, classroom in enumerate(classrooms): classroom_user_data = facility_user_data[ - i * self.n_users : (i + 1) * self.n_users + i * cls.n_users : (i + 1) * cls.n_users ] users = user_data.get_or_create_classroom_users( - n_users=self.n_users, + n_users=cls.n_users, classroom=classroom, user_data=classroom_user_data, facility=facility, ) - self.users.extend(users) + cls.users.extend(users) # create 1 of each type of log per user for user in users: for _ in range(1): sessionlog = ContentSessionLog.objects.create( user=user, - start_timestamp=self.min_timestamp, - end_timestamp=self.max_timestamp, - content_id=self.content_id, - channel_id=self.channel.id, + start_timestamp=cls.min_timestamp, + end_timestamp=cls.max_timestamp, + content_id=cls.content_id, + channel_id=cls.channel.id, time_spent=60, # 1 minute kind=content_kinds.EXERCISE, ) AttemptLog.objects.create( item="item", - start_timestamp=self.min_timestamp, - end_timestamp=self.max_timestamp, - completion_timestamp=self.max_timestamp, + start_timestamp=cls.min_timestamp, + end_timestamp=cls.max_timestamp, + completion_timestamp=cls.max_timestamp, correct=1, sessionlog=sessionlog, ) @@ -157,34 +158,34 @@ def setUp(self): ContentSessionLog.objects.create( dataset=dataset, user=None, - start_timestamp=self.min_timestamp, - end_timestamp=self.max_timestamp, - content_id=self.content_id, - channel_id=self.channel.id, + start_timestamp=cls.min_timestamp, + end_timestamp=cls.max_timestamp, + content_id=cls.content_id, + channel_id=cls.channel.id, time_spent=60, # 1 minute, kind=content_kinds.VIDEO, ) for _ in range(1): UserSessionLog.objects.create( user=user, - start_timestamp=self.min_timestamp, - last_interaction_timestamp=self.max_timestamp, + start_timestamp=cls.min_timestamp, + last_interaction_timestamp=cls.max_timestamp, device_info="Android,9/Chrome Mobile,86", ) for _ in range(1): ContentSummaryLog.objects.create( user=user, - start_timestamp=self.min_timestamp, - end_timestamp=self.max_timestamp, - completion_timestamp=self.max_timestamp, + start_timestamp=cls.min_timestamp, + end_timestamp=cls.max_timestamp, + completion_timestamp=cls.max_timestamp, content_id=uuid.uuid4().hex, - channel_id=self.channel.id, + channel_id=cls.channel.id, ) for _ in range(1): sl = ContentSessionLog.objects.create( user=user, - start_timestamp=self.min_timestamp, - end_timestamp=self.max_timestamp, + start_timestamp=cls.min_timestamp, + end_timestamp=cls.max_timestamp, content_id=exam_id, channel_id=None, time_spent=60, # 1 minute @@ -192,9 +193,9 @@ def setUp(self): ) summarylog = ContentSummaryLog.objects.create( user=user, - start_timestamp=self.min_timestamp, - end_timestamp=self.max_timestamp, - completion_timestamp=self.max_timestamp, + start_timestamp=cls.min_timestamp, + end_timestamp=cls.max_timestamp, + completion_timestamp=cls.max_timestamp, content_id=exam_id, channel_id=None, kind=content_kinds.QUIZ, @@ -209,9 +210,9 @@ def setUp(self): AttemptLog.objects.create( masterylog=masterylog, sessionlog=sl, - start_timestamp=self.min_timestamp, - end_timestamp=self.max_timestamp, - completion_timestamp=self.max_timestamp, + start_timestamp=cls.min_timestamp, + end_timestamp=cls.max_timestamp, + completion_timestamp=cls.max_timestamp, correct=1, item="test:test", ) @@ -221,7 +222,7 @@ def tearDown(self): DeviceSettings.objects.delete() -class FacilityStatisticsTestCase(BaseDeviceSetupMixin, TransactionTestCase): +class FacilityStatisticsTestCase(BaseDeviceSetupMixin, TestCase): def test_extract_facility_statistics(self): provision_device(allow_guest_access=True) facility = self.facilities[0] @@ -323,7 +324,7 @@ def test_regression_4606_no_contentsessions_or_usersessions(self): assert actual["l"] is None -class SoudFacilityStatisticsTestCase(BaseDeviceSetupMixin, TransactionTestCase): +class SoudFacilityStatisticsTestCase(BaseDeviceSetupMixin, TestCase): n_facilities = 1 n_superusers = 0 n_users = 2 @@ -340,7 +341,7 @@ def test_extract_facility_statistics__soud_hash(self): self.assertEqual(expected_soud_hash, actual.pop("sh")) -class ChannelStatisticsTestCase(BaseDeviceSetupMixin, TransactionTestCase): +class ChannelStatisticsTestCase(BaseDeviceSetupMixin, TestCase): def test_extract_channel_statistics(self): actual = extract_channel_statistics(self.channel) birth_year_list_learners = [ @@ -388,7 +389,7 @@ def test_extract_channel_statistics(self): assert actual == expected -class CreateUpdateNotificationsTestCase(TransactionTestCase): +class CreateUpdateNotificationsTestCase(TestCase): def setUp(self): self.msg = { "i18n": {}, @@ -406,6 +407,7 @@ def setUp(self): "version_range": "<1.0.0", "source": PINGBACK, } + PingbackNotification.objects.create(**self.data) def test_no_messages_still_updates(self): diff --git a/kolibri/core/auth/test/test_morango_integration.py b/kolibri/core/auth/test/test_morango_integration.py index d472619fcc1..3fad0e545ca 100644 --- a/kolibri/core/auth/test/test_morango_integration.py +++ b/kolibri/core/auth/test/test_morango_integration.py @@ -63,6 +63,8 @@ def test_partition_and_id_values(self): self.assertTrue(partition.startswith(dataset_id)) +# This needs to be a TransactionTestCase for Postgres, due to transaction +# isolation requirements - but works fine otherwise in SQLite. class DateTimeTZFieldTestCase(TransactionTestCase): def setUp(self): self.controller = MorangoProfileController(PROFILE_FACILITY_DATA) diff --git a/kolibri/core/content/test/test_delete_content.py b/kolibri/core/content/test/test_delete_content.py index a6155abc80f..0f616028a46 100644 --- a/kolibri/core/content/test/test_delete_content.py +++ b/kolibri/core/content/test/test_delete_content.py @@ -3,6 +3,7 @@ import uuid from django.core.management import call_command +from django.test import TestCase from django.test import TransactionTestCase from le_utils.constants import content_kinds from le_utils.constants import file_formats @@ -23,8 +24,7 @@ def get_engine(connection_string): test_channel_id = "6199dde695db4ee4ab392222d5af1e5c" -@patch("kolibri.core.content.utils.sqlalchemybridge.get_engine", new=get_engine) -class UnavailableContentDeletion(TransactionTestCase): +class UnavailableContentDeletion(TestCase): databases = "__all__" def setUp(self): @@ -101,10 +101,6 @@ def test_dont_delete_used_stored_files(self): deleted = self.delete_content() self.assertEqual(deleted, 0) - def tearDown(self): - call_command("flush", interactive=False) - super(UnavailableContentDeletion, self).tearDown() - @patch("kolibri.core.content.utils.sqlalchemybridge.get_engine", new=get_engine) class DeleteContentTestCase(TransactionTestCase): diff --git a/kolibri/core/discovery/test/test_network_search.py b/kolibri/core/discovery/test/test_network_search.py index bbfa348d7ab..31d67af2a90 100644 --- a/kolibri/core/discovery/test/test_network_search.py +++ b/kolibri/core/discovery/test/test_network_search.py @@ -1,5 +1,5 @@ import mock -from django.test import TransactionTestCase +from django.test import TestCase from ..utils.network.broadcast import KolibriBroadcast from ..utils.network.broadcast import KolibriInstance @@ -16,7 +16,7 @@ ) -class NetworkLocationListenerTestCase(TransactionTestCase): +class NetworkLocationListenerTestCase(TestCase): databases = "__all__" def setUp(self): diff --git a/kolibri/core/public/test/test_api.py b/kolibri/core/public/test/test_api.py index 4b4ecdf4506..751e17339ee 100644 --- a/kolibri/core/public/test/test_api.py +++ b/kolibri/core/public/test/test_api.py @@ -14,7 +14,6 @@ from morango.models import TransferSession from rest_framework import status from rest_framework.test import APITestCase -from rest_framework.test import APITransactionTestCase import kolibri from kolibri.core.auth.models import Facility @@ -95,22 +94,23 @@ def create_mini_channel( ) -class PublicAPITestCase(APITransactionTestCase): +class PublicAPITestCase(APITestCase): """ IMPORTANT: These tests are to never be changed. They are enforcing a public API contract. If the tests fail, then the implementation needs to be changed, and not the tests themselves. """ - def setUp(self): + @classmethod + def setUpTestData(cls): provision_device() Language.objects.create(id="en", lang_code="en") Language.objects.create(id="es", lang_code="es") - self.channel_id1 = uuid.uuid4().hex - self.channel_id2 = uuid.uuid4().hex - create_mini_channel(channel_name="math", channel_id=self.channel_id1) + cls.channel_id1 = uuid.uuid4().hex + cls.channel_id2 = uuid.uuid4().hex + create_mini_channel(channel_name="math", channel_id=cls.channel_id1) channel2 = create_mini_channel( - channel_name="science", channel_id=self.channel_id2, root_lang="es" + channel_name="science", channel_id=cls.channel_id2, root_lang="es" ) set_channel_metadata_fields(channel2.id, public=True) diff --git a/kolibri/core/test/test_key_urls.py b/kolibri/core/test/test_key_urls.py index 24c5422d41e..cb9af2c72c4 100644 --- a/kolibri/core/test/test_key_urls.py +++ b/kolibri/core/test/test_key_urls.py @@ -3,7 +3,6 @@ from django.urls.exceptions import NoReverseMatch from mock import patch from rest_framework.test import APITestCase -from rest_framework.test import APITransactionTestCase from kolibri.core.auth.constants import role_kinds from kolibri.core.auth.test.helpers import clear_process_cache @@ -72,7 +71,7 @@ def test_class_coach_is_redirected_to_coach_plugin(self): self._assert_location_reverse_url("kolibri:kolibri.plugins.coach:coach") -class AllUrlsTest(APITransactionTestCase): +class AllUrlsTest(APITestCase): databases = "__all__" From 062fae368d19efdb1f0ed495071e2a18a2309454 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 17 Jul 2024 07:18:49 -0700 Subject: [PATCH 09/13] Cleanup test case inheritance to prevent duplicate runs. --- kolibri/core/content/test/test_content_sync_hook.py | 4 ++-- kolibri/core/content/test/utils/test_content_request.py | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/kolibri/core/content/test/test_content_sync_hook.py b/kolibri/core/content/test/test_content_sync_hook.py index 480b7e90c2d..7e18382823c 100644 --- a/kolibri/core/content/test/test_content_sync_hook.py +++ b/kolibri/core/content/test/test_content_sync_hook.py @@ -4,12 +4,12 @@ from kolibri.core.auth.sync_operations import KolibriSyncOperations from kolibri.core.content.kolibri_plugin import ContentSyncHook from kolibri.core.content.test.utils.test_content_request import ( - IncompleteDownloadsQuerysetTestCase, + BaseIncompleteDownloadsQuerysetTestCase, ) from kolibri.core.device.models import DeviceStatus -class KolibriContentSyncHookTestCase(IncompleteDownloadsQuerysetTestCase): +class KolibriContentSyncHookTestCase(BaseIncompleteDownloadsQuerysetTestCase): def setUp(self): super(KolibriContentSyncHookTestCase, self).setUp() self.operation = KolibriSyncOperations() diff --git a/kolibri/core/content/test/utils/test_content_request.py b/kolibri/core/content/test/utils/test_content_request.py index 8585995935c..b787e40b0bc 100644 --- a/kolibri/core/content/test/utils/test_content_request.py +++ b/kolibri/core/content/test/utils/test_content_request.py @@ -414,9 +414,9 @@ def _create_resources(self, node_id, available=False): return (parent, node) -class IncompleteDownloadsQuerysetTestCase(BaseQuerysetTestCase): +class BaseIncompleteDownloadsQuerysetTestCase(BaseQuerysetTestCase): def setUp(self): - super(IncompleteDownloadsQuerysetTestCase, self).setUp() + super(BaseIncompleteDownloadsQuerysetTestCase, self).setUp() self.admin_request = ContentDownloadRequest.build_for_user(self.admin) self.admin_request.contentnode_id = uuid.uuid4().hex self.admin_request.save() @@ -424,6 +424,8 @@ def setUp(self): self.learner_request.contentnode_id = uuid.uuid4().hex self.learner_request.save() + +class IncompleteDownloadsQuerysetTestCase(BaseIncompleteDownloadsQuerysetTestCase): @mock.patch(_module + "get_device_setting", return_value=False) def test_learner_downloads_disabled(self, mock_get_device_setting): qs = incomplete_downloads_queryset() From be57772058685132271051411db4c4b6d974bf6d Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 17 Jul 2024 07:19:27 -0700 Subject: [PATCH 10/13] Use patch to prevent side effects from connection overrides. --- .../core/deviceadmin/tests/test_dbrestore.py | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/kolibri/core/deviceadmin/tests/test_dbrestore.py b/kolibri/core/deviceadmin/tests/test_dbrestore.py index 19f75f3aeb4..365a5582b6c 100644 --- a/kolibri/core/deviceadmin/tests/test_dbrestore.py +++ b/kolibri/core/deviceadmin/tests/test_dbrestore.py @@ -5,6 +5,7 @@ import pytest from django.conf import settings from django.core.management import call_command +from django.db import ConnectionHandler from django.test.utils import override_settings from mock import patch @@ -174,14 +175,13 @@ def test_restore_from_file_to_memory(): # Restore it into a new test database setting with override_settings(DATABASES=MOCK_DATABASES): - from django import db - - # Destroy current connections and create new ones: - db.connections.close_all() - db.connections = db.ConnectionHandler() - call_command("dbrestore", backup) - # Test that the user has been restored! - assert Facility.objects.filter(name="test file", kind=FACILITY).count() == 1 + with patch("django.db.connections", ConnectionHandler()): + call_command("dbrestore", backup) + # Test that the user has been restored! + assert ( + Facility.objects.filter(name="test file", kind=FACILITY).count() + == 1 + ) _clear_backups(dest_folder) @@ -210,15 +210,13 @@ def test_restore_from_file_to_file(): # Restore it into a new test database setting with override_settings(DATABASES=MOCK_DATABASES_FILE): - # Destroy current connections and create new ones: - db.connections.close_all() - db.connections = db.ConnectionHandler() - # Purposefully destroy the connection pointer, which is the default - # state of an unopened connection - db.connections["default"].connection = None - call_command("dbrestore", backup) - # Test that the user has been restored! - assert Facility.objects.filter(name="test file", kind=FACILITY).count() == 1 + with patch("django.db.connections", ConnectionHandler()): + call_command("dbrestore", backup) + # Test that the user has been restored! + assert ( + Facility.objects.filter(name="test file", kind=FACILITY).count() + == 1 + ) _clear_backups(dest_folder) From 0fb20ca7384b9fa5c4142239ee1d580805cebef6 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 17 Jul 2024 07:20:12 -0700 Subject: [PATCH 11/13] Cleanup futures cleanup tests to prevent errors in threads. --- .../core/tasks/test/taskrunner/test_worker.py | 25 +++---------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/kolibri/core/tasks/test/taskrunner/test_worker.py b/kolibri/core/tasks/test/taskrunner/test_worker.py index d43f155e22b..49d10e0d9dc 100644 --- a/kolibri/core/tasks/test/taskrunner/test_worker.py +++ b/kolibri/core/tasks/test/taskrunner/test_worker.py @@ -1,5 +1,4 @@ # -*- coding: utf-8 -*- -import threading import time import pytest @@ -56,17 +55,9 @@ def test_keyerror_prevention(worker): job = Job(id, args=(9,)) worker.storage.enqueue_job(job, QUEUE) - # Simulate a race condition by having another thread try to delete the future - # while the job is running - def delete_future(): - time.sleep(0.5) # Wait for the job to start - del worker.future_job_mapping[job.job_id] - - # Start the delete_future thread - delete_thread = threading.Thread(target=delete_future) - delete_thread.start() - while job.state != "COMPLETED": + if job.job_id in worker.future_job_mapping: + del worker.future_job_mapping[job.job_id] job = worker.storage.get_job(job.job_id) time.sleep(0.1) @@ -81,20 +72,12 @@ def test_keyerror_prevention_multiple_jobs(worker): # Enqueue the first job worker.storage.enqueue_job(job1, QUEUE) - # Simulate a race condition by having another thread try to delete the future - # while the first job is running - def delete_future(): - time.sleep(0.5) # Wait for the first job to start - del worker.future_job_mapping[job1.job_id] - - # Start the delete_future thread - delete_thread = threading.Thread(target=delete_future) - delete_thread.start() - # Enqueue the second job worker.storage.enqueue_job(job2, QUEUE) while job1.state != "COMPLETED": + if job1.job_id in worker.future_job_mapping: + del worker.future_job_mapping[job1.job_id] job1 = worker.storage.get_job(job1.job_id) time.sleep(0.1) From 8111f143263b1ff374fccd9331e295157489f347 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 17 Jul 2024 07:20:36 -0700 Subject: [PATCH 12/13] Cleanup temporarily activated plugins after the test has completed. --- kolibri/plugins/utils/test/helpers.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/kolibri/plugins/utils/test/helpers.py b/kolibri/plugins/utils/test/helpers.py index 7ed5a2f9e4e..88cbf3c6632 100644 --- a/kolibri/plugins/utils/test/helpers.py +++ b/kolibri/plugins/utils/test/helpers.py @@ -12,6 +12,7 @@ from django.conf import settings from django.urls import clear_url_caches +from kolibri.plugins import config from kolibri.plugins.registry import registered_plugins @@ -42,8 +43,11 @@ def plugin_enabled(*plugin_names): for plugin_name in plugin_names: try: del registered_plugins._apps[plugin_name] + if plugin_name in config["UPDATED_PLUGINS"]: + config["UPDATED_PLUGINS"].remove(plugin_name) except KeyError: pass + config.save() _reset_plugin_dependent_modules() From 5a7f0553c9567c52219637da3f5be2ff1496cb1a Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 17 Jul 2024 08:47:39 -0700 Subject: [PATCH 13/13] Remove code coverage reporting, due to difficulties supporting Python 3.6 and 3.11 --- .github/PULL_REQUEST_TEMPLATE.md | 1 - .github/workflows/c_extensions.yml | 12 +- docs/stack.rst | 1 - requirements/test.txt | 2 - test/coverage_blame.py | 187 ----------------------------- tox.ini | 8 +- 6 files changed, 10 insertions(+), 201 deletions(-) delete mode 100755 test/coverage_blame.py diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 42af345e332..549853605b5 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -51,7 +51,6 @@ ## Reviewer checklist -- Automated test coverage is satisfactory - PR is fully functional - PR has been tested for [accessibility regressions](http://kolibri-dev.readthedocs.io/en/develop/manual_testing.html#accessibility-a11y-testing) - External dependency files were updated if necessary (`yarn` and `pip`) diff --git a/.github/workflows/c_extensions.yml b/.github/workflows/c_extensions.yml index 13eff01917f..1534f3a690b 100644 --- a/.github/workflows/c_extensions.yml +++ b/.github/workflows/c_extensions.yml @@ -57,10 +57,10 @@ jobs: make staticdeps-cext pip install . # Start and stop kolibri - coverage run -p kolibri start --port=8081 - coverage run -p kolibri stop + kolibri start --port=8081 + kolibri stop # Run just tests in test/ - py.test --cov=kolibri --cov-report= --cov-append --color=no test/ + py.test --color=no test/ no_c_ext: name: No C Extensions needs: pre_job @@ -94,7 +94,7 @@ jobs: make staticdeps pip install . # Start and stop kolibri - coverage run -p kolibri start --port=8081 - coverage run -p kolibri stop + kolibri start --port=8081 + kolibri stop # Run just tests in test/ - py.test --cov=kolibri --cov-report= --cov-append --color=no test/ + py.test --color=no test/ diff --git a/docs/stack.rst b/docs/stack.rst index 4f69109c401..3cbfbf7ce68 100644 --- a/docs/stack.rst +++ b/docs/stack.rst @@ -71,5 +71,4 @@ We use a number of mechanisms to help encourage code quality and consistency. Mo - `pytest `__ runs our Python unit tests. We also leverage the `Django test framework `__. - In addition to building client assets, `webpack `__ runs linters on client-side code: `ESLint `__ for ES6 JavaScript, `Stylelint `__ for SCSS, and `HTMLHint `__ for HTML and Vue.js components. - Client-side code is tested using `Jest `__ -- `codecov `__ reports on the test coverage - We have `Sentry `__ clients integrated (off by default) for automated error reporting diff --git a/requirements/test.txt b/requirements/test.txt index 2f9cee0cdf5..872fb52727d 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -2,10 +2,8 @@ beautifulsoup4==4.8.2 factory-boy==2.7.0 fake-factory==0.5.7 -coverage==6.2 mock==2.0.0 mixer==6.0.1 -pytest-cov==3.0.0 pytest==6.2.5 pytest-django==4.5.2 pytest-env==0.6.2 diff --git a/test/coverage_blame.py b/test/coverage_blame.py deleted file mode 100755 index a945fdf2b9c..00000000000 --- a/test/coverage_blame.py +++ /dev/null @@ -1,187 +0,0 @@ -#!/usr/bin/env python -# -*- coding: utf-8 -*- -# Derived from http://scottlobdell.me/2015/04/gamifying-test-coverage-project/ -# Before running this script, first run tests with coverage using: -# tox -e py3.9 -import logging -from collections import Counter -from subprocess import PIPE -from subprocess import Popen - -logger = logging.getLogger(__name__) - -LINE_COUNT_THRESH = 50 - - -class ExcludeLineParser(object): - @classmethod - def get_missing_lines_for_filename(cls, filename): - command = "coverage report -m %s" % filename - process = Popen(command.split(), stdout=PIPE) - output, _ = process.communicate() - return cls._get_excluded_lines(output) - - @classmethod - def _get_excluded_lines(cls, coverage_output): - excluded_line_numbers = [] - ignore_line_count = 2 - ignore_column_count = 6 - lines = [ - line for line in coverage_output.split("\n")[ignore_line_count:] if line - ] - for line in lines: - exclude_line_strings = line.split()[ignore_column_count:] - for exclude_line_string in exclude_line_strings: - exclude_line_string = exclude_line_string.replace(",", "").replace( - " ", "" - ) - exclude_lines = cls._convert_exclude_line_string_to_ints( - exclude_line_string - ) - excluded_line_numbers.extend(exclude_lines) - - return excluded_line_numbers - - @classmethod - def _convert_exclude_line_string_to_ints(cls, exclude_line_string): - if "->" in exclude_line_string: - return [int(exclude_line_string.split("->")[0])] - if "-" in exclude_line_string: - line_start, line_end = exclude_line_string.split("-") - if line_end == "exit": - line_end = line_start - return range(int(line_start), int(line_end) + 1) - else: - try: - line_number = int(exclude_line_string) - except ValueError: - logger.error("Error for values ({})".format(exclude_line_string)) - return [] - return [line_number] - - -def _get_output_from_pipe_command(command_with_pipes): - piped_commands = [command.strip() for command in command_with_pipes.split("|")] - previous_process = None - for command in piped_commands: - process = Popen( - command.split(), - stdin=previous_process and previous_process.stdout, - stdout=PIPE, - ) - previous_process = process - output, err = previous_process.communicate() - return output - - -def get_python_files(): - full_command = ( - "find kolibri | grep py | grep -v pyc | grep -v test | grep -v virtualenv" - ) - output = _get_output_from_pipe_command(full_command) - filenames = [filename for filename in output.split("\n") if filename] - return filenames - - -def git_blame_on_files(file_list): - total_counter = Counter() - miss_counter = Counter() - for index, filename in enumerate(file_list): - full_command = ( - "git blame --line-porcelain %s | grep author | grep -v author-" % filename - ) - output = _get_output_from_pipe_command(full_command) - - git_scorer = GitScorer(output) - counter = git_scorer.get_author_counts() - - line_to_author = git_scorer.get_line_to_author() - non_covered_lines = ExcludeLineParser.get_missing_lines_for_filename(filename) - miss_counter += attribute_missing_coverage_to_author( - line_to_author, non_covered_lines - ) - - total_counter += counter - return total_counter, miss_counter - - -def apply_threshold_to_counter(counter): - for key in counter.keys(): - if counter[key] < LINE_COUNT_THRESH: - del counter[key] - - -def attribute_missing_coverage_to_author(line_to_author, non_covered_lines): - author_to_miss_count = Counter() - for line_number in non_covered_lines: - try: - author = line_to_author[line_number] - except KeyError: - return Counter() - author_to_miss_count[author] += 1 - return author_to_miss_count - - -class GitScorer(object): - def __init__(self, gblame_output): - self.counts_this_file = Counter() - self.line_to_author = {} - self._parse_git_blame_output(gblame_output) - - def _get_author_from_line(self, line): - author = line.replace("author ", "") - if author.startswith(" ") or author.startswith("\t"): - return None - return author - - def _parse_git_blame_output(self, git_blame_output): - lines = git_blame_output.split("\n") - for index, line in enumerate(lines): - if not line: - continue - line_number = index + 1 - author = self._get_author_from_line(line) - if author: - self.counts_this_file[author] += 1 - self.line_to_author[line_number] = author - - def get_author_counts(self): - return self.counts_this_file - - def get_line_to_author(self): - return self.line_to_author - - -def get_test_coverage_percent_per_author(line_counter, miss_counter): - author_to_test_coverage = {} - for author in line_counter.keys(): - line_count = line_counter[author] - miss_count = miss_counter.get(author, 0) - miss_percent = float(miss_count) / line_count - test_coverage_percent = 1.0 - miss_percent - author_to_test_coverage[author] = test_coverage_percent - return author_to_test_coverage - - -if __name__ == "__main__": - python_files = get_python_files() - line_counter, miss_counter = git_blame_on_files(python_files) - apply_threshold_to_counter(line_counter) - author_to_test_coverage = get_test_coverage_percent_per_author( - line_counter, miss_counter - ) - rank = 1 - for author, cov in sorted( - author_to_test_coverage.items(), key=lambda t: t[1], reverse=True - ): - logger.info( - "#%s. %s: %.2d%% coverage (%d out of %d lines)" - % ( - rank, - author, - cov * 100, - line_counter[author] - miss_counter[author], - line_counter[author], - ) - ) - rank += 1 diff --git a/tox.ini b/tox.ini index 585bde51f51..0d2e5ef1852 100644 --- a/tox.ini +++ b/tox.ini @@ -27,8 +27,8 @@ deps = commands = sh -c 'kolibri manage makemigrations --check' # Run the actual tests - python -O -m pytest {posargs:kolibri --cov=kolibri --cov-report= --cov-append --color=no} - python -O -m pytest --cov=kolibri --cov-report= --cov-append --color=no -p no:django test + python -O -m pytest {posargs:kolibri --color=no} + python -O -m pytest --color=no -p no:django test # Fail if the log is longer than 200 lines (something erroring or very noisy got added) sh -c "if [ `cat {env:KOLIBRI_HOME}/logs/kolibri.txt | wc -l` -gt 200 ] ; then echo 'Log too long' && echo '' && tail -n 20 {env:KOLIBRI_HOME}/logs/kolibri.txt && exit 1 ; fi" @@ -54,5 +54,5 @@ deps = -r{toxinidir}/requirements/cext.txt -r{toxinidir}/requirements/postgres.txt commands = - python -O -m pytest {posargs:kolibri --cov=kolibri --cov-report= --cov-append --color=no} - python -O -m pytest --cov=kolibri --cov-report= --cov-append --color=no -p no:django test + python -O -m pytest {posargs:kolibri --color=no} + python -O -m pytest --color=no -p no:django test