diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml index 87dd60c66a2..d0fd6443b1a 100644 --- a/.github/workflows/tox.yml +++ b/.github/workflows/tox.yml @@ -83,3 +83,52 @@ jobs: key: ${{ runner.os }}-tox-py${{ matrix.python-version }}-${{ hashFiles('requirements/*.txt') }} - name: Test with tox run: tox -e postgres + macos: + name: Python unit tests on Mac OS + needs: pre_job + if: ${{ needs.pre_job.outputs.should_skip != 'true' }} + runs-on: macos-latest + strategy: + max-parallel: 5 + matrix: + python-version: [3.6] + + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + - name: Install tox + run: | + python -m pip install --upgrade pip + pip install tox + - name: tox env cache + uses: actions/cache@v2 + with: + path: ${{ github.workspace }}/.tox/py${{ matrix.python-version }} + key: ${{ runner.os }}-tox-py${{ matrix.python-version }}-${{ hashFiles('requirements/*.txt') }} + - name: Test with tox + run: tox -e py${{ matrix.python-version }} + windows: + name: Python unit tests on Windows Server + needs: pre_job + if: ${{ needs.pre_job.outputs.should_skip != 'true' }} + runs-on: windows-latest + strategy: + max-parallel: 5 + matrix: + python-version: [3.6] + + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + - name: Install tox + run: | + python -m pip install --upgrade pip + pip install tox + - name: Test with tox + run: tox -e py${{ matrix.python-version }} diff --git a/kolibri/conftest.py b/kolibri/conftest.py index 252ded82d64..0f06da472bb 100644 --- a/kolibri/conftest.py +++ b/kolibri/conftest.py @@ -15,4 +15,8 @@ def global_fixture(): os.mkdir(os.path.join(TEMP_KOLIBRI_HOME, "content")) yield # wait until the test ended if os.path.exists(TEMP_KOLIBRI_HOME): - shutil.rmtree(TEMP_KOLIBRI_HOME) + try: + shutil.rmtree(TEMP_KOLIBRI_HOME) + except OSError: + # Don't fail a test just because we failed to cleanup + pass diff --git a/kolibri/core/auth/test/sync_utils.py b/kolibri/core/auth/test/sync_utils.py index d132266b24b..aba869d5099 100644 --- a/kolibri/core/auth/test/sync_utils.py +++ b/kolibri/core/auth/test/sync_utils.py @@ -4,12 +4,12 @@ import os import shutil import socket +import subprocess import tempfile import time import uuid import requests -import sh from django.db import connection from django.db import connections from django.utils.functional import wraps @@ -49,8 +49,9 @@ def __init__( def start(self, pre_migrate=True): if pre_migrate: KolibriServer.get_pre_migrated_database(self.env["KOLIBRI_HOME"]) - self._instance = sh.kolibri.start( - port=self.port, foreground=True, _bg=True, _env=self.env + self._instance = subprocess.Popen( + ["kolibri start", "--port {}".format(self.port), "--foreground"], + env=self.env, ) self._wait_for_server_start() @@ -68,20 +69,17 @@ def _wait_for_server_start(self, timeout=20): def kill(self): try: - self._instance.process.kill() + self._instance.kill() shutil.rmtree(self.env["KOLIBRI_HOME"]) except OSError: pass - def manage(self, *args, **kwargs): - sh.kolibri.manage(*args, _env=self.env, **kwargs) - @classmethod def get_pre_migrated_database(cls, home_path): if not cls._pre_migrated_db_dir: # if no pre-migrated db files, then create them server = cls(autostart=False) - server.manage("migrate") + subprocess.run(["kolibri manage migrate"], env=server.env) # put pre-migrated files in temp directory cls._pre_migrated_db_dir = tempfile.mkdtemp() for f in glob.glob(os.path.join(server.env["KOLIBRI_HOME"], "db.sqlite3*")): diff --git a/kolibri/core/auth/test/test_user_import.py b/kolibri/core/auth/test/test_user_import.py index 8f9b27c83ef..1fa66b1dd54 100644 --- a/kolibri/core/auth/test/test_user_import.py +++ b/kolibri/core/auth/test/test_user_import.py @@ -142,6 +142,7 @@ def test_device_not_setup(self): csvfile, csvpath = tempfile.mkstemp(suffix="csv") with self.assertRaisesRegexp(CommandError, "No default facility exists"): call_command("importusers", csvpath) + os.close(csvfile) os.remove(csvpath) @@ -160,6 +161,7 @@ def setUp(self): def tearDown(self): FacilityUser.objects.exclude(username=self.superuser.username).delete() + os.close(self.csvfile) os.remove(self.csvpath) def importFromRows(self, *args): diff --git a/kolibri/core/content/test/test_import_export.py b/kolibri/core/content/test/test_import_export.py index 938f86c8f93..a604c26efbc 100644 --- a/kolibri/core/content/test/test_import_export.py +++ b/kolibri/core/content/test/test_import_export.py @@ -79,7 +79,8 @@ def test_remote_cancel_during_transfer( start_progress_mock, import_channel_mock, ): - local_path = tempfile.mkstemp()[1] + fd, local_path = tempfile.mkstemp() + os.close(fd) local_path_mock.return_value = local_path remote_path_mock.return_value = "notest" FileDownloadMock.return_value.__iter__.side_effect = TransferCanceled() @@ -116,8 +117,10 @@ def test_local_cancel_during_transfer( start_progress_mock, import_channel_mock, ): - local_dest_path = tempfile.mkstemp()[1] - local_src_path = tempfile.mkstemp()[1] + fd1, local_dest_path = tempfile.mkstemp() + fd2, local_src_path = tempfile.mkstemp() + os.close(fd1) + os.close(fd2) local_path_mock.side_effect = [local_dest_path, local_src_path] FileCopyMock.return_value.__iter__.side_effect = TransferCanceled() call_command("importchannel", "disk", self.the_channel_id, tempfile.mkdtemp()) @@ -214,7 +217,8 @@ def test_remote_successful_import_clears_stats_cache( start_progress_mock, import_channel_mock, ): - local_path = tempfile.mkstemp()[1] + fd, local_path = tempfile.mkstemp() + os.close(fd) local_path_mock.return_value = local_path remote_path_mock.return_value = "notest" FileDownloadMock.return_value.__iter__.return_value = ["one", "two", "three"] @@ -295,7 +299,8 @@ def test_remote_cancel_during_transfer( channel_list_status_mock, ): # If transfer is cancelled during transfer of first file - local_path = tempfile.mkstemp()[1] + fd, local_path = tempfile.mkstemp() + os.close(fd) local_path_mock.return_value = local_path remote_path_mock.return_value = "notest" # Mock this __iter__ so that the filetransfer can be looped over @@ -345,8 +350,10 @@ def test_remote_cancel_after_file_copy_file_not_deleted( channel_list_status_mock, ): # If transfer is cancelled after transfer of first file - local_path_1 = tempfile.mkstemp()[1] - local_path_2 = tempfile.mkstemp()[1] + fd1, local_path_1 = tempfile.mkstemp() + fd2, local_path_2 = tempfile.mkstemp() + os.close(fd1) + os.close(fd2) with open(local_path_1, "w") as f: f.write("a") local_path_mock.side_effect = [local_path_1, local_path_2] @@ -410,8 +417,10 @@ def test_local_cancel_during_transfer( channel_list_status_mock, ): # Local version of test above - local_dest_path = tempfile.mkstemp()[1] - local_src_path = tempfile.mkstemp()[1] + fd1, local_dest_path = tempfile.mkstemp() + fd2, local_src_path = tempfile.mkstemp() + os.close(fd1) + os.close(fd2) local_path_mock.side_effect = [local_dest_path, local_src_path] FileCopyMock.return_value.__iter__.side_effect = TransferCanceled() get_import_export_mock.return_value = (1, list(LocalFile.objects.all()), 10) @@ -480,9 +489,12 @@ def test_remote_import_httperror_404( get_import_export_mock, channel_list_status_mock, ): - local_dest_path_1 = tempfile.mkstemp()[1] - local_dest_path_2 = tempfile.mkstemp()[1] - local_dest_path_3 = tempfile.mkstemp()[1] + fd1, local_dest_path_1 = tempfile.mkstemp() + fd2, local_dest_path_2 = tempfile.mkstemp() + fd3, local_dest_path_3 = tempfile.mkstemp() + os.close(fd1) + os.close(fd2) + os.close(fd3) path_mock.side_effect = [ local_dest_path_1, local_dest_path_2, @@ -654,7 +666,8 @@ def test_local_import_oserror_dne( get_import_export_mock, channel_list_status_mock, ): - dest_path = tempfile.mkstemp()[1] + fd, dest_path = tempfile.mkstemp() + os.close(fd) path_mock.side_effect = [dest_path, "/test/dne"] LocalFile.objects.filter( files__contentnode__channel_id=self.the_channel_id @@ -678,7 +691,8 @@ def test_local_import_oserror_permission_denied( get_import_export_mock, channel_list_status_mock, ): - dest_path = tempfile.mkstemp()[1] + fd, dest_path = tempfile.mkstemp() + os.close(fd) path_mock.side_effect = [dest_path, "/test/dne"] getsize_mock.side_effect = ["1", OSError("Permission denied")] get_import_export_mock.return_value = (1, [LocalFile.objects.first()], 10) @@ -711,8 +725,10 @@ def test_local_import_source_corrupted( get_import_export_mock, channel_list_status_mock, ): - local_src_path = tempfile.mkstemp()[1] - local_dest_path = tempfile.mkstemp()[1] + fd1, local_dest_path = tempfile.mkstemp() + fd2, local_src_path = tempfile.mkstemp() + os.close(fd1) + os.close(fd2) LocalFile.objects.filter( files__contentnode="32a941fb77c2576e8f6b294cde4c3b0c" ).update(file_size=1) @@ -770,7 +786,8 @@ def test_local_import_source_corrupted_full_progress( with open(local_src_path, "w") as f: f.write("This is just a test") expected_file_size = 10000 - local_dest_path = tempfile.mkstemp()[1] + fd, local_dest_path = tempfile.mkstemp() + os.close(fd) os.remove(local_dest_path) # Delete all but one file associated with ContentNode to reduce need for mocking files = ContentNode.objects.get( @@ -829,8 +846,10 @@ def test_remote_import_source_corrupted( get_import_export_mock, channel_list_status_mock, ): - dest_path_1 = tempfile.mkstemp()[1] - dest_path_2 = tempfile.mkstemp()[1] + fd1, dest_path_1 = tempfile.mkstemp() + fd2, dest_path_2 = tempfile.mkstemp() + os.close(fd1) + os.close(fd2) path_mock.side_effect = [dest_path_1, dest_path_2] LocalFile.objects.filter(pk="6bdfea4a01830fdd4a585181c0b8068c").update( file_size=2201062 @@ -883,8 +902,10 @@ def test_remote_import_full_import( get_import_export_mock, channel_list_status_mock, ): - dest_path_1 = tempfile.mkstemp()[1] - dest_path_2 = tempfile.mkstemp()[1] + fd1, dest_path_1 = tempfile.mkstemp() + fd2, dest_path_2 = tempfile.mkstemp() + os.close(fd1) + os.close(fd2) path_mock.side_effect = [dest_path_1, dest_path_2] LocalFile.objects.filter(pk="6bdfea4a01830fdd4a585181c0b8068c").update( file_size=2201062 @@ -989,8 +1010,10 @@ def test_cancel_during_transfer( start_progress_mock, ): # Make sure we clean up a database file that is canceled during export - local_dest_path = tempfile.mkstemp()[1] - local_src_path = tempfile.mkstemp()[1] + fd1, local_dest_path = tempfile.mkstemp() + fd2, local_src_path = tempfile.mkstemp() + os.close(fd1) + os.close(fd2) local_path_mock.side_effect = [local_src_path, local_dest_path] FileCopyMock.return_value.__iter__.side_effect = TransferCanceled() call_command("exportchannel", self.the_channel_id, local_dest_path) @@ -1055,8 +1078,10 @@ def test_local_cancel_during_transfer( get_import_export_mock, ): # Make sure we cancel during transfer - local_dest_path = tempfile.mkstemp()[1] - local_src_path = tempfile.mkstemp()[1] + fd1, local_dest_path = tempfile.mkstemp() + fd2, local_src_path = tempfile.mkstemp() + os.close(fd1) + os.close(fd2) local_path_mock.side_effect = [local_src_path, local_dest_path] FileCopyMock.return_value.__iter__.side_effect = TransferCanceled() get_import_export_mock.return_value = (1, [LocalFile.objects.first()], 10) diff --git a/kolibri/core/content/test/test_movedirectory.py b/kolibri/core/content/test/test_movedirectory.py index 4c18bf9cb50..5c8bec0324f 100644 --- a/kolibri/core/content/test/test_movedirectory.py +++ b/kolibri/core/content/test/test_movedirectory.py @@ -1,9 +1,22 @@ +import os +import sys + from django.core.management import call_command from django.test import TestCase from mock import patch from kolibri.utils.conf import OPTIONS +path_prefix = "C:\\" if sys.platform == "win32" else "/" + +success_path = os.path.join(path_prefix, "test", "success") + +no_content_path = os.path.join(path_prefix, "test", "content_exists_no") + +yes_content_path = os.path.join(path_prefix, "test", "content_exists_yes") + +random_content_path = os.path.join(path_prefix, "test", "content_exists_random") + @patch("kolibri.core.content.management.commands.content.update_options_file") class ContentMoveDirectoryTestCase(TestCase): @@ -15,16 +28,16 @@ class ContentMoveDirectoryTestCase(TestCase): def _path_exists_side_effect(*args): if args[0] == OPTIONS["Paths"]["CONTENT_DIR"]: return True - elif args[0].startswith("/test/success"): + elif args[0].startswith(success_path): return False return True def _listdir_side_effect(*args): - if args[0] == OPTIONS["Paths"]["CONTENT_DIR"] + "/databases": + if args[0] == os.path.join(OPTIONS["Paths"]["CONTENT_DIR"], "databases"): return ["test.sqlite3"] - elif args[0] == OPTIONS["Paths"]["CONTENT_DIR"] + "/storage": + elif args[0] == os.path.join(OPTIONS["Paths"]["CONTENT_DIR"], "storage"): return ["test.mp3"] - elif args[0] == "/test/content_exists_no/databases": + elif args[0] == os.path.join(no_content_path, "databases"): return ["exists.sqlite3"] return [] @@ -73,7 +86,7 @@ def test_migrate_while_dest_content_exists_no( remove_mock, update_mock, ): - destination = "/test/content_exists_no" + destination = no_content_path call_command("content", "movedirectory", destination) self.assertEqual(copyfile_mock.call_count, 2) self.assertEqual(remove_mock.call_count, 2) @@ -98,7 +111,7 @@ def test_migrate_while_dest_content_exists_yes( copy_mock, update_mock, ): - destination = "/test/content_exists_yes" + destination = yes_content_path call_command("content", "movedirectory", destination) copy_mock.assert_called() self.assertEqual(remove_mock.call_count, 4) @@ -149,7 +162,7 @@ def test_migrate_while_dest_dir_dne_success( remove_mock, update_mock, ): - destination = "/test/success" + destination = success_path call_command("content", "movedirectory", destination) remove_mock.assert_called() mkdir_mock.assert_called() diff --git a/kolibri/core/discovery/utils/filesystem/windows.py b/kolibri/core/discovery/utils/filesystem/windows.py index 26cca9e85c8..bffd91ceff4 100644 --- a/kolibri/core/discovery/utils/filesystem/windows.py +++ b/kolibri/core/discovery/utils/filesystem/windows.py @@ -84,18 +84,21 @@ def _wmic_output(): tempfile.gettempdir(), "kolibri_disks-{}.txt".format(uuid.uuid4()) ) - # pipe output from the WMIC command to the temp file - csv_path = os.path.join( - os.environ["WINDIR"], "System32", "wbem", "en-us", "csv.xsl" - ) - # Use different WMIC commands, depending on whether the csv_path exists. - if os.path.exists(csv_path): - cmd = 'wmic logicaldisk list full /format:"{}" > "{}"'.format( - csv_path, OUTPUT_PATH + # fallback when en-us directory does not exist + cmd = 'wmic logicaldisk list full /format:csv > "{}"'.format(OUTPUT_PATH) + try: + # pipe output from the WMIC command to the temp file + csv_path = os.path.join( + os.environ["WINDIR"], "System32", "wbem", "en-us", "csv.xsl" ) - else: - # fallback when en-us directory does not exist - cmd = 'wmic logicaldisk list full /format:csv > "{}"'.format(OUTPUT_PATH) + # If csv_path exists, use a different WMIC command. + if os.path.exists(csv_path): + cmd = 'wmic logicaldisk list full /format:"{}" > "{}"'.format( + csv_path, OUTPUT_PATH + ) + except KeyError: + # If WINDIR is undefined on env + pass returnCode = os.system(cmd) if returnCode: raise Exception("Could not run command '{}'".format(cmd)) diff --git a/kolibri/core/logger/csv_export.py b/kolibri/core/logger/csv_export.py index 0e2ff6c9ff3..6526e9ae2ba 100644 --- a/kolibri/core/logger/csv_export.py +++ b/kolibri/core/logger/csv_export.py @@ -154,7 +154,7 @@ def csv_file_generator(facility, log_type, filepath, overwrite=False): if sys.version_info[0] < 3: csv_file = io.open(filepath, "wb") else: - csv_file = io.open(filepath, "w", newline="") + csv_file = io.open(filepath, "w", newline="", encoding="utf-8") with csv_file as f: writer = csv.DictWriter(f, header_labels) diff --git a/kolibri/core/logger/test/test_api.py b/kolibri/core/logger/test/test_api.py index 7d39e7a92b2..6adada8577b 100644 --- a/kolibri/core/logger/test/test_api.py +++ b/kolibri/core/logger/test/test_api.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- """ Tests that ensure the correct items are returned from api calls. Also tests whether the users with permissions can create logs. @@ -556,6 +557,32 @@ def test_csv_download_deleted_content(self): self.assertEqual(len(results[0]), len(row)) self.assertEqual(len(results[1:]), expected_count) + def test_csv_download_unicode_username(self): + user = FacilityUserFactory.create( + facility=self.facility, username="كوليبري", full_name="كوليبري" + ) + for _ in range(3): + ContentSummaryLogFactory.create( + user=user, + content_id=uuid.uuid4().hex, + channel_id="6199dde695db4ee4ab392222d5af1e5c", + ) + + expected_count = ContentSummaryLog.objects.count() + _, filepath = tempfile.mkstemp(suffix=".csv") + call_command( + "exportlogs", log_type="summary", output_file=filepath, overwrite=True + ) + if sys.version_info[0] < 3: + csv_file = open(filepath, "rb") + else: + csv_file = open(filepath, "r", newline="") + with csv_file as f: + results = list(csv.reader(f)) + for row in results[1:]: + self.assertEqual(len(results[0]), len(row)) + self.assertEqual(len(results[1:]), expected_count) + class ContentSessionLogCSVExportTestCase(APITestCase): @@ -612,6 +639,32 @@ def test_csv_download_deleted_content(self): self.assertEqual(len(results[0]), len(row)) self.assertEqual(len(results[1:]), expected_count) + def test_csv_download_unicode_username(self): + user = FacilityUserFactory.create( + facility=self.facility, username="كوليبري", full_name="كوليبري" + ) + for _ in range(3): + ContentSessionLogFactory.create( + user=user, + content_id=uuid.uuid4().hex, + channel_id="6199dde695db4ee4ab392222d5af1e5c", + ) + + expected_count = ContentSessionLog.objects.count() + _, filepath = tempfile.mkstemp(suffix=".csv") + call_command( + "exportlogs", log_type="session", output_file=filepath, overwrite=True + ) + if sys.version_info[0] < 3: + csv_file = open(filepath, "rb") + else: + csv_file = open(filepath, "r", newline="") + with csv_file as f: + results = list(csv.reader(f)) + for row in results[1:]: + self.assertEqual(len(results[0]), len(row)) + self.assertEqual(len(results[1:]), expected_count) + class ExamAttemptLogAPITestCase(APITestCase): @classmethod diff --git a/kolibri/core/tasks/api.py b/kolibri/core/tasks/api.py index 8b26251d2c6..4139b34f5b7 100644 --- a/kolibri/core/tasks/api.py +++ b/kolibri/core/tasks/api.py @@ -3,7 +3,7 @@ import os import shutil from functools import partial -from tempfile import NamedTemporaryFile +from tempfile import mkstemp import requests from django.apps.registry import AppRegistryNotReady @@ -643,9 +643,8 @@ def manage_fileobject(request, temp_dir): # Django uses InMemoryUploadedFile for files less than 2.5Mb # and TemporaryUploadedFile for bigger files: if type(upload.file) == InMemoryUploadedFile: - with NamedTemporaryFile( - dir=temp_dir, suffix=".upload", delete=False - ) as dest: + _, filepath = mkstemp(dir=temp_dir, suffix=".upload") + with open(filepath, "w+b") as dest: filepath = dest.name for chunk in upload.file.chunks(): dest.write(chunk) diff --git a/kolibri/core/tasks/test/test_job_running.py b/kolibri/core/tasks/test/test_job_running.py index d2abc8f6ba6..c8a6f6a5077 100644 --- a/kolibri/core/tasks/test/test_job_running.py +++ b/kolibri/core/tasks/test/test_job_running.py @@ -1,3 +1,4 @@ +import os import tempfile import time import uuid @@ -20,30 +21,37 @@ @pytest.fixture def backend(): - with tempfile.NamedTemporaryFile() as f: - connection = create_engine( - "sqlite:///{path}".format(path=f.name), - connect_args={"check_same_thread": False}, - poolclass=NullPool, - ) - b = Storage(connection) - yield b - b.clear() + fd, filepath = tempfile.mkstemp() + connection = create_engine( + "sqlite:///{path}".format(path=filepath), + connect_args={"check_same_thread": False}, + poolclass=NullPool, + ) + b = Storage(connection) + yield b + b.clear() + os.close(fd) + os.remove(filepath) @pytest.fixture def inmem_queue(): - with tempfile.NamedTemporaryFile() as f: - connection = create_engine( - "sqlite:///{path}".format(path=f.name), - connect_args={"check_same_thread": False}, - poolclass=NullPool, - ) - e = Worker(queues="pytest", connection=connection) - c = Queue(queue="pytest", connection=connection) - c.e = e - yield c - e.shutdown() + fd, filepath = tempfile.mkstemp() + connection = create_engine( + "sqlite:///{path}".format(path=filepath), + connect_args={"check_same_thread": False}, + poolclass=NullPool, + ) + e = Worker(queues="pytest", connection=connection) + c = Queue(queue="pytest", connection=connection) + c.e = e + yield c + e.shutdown() + os.close(fd) + try: + os.remove(filepath) + except OSError: + pass @pytest.fixture diff --git a/kolibri/core/tasks/test/test_scheduler.py b/kolibri/core/tasks/test/test_scheduler.py index 122da193c35..c13f39440d0 100644 --- a/kolibri/core/tasks/test/test_scheduler.py +++ b/kolibri/core/tasks/test/test_scheduler.py @@ -1,4 +1,5 @@ import datetime +import os import tempfile import pytest @@ -14,14 +15,16 @@ @pytest.fixture def queue(): - with tempfile.NamedTemporaryFile() as f: - connection = create_engine( - "sqlite:///{path}".format(path=f.name), - connect_args={"check_same_thread": False}, - poolclass=NullPool, - ) - q = Queue("pytest", connection) - yield q + fd, filepath = tempfile.mkstemp() + connection = create_engine( + "sqlite:///{path}".format(path=filepath), + connect_args={"check_same_thread": False}, + poolclass=NullPool, + ) + q = Queue("pytest", connection) + yield q + os.close(fd) + os.remove(filepath) @pytest.fixture diff --git a/kolibri/core/tasks/test/test_storage.py b/kolibri/core/tasks/test/test_storage.py index 920c6e6624f..b1526c9f6c3 100644 --- a/kolibri/core/tasks/test/test_storage.py +++ b/kolibri/core/tasks/test/test_storage.py @@ -1,3 +1,4 @@ +import os import tempfile import pytest @@ -15,15 +16,17 @@ @pytest.fixture def defaultbackend(): - with tempfile.NamedTemporaryFile() as f: - connection = create_engine( - "sqlite:///{path}".format(path=f.name), - connect_args={"check_same_thread": False}, - poolclass=NullPool, - ) - b = Storage(connection) - yield b - b.clear() + fd, filepath = tempfile.mkstemp() + connection = create_engine( + "sqlite:///{path}".format(path=filepath), + connect_args={"check_same_thread": False}, + poolclass=NullPool, + ) + b = Storage(connection) + yield b + b.clear() + os.close(fd) + os.remove(filepath) @pytest.fixture diff --git a/kolibri/core/tasks/test/test_worker.py b/kolibri/core/tasks/test/test_worker.py index f025c6dc99e..39e17a2c3a3 100644 --- a/kolibri/core/tasks/test/test_worker.py +++ b/kolibri/core/tasks/test/test_worker.py @@ -1,3 +1,4 @@ +import os import tempfile import time @@ -16,15 +17,19 @@ @pytest.fixture def worker(): - with tempfile.NamedTemporaryFile() as f: - connection = create_engine( - "sqlite:///{path}".format(path=f.name), - connect_args={"check_same_thread": False}, - poolclass=NullPool, - ) - b = Worker(QUEUE, connection) - yield b - b.shutdown() + _, filepath = tempfile.mkstemp() + connection = create_engine( + "sqlite:///{path}".format(path=filepath), + connect_args={"check_same_thread": False}, + poolclass=NullPool, + ) + b = Worker(QUEUE, connection) + yield b + b.shutdown() + try: + os.remove(filepath) + except OSError: + pass class TestWorker: diff --git a/kolibri/core/webpack/test/base.py b/kolibri/core/webpack/test/base.py index 144d8fdacff..8b6d3bccfe9 100644 --- a/kolibri/core/webpack/test/base.py +++ b/kolibri/core/webpack/test/base.py @@ -34,7 +34,8 @@ class HookMixin(object): @property def _stats_file(self): - self.TEST_STATS_FILE = tempfile.NamedTemporaryFile(mode="w+", delete=False) + _, filename = tempfile.mkstemp() + self.TEST_STATS_FILE = open(filename, mode="w+") self.TEST_STATS_FILE_DATA = copy.deepcopy(TEST_STATS_FILE_DATA) self.TEST_STATS_FILE_DATA["chunks"][self.unique_id] = self.TEST_STATS_FILE_DATA[ "chunks" diff --git a/kolibri/utils/tests/test_options.py b/kolibri/utils/tests/test_options.py index 6e0baf3ce67..e5c73ae5d7f 100644 --- a/kolibri/utils/tests/test_options.py +++ b/kolibri/utils/tests/test_options.py @@ -7,6 +7,7 @@ import logging import os +import sys import tempfile import mock @@ -44,7 +45,10 @@ def test_option_reading_and_precedence_rules(): """ Checks that options can be read from a dummy options.ini file, and overridden by env vars. """ - _CONTENT_DIR = "/mycontentdir" + if sys.platform == "win32": + _CONTENT_DIR = "C:\\mycontentdir" + else: + _CONTENT_DIR = "/mycontentdir" _HTTP_PORT_INI = 7007 _HTTP_PORT_ENV = 9009 @@ -135,8 +139,12 @@ def test_option_writing(): """ Checks that options can be written to a dummy options.ini file, validated, and then read back. """ - _OLD_CONTENT_DIR = "/mycontentdir" - _NEW_CONTENT_DIR = "/goodnessme" + if sys.platform == "win32": + _OLD_CONTENT_DIR = "C:\\mycontentdir" + _NEW_CONTENT_DIR = "C:\\goodnessme" + else: + _OLD_CONTENT_DIR = "/mycontentdir" + _NEW_CONTENT_DIR = "/goodnessme" _HTTP_PORT_GOOD = 7007 _HTTP_PORT_BAD = "abba" @@ -202,11 +210,13 @@ def test_path_expansion(): _, tmp_ini_path = tempfile.mkstemp(prefix="options", suffix=".ini") - with mock.patch.dict(os.environ, {"KOLIBRI_CONTENT_DIR": "/absolute"}): + absolute_path = "C:\\absolute" if sys.platform == "win32" else "/absolute" + + with mock.patch.dict(os.environ, {"KOLIBRI_CONTENT_DIR": absolute_path}): OPTIONS = options.read_options_file( KOLIBRI_HOME_TEMP, ini_filename=tmp_ini_path ) - assert OPTIONS["Paths"]["CONTENT_DIR"] == "/absolute" + assert OPTIONS["Paths"]["CONTENT_DIR"] == absolute_path with mock.patch.dict(os.environ, {"KOLIBRI_CONTENT_DIR": "relative"}): OPTIONS = options.read_options_file( @@ -216,10 +226,10 @@ def test_path_expansion(): KOLIBRI_HOME_TEMP, "relative" ) - with mock.patch.dict(os.environ, {"KOLIBRI_CONTENT_DIR": "~/homeiswherethecatis"}): + user_path = os.path.join("~", "homeiswherethecatis") + + with mock.patch.dict(os.environ, {"KOLIBRI_CONTENT_DIR": user_path}): OPTIONS = options.read_options_file( KOLIBRI_HOME_TEMP, ini_filename=tmp_ini_path ) - assert OPTIONS["Paths"]["CONTENT_DIR"] == os.path.expanduser( - "~/homeiswherethecatis" - ) + assert OPTIONS["Paths"]["CONTENT_DIR"] == os.path.expanduser(user_path) diff --git a/kolibri/utils/tests/test_sanity_check.py b/kolibri/utils/tests/test_sanity_check.py index 3d86f4d86b6..da96401f977 100644 --- a/kolibri/utils/tests/test_sanity_check.py +++ b/kolibri/utils/tests/test_sanity_check.py @@ -1,4 +1,5 @@ import os +import sys import tempfile import portend @@ -41,7 +42,9 @@ def test_socket_activation_support(self, portend_mock, logging_mock): @patch("kolibri.utils.cli.get_version", return_value="") @patch("kolibri.utils.sanity_checks.logging.error") - @override_option("Paths", "CONTENT_DIR", "/dir_dne") + @override_option( + "Paths", "CONTENT_DIR", "Z:\\NOTREAL" if sys.platform == "win32" else "/dir_dne" + ) def test_content_dir_dne(self, logging_mock, get_version): with self.assertRaises(SystemExit): sanity_checks.check_content_directory_exists_and_writable() diff --git a/requirements/test.txt b/requirements/test.txt index daa75c6685a..1e5d576e67c 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -10,4 +10,3 @@ pytest-cov==2.5.1 pytest-django==3.3.3 pytest-env==0.6.2 pytest-pythonpath==0.7.2 -sh==1.12.14