From 4a5345f01114235a3ee77af3fc629dcc46e25b7e Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Fri, 3 Dec 2021 13:15:11 +0000 Subject: [PATCH 1/2] Move Spreadsheet models tests into own file Previously this class was in app/utils.py, so it made sense for the tests to be in "utils/test_csv.py". Since the class is now a proper model [1], we can also move the tests into their own file. [1]: https://github.com/alphagov/notifications-admin/commit/2c46d023da151246271197c4ae95199ffa559432#diff-ac7b8d56e509c921efaadb5a776e1c9037531c4d2af78787f06f67a1f3533ae4 --- tests/app/models/test_spreadsheet.py | 42 ++++++++++++++++++++++++++ tests/app/utils/test_csv.py | 45 ++-------------------------- 2 files changed, 44 insertions(+), 43 deletions(-) create mode 100644 tests/app/models/test_spreadsheet.py diff --git a/tests/app/models/test_spreadsheet.py b/tests/app/models/test_spreadsheet.py new file mode 100644 index 000000000..54fa925f6 --- /dev/null +++ b/tests/app/models/test_spreadsheet.py @@ -0,0 +1,42 @@ +from collections import OrderedDict +from pathlib import Path + +import pytest + +from app.models.spreadsheet import Spreadsheet + + +def test_can_create_spreadsheet_from_large_excel_file(): + with open(str(Path.cwd() / 'tests' / 'spreadsheet_files' / 'excel 2007.xlsx'), 'rb') as xl: + ret = Spreadsheet.from_file(xl, filename='xl.xlsx') + assert ret.as_csv_data + + +def test_can_create_spreadsheet_from_dict(): + assert Spreadsheet.from_dict(OrderedDict( + foo='bar', + name='Jane', + )).as_csv_data == ( + "foo,name\r\n" + "bar,Jane\r\n" + ) + + +def test_can_create_spreadsheet_from_dict_with_filename(): + assert Spreadsheet.from_dict({}, filename='empty.csv').as_dict['file_name'] == "empty.csv" + + +@pytest.mark.parametrize('args, kwargs', ( + ( + ('hello', ['hello']), + {}, + ), + ( + (), + {'csv_data': 'hello', 'rows': ['hello']} + ), +)) +def test_spreadsheet_checks_for_bad_arguments(args, kwargs): + with pytest.raises(TypeError) as exception: + Spreadsheet(*args, **kwargs) + assert str(exception.value) == 'Spreadsheet must be created from either rows or CSV data' diff --git a/tests/app/utils/test_csv.py b/tests/app/utils/test_csv.py index 68328e18a..6ceded4e1 100644 --- a/tests/app/utils/test_csv.py +++ b/tests/app/utils/test_csv.py @@ -1,15 +1,10 @@ -from collections import OrderedDict, namedtuple +from collections import namedtuple from csv import DictReader from io import StringIO -from pathlib import Path import pytest -from app.utils.csv import ( - Spreadsheet, - generate_notifications_csv, - get_errors_for_csv, -) +from app.utils.csv import generate_notifications_csv, get_errors_for_csv from tests.conftest import fake_uuid @@ -79,42 +74,6 @@ def _get_notifications_csv_mock( ) -def test_can_create_spreadsheet_from_large_excel_file(): - with open(str(Path.cwd() / 'tests' / 'spreadsheet_files' / 'excel 2007.xlsx'), 'rb') as xl: - ret = Spreadsheet.from_file(xl, filename='xl.xlsx') - assert ret.as_csv_data - - -def test_can_create_spreadsheet_from_dict(): - assert Spreadsheet.from_dict(OrderedDict( - foo='bar', - name='Jane', - )).as_csv_data == ( - "foo,name\r\n" - "bar,Jane\r\n" - ) - - -def test_can_create_spreadsheet_from_dict_with_filename(): - assert Spreadsheet.from_dict({}, filename='empty.csv').as_dict['file_name'] == "empty.csv" - - -@pytest.mark.parametrize('args, kwargs', ( - ( - ('hello', ['hello']), - {}, - ), - ( - (), - {'csv_data': 'hello', 'rows': ['hello']} - ), -)) -def test_spreadsheet_checks_for_bad_arguments(args, kwargs): - with pytest.raises(TypeError) as exception: - Spreadsheet(*args, **kwargs) - assert str(exception.value) == 'Spreadsheet must be created from either rows or CSV data' - - @pytest.mark.parametrize('created_by_name, expected_content', [ ( None, [ From 3c79675ba24a9dc8bb49195477b868b15d0ca37f Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Fri, 3 Dec 2021 13:19:03 +0000 Subject: [PATCH 2/2] Remove unused properties in Spreadsheet class These are no longer used anywhere. Note that pyexcel-xlsx is still a necessary dependency: it acts as an implicit plugin to pyexcel that, surprisingly, allows pyexcel to...read excel files [1]. [1]: https://github.com/pyexcel/pyexcel-xlsx#as-a-pyexcel-plugin --- app/models/spreadsheet.py | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/app/models/spreadsheet.py b/app/models/spreadsheet.py index 3621d4e93..7b66a7b0f 100644 --- a/app/models/spreadsheet.py +++ b/app/models/spreadsheet.py @@ -1,9 +1,8 @@ import csv -from io import BytesIO, StringIO +from io import StringIO from os import path import pyexcel -import pyexcel_xlsx class Spreadsheet(): @@ -87,19 +86,3 @@ class Spreadsheet(): form.file.data, filename=form.file.data.filename, ) - - @property - def as_rows(self): - if not self._rows: - self._rows = list(csv.reader( - StringIO(self._csv_data), - quoting=csv.QUOTE_MINIMAL, - skipinitialspace=True, - )) - return self._rows - - @property - def as_excel_file(self): - io = BytesIO() - pyexcel_xlsx.save_data(io, {'Sheet 1': self.as_rows}) - return io.getvalue()