From 5b66e2804622b228abe68939d221b3545c8574b9 Mon Sep 17 00:00:00 2001 From: Craig de Stigter Date: Wed, 14 Feb 2024 08:24:26 +1300 Subject: [PATCH 1/5] Fix tests with moto 5 ; require it --- tests/test_s3.py | 6 +++--- tox.ini | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_s3.py b/tests/test_s3.py index 2d4e3e53..8707ec3a 100644 --- a/tests/test_s3.py +++ b/tests/test_s3.py @@ -19,7 +19,7 @@ from django.test import TestCase from django.test import override_settings from django.utils.timezone import is_aware -from moto import mock_s3 +from moto import mock_aws from storages.backends import s3 from tests.utils import NonSeekableContentFile @@ -1059,10 +1059,10 @@ def test_reopening(self): self.assertIsNone(f._multipart) -@mock_s3 +@mock_aws class S3StorageTestsWithMoto(TestCase): """ - Using mock_s3 as a class decorator automatically decorates methods, + Using mock_aws as a class decorator automatically decorates methods, but NOT classmethods or staticmethods. """ diff --git a/tox.ini b/tox.ini index aecb5ef2..44d34764 100644 --- a/tox.ini +++ b/tox.ini @@ -21,7 +21,7 @@ deps = django4.2: django~=4.2.0 django5.0: django~=5.0b1 djangomain: https://github.com/django/django/archive/main.tar.gz - moto + moto>=5.0 pytest pytest-cov rsa From d16c486db935b6241243a7f819d27198bcc94c83 Mon Sep 17 00:00:00 2001 From: Craig de Stigter Date: Wed, 14 Feb 2024 08:34:43 +1300 Subject: [PATCH 2/5] tox: make it possible to run specific test files --- tox.ini | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 44d34764..ecaab44c 100644 --- a/tox.ini +++ b/tox.ini @@ -13,7 +13,7 @@ setenv = DJANGO_SETTINGS_MODULE = tests.settings PYTHONWARNINGS = always PYTHONDONTWRITEBYTECODE = 1 -commands = pytest --cov=storages tests/ {posargs} +commands = pytest --cov=storages {posargs} deps = cryptography django3.2: django~=3.2.9 @@ -41,3 +41,8 @@ commands = ruff . black --check . skip_install = true + +[pytest] +# Default test paths to run, if no other paths are specified on the CLI +# (specify paths after a -- e.g. `tox -- tests/test_s3.py`) +testpaths = tests/ From 795c82fab2d6a7de3bcd4d6b6c46f840f2c43418 Mon Sep 17 00:00:00 2001 From: Craig de Stigter Date: Wed, 14 Feb 2024 08:44:02 +1300 Subject: [PATCH 3/5] s3: Failing test for universal newline handling --- tests/test_s3.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/test_s3.py b/tests/test_s3.py index 8707ec3a..b4aeaac4 100644 --- a/tests/test_s3.py +++ b/tests/test_s3.py @@ -305,11 +305,25 @@ def test_storage_open_read_string(self): self.assertEqual(content_str, "") file.close() + def test_storage_open_read_with_newlines(self): + """ + Test opening a file in "r" mode with various newline characters + """ + name = "test_storage_open_read_with_newlines.txt" + with io.BytesIO() as temp_file: + temp_file.write(b"line1\nline2\r\nmore\rtext\n") + temp_file.seek(0) + file = self.storage.open(name, "r") + file._file = temp_file + content_str = file.read() + file.close() + self.assertEqual(content_str, "line1\nline2\nmore\ntext\n") + def test_storage_open_readlines(self): """ Test readlines with file opened in "r" and "rb" modes """ - name = "test_open_readlines_string.txt" + name = "test_storage_open_readlines.txt" with io.BytesIO() as temp_file: temp_file.write(b"line1\nline2") file = self.storage.open(name, "r") @@ -324,6 +338,19 @@ def test_storage_open_readlines(self): content_lines = file.readlines() self.assertEqual(content_lines, [b"line1\n", b"line2"]) + def test_storage_open_readlines_with_newlines(self): + """ + Test readlines with file opened in "r" mode with various newline characters + """ + name = "test_storage_open_readlines_with_newlines.txt" + with io.BytesIO() as temp_file: + temp_file.write(b"line1\nline2\r\nmore\rtext") + file = self.storage.open(name, "r") + file._file = temp_file + + content_lines = file.readlines() + self.assertEqual(content_lines, ['line1\n', 'line2\n', 'more\n', 'text']) + def test_storage_open_write(self): """ Test opening a file in write mode From 9b1b76f73e0cf1e358af162caa94e218328bc82f Mon Sep 17 00:00:00 2001 From: Sooyong Kim Date: Wed, 14 Feb 2024 16:36:33 +1300 Subject: [PATCH 4/5] Remove _force_mode: Wrap non gzip files in r mode with TextIOWrapper to support universal newlines --- storages/backends/s3.py | 8 ++-- tests/test_s3.py | 88 ++++++++++++++++++++--------------------- 2 files changed, 47 insertions(+), 49 deletions(-) diff --git a/storages/backends/s3.py b/storages/backends/s3.py index ef915164..dc7938bc 100644 --- a/storages/backends/s3.py +++ b/storages/backends/s3.py @@ -1,5 +1,6 @@ import mimetypes import os +import io import posixpath import tempfile import threading @@ -123,7 +124,6 @@ def __init__(self, name, mode, storage, buffer_size=None): self._storage = storage self.name = name[len(self._storage.location) :].lstrip("/") self._mode = mode - self._force_mode = (lambda b: b) if "b" in mode else (lambda b: b.decode()) self.obj = storage.bucket.Object(name) if "w" not in mode: # Force early RAII-style exception if object does not exist @@ -184,6 +184,8 @@ def _get_file(self): self._file.seek(0) if self._storage.gzip and self.obj.content_encoding == "gzip": self._file = self._decompress_file(mode=self._mode, file=self._file) + elif "b" not in self._mode: + self._file = io.TextIOWrapper(self._file._file, encoding="utf-8") self._closed = False return self._file @@ -195,12 +197,12 @@ def _set_file(self, value): def read(self, *args, **kwargs): if "r" not in self._mode: raise AttributeError("File was not opened in read mode.") - return self._force_mode(super().read(*args, **kwargs)) + return super().read(*args, **kwargs) def readline(self, *args, **kwargs): if "r" not in self._mode: raise AttributeError("File was not opened in read mode.") - return self._force_mode(super().readline(*args, **kwargs)) + return super().readline(*args, **kwargs) def readlines(self): return list(self) diff --git a/tests/test_s3.py b/tests/test_s3.py index b4aeaac4..e4ce9f1a 100644 --- a/tests/test_s3.py +++ b/tests/test_s3.py @@ -305,52 +305,6 @@ def test_storage_open_read_string(self): self.assertEqual(content_str, "") file.close() - def test_storage_open_read_with_newlines(self): - """ - Test opening a file in "r" mode with various newline characters - """ - name = "test_storage_open_read_with_newlines.txt" - with io.BytesIO() as temp_file: - temp_file.write(b"line1\nline2\r\nmore\rtext\n") - temp_file.seek(0) - file = self.storage.open(name, "r") - file._file = temp_file - content_str = file.read() - file.close() - self.assertEqual(content_str, "line1\nline2\nmore\ntext\n") - - def test_storage_open_readlines(self): - """ - Test readlines with file opened in "r" and "rb" modes - """ - name = "test_storage_open_readlines.txt" - with io.BytesIO() as temp_file: - temp_file.write(b"line1\nline2") - file = self.storage.open(name, "r") - file._file = temp_file - - content_lines = file.readlines() - self.assertEqual(content_lines, ["line1\n", "line2"]) - - temp_file.seek(0) - file = self.storage.open(name, "rb") - file._file = temp_file - content_lines = file.readlines() - self.assertEqual(content_lines, [b"line1\n", b"line2"]) - - def test_storage_open_readlines_with_newlines(self): - """ - Test readlines with file opened in "r" mode with various newline characters - """ - name = "test_storage_open_readlines_with_newlines.txt" - with io.BytesIO() as temp_file: - temp_file.write(b"line1\nline2\r\nmore\rtext") - file = self.storage.open(name, "r") - file._file = temp_file - - content_lines = file.readlines() - self.assertEqual(content_lines, ['line1\n', 'line2\n', 'more\n', 'text']) - def test_storage_open_write(self): """ Test opening a file in write mode @@ -1172,6 +1126,48 @@ def test_content_type_not_detectable(self): s3.S3Storage.default_content_type, ) + def test_storage_open_read_with_newlines(self): + """ + Test opening a file in "r" and "rb" mode with various newline characters + """ + name = "test_storage_open_read_with_newlines.txt" + with io.BytesIO() as temp_file: + temp_file.write(b"line1\nline2\r\nmore\rtext\n") + self.storage.save(name, temp_file) + file = self.storage.open(name, "r") + content_str = file.read() + file.close() + self.assertEqual(content_str, "line1\nline2\nmore\ntext\n") + + with io.BytesIO() as temp_file: + temp_file.write(b"line1\nline2\r\nmore\rtext\n") + self.storage.save(name, temp_file) + file = self.storage.open(name, "rb") + content_str = file.read() + file.close() + self.assertEqual(content_str, b"line1\nline2\r\nmore\rtext\n") + + def test_storage_open_readlines_with_newlines(self): + """ + Test readlines with file opened in "r" and "rb" mode with various newline characters + """ + name = "test_storage_open_readlines_with_newlines.txt" + with io.BytesIO() as temp_file: + temp_file.write(b"line1\nline2\r\nmore\rtext") + self.storage.save(name, temp_file) + file = self.storage.open(name, "r") + content_lines = file.readlines() + file.close() + self.assertEqual(content_lines, ['line1\n', 'line2\n', 'more\n', 'text']) + + with io.BytesIO() as temp_file: + temp_file.write(b"line1\nline2\r\nmore\rtext") + self.storage.save(name, temp_file) + file = self.storage.open(name, "rb") + content_lines = file.readlines() + file.close() + self.assertEqual(content_lines, [b'line1\n', b'line2\r\n', b'more\r', b'text']) + class TestBackwardsNames(TestCase): def test_importing(self): From 489a394b1cee7bed4cb14ad111ce0710edb2710b Mon Sep 17 00:00:00 2001 From: Craig de Stigter Date: Tue, 12 Mar 2024 12:42:42 +1300 Subject: [PATCH 5/5] Pin to moto<5 until we drop python 3.7 support --- tests/test_s3.py | 6 +++--- tox.ini | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_s3.py b/tests/test_s3.py index e4ce9f1a..33a6277e 100644 --- a/tests/test_s3.py +++ b/tests/test_s3.py @@ -19,7 +19,7 @@ from django.test import TestCase from django.test import override_settings from django.utils.timezone import is_aware -from moto import mock_aws +from moto import mock_s3 from storages.backends import s3 from tests.utils import NonSeekableContentFile @@ -1040,10 +1040,10 @@ def test_reopening(self): self.assertIsNone(f._multipart) -@mock_aws +@mock_s3 class S3StorageTestsWithMoto(TestCase): """ - Using mock_aws as a class decorator automatically decorates methods, + Using mock_s3 as a class decorator automatically decorates methods, but NOT classmethods or staticmethods. """ diff --git a/tox.ini b/tox.ini index ecaab44c..e6e2c6aa 100644 --- a/tox.ini +++ b/tox.ini @@ -21,7 +21,8 @@ deps = django4.2: django~=4.2.0 django5.0: django~=5.0b1 djangomain: https://github.com/django/django/archive/main.tar.gz - moto>=5.0 + # 5 changes API a bit and also requires python 3.8+, so keep this until we drop py 3.7 + moto<5 pytest pytest-cov rsa