From 443042bcbe2e2dcfc64def0e1e27427c0e681a0a Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Thu, 30 Jan 2020 11:14:34 +0000 Subject: [PATCH 01/44] Bump pytest from 5.3.4 to 5.3.5 Bumps [pytest](https://github.com/pytest-dev/pytest) from 5.3.4 to 5.3.5. - [Release notes](https://github.com/pytest-dev/pytest/releases) - [Changelog](https://github.com/pytest-dev/pytest/blob/master/CHANGELOG.rst) - [Commits](https://github.com/pytest-dev/pytest/compare/5.3.4...5.3.5) Signed-off-by: dependabot-preview[bot] --- requirements_dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_dev.txt b/requirements_dev.txt index 902d2a5f..43872886 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -1,4 +1,4 @@ -pytest==5.3.4 +pytest==5.3.5 # Running tests on Travis codacy-coverage==1.3.11 From c25bc7b11533e3779f46f6de6093b3d99c427205 Mon Sep 17 00:00:00 2001 From: "allcontributors[bot]" <46447321+allcontributors[bot]@users.noreply.github.com> Date: Fri, 31 Jan 2020 17:13:13 +0000 Subject: [PATCH 02/44] docs: update README.md [skip ci] --- README.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/README.md b/README.md index 2ee1dda6..020e697e 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,7 @@ ## dionysus - Avatar chart generator + +[![All Contributors](https://img.shields.io/badge/all_contributors-1-orange.svg?style=flat-square)](#contributors-) + [![Build Status](https://travis-ci.org/toonarmycaptain/dionysus.svg?branch=master)](https://travis-ci.org/toonarmycaptain/dionysus) [![CircleCI](https://circleci.com/gh/toonarmycaptain/dionysus/tree/master.svg?style=svg)](https://circleci.com/gh/toonarmycaptain/dionysus/tree/master) [![Build status](https://ci.appveyor.com/api/projects/status/yb33uwd13tkv7l79?svg=true)](https://ci.appveyor.com/project/toonarmycaptain/dionysus) @@ -36,3 +39,22 @@ Either click on `app_main.py` in the project folder, or navigate to the folder a #### Versioning As much as possible this project will follow [Semantic Versioning](https://semver.org/). + +## Contributors ✨ + +Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/docs/en/emoji-key)): + + + + + + + + +

Piotr Czajka
💻 ⚠️
+ + + + + +This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome! \ No newline at end of file From 875427e844f2af4467a52ba4c85e4b8b5eff173d Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Fri, 31 Jan 2020 15:21:50 -0600 Subject: [PATCH 03/44] Added contributer tags via @all-contributors, in rough order of contribution magnitude. docs: add schedutron as a contributor docs: add AbdullahElagha as a contributor docs: add HCamberos as a contributor docs: add mbarakaja as a contributor docs: add destag as a contributor docs: add malexanderboyd as a contributor docs: add Ginkooo as a contributor docs: add toonarmycaptain as a contributor docs: create .all-contributorsrc [skip ci] docs: update README.md [skip ci] --- .all-contributorsrc | 94 +++++++++++++++++++++++++++++++++++++++++++++ README.md | 14 +++++-- 2 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 .all-contributorsrc diff --git a/.all-contributorsrc b/.all-contributorsrc new file mode 100644 index 00000000..a3b4c07b --- /dev/null +++ b/.all-contributorsrc @@ -0,0 +1,94 @@ +{ + "files": [ + "README.md" + ], + "imageSize": 100, + "commit": false, + "contributors": [ + { + "login": "HCamberos", + "name": "HCamberos", + "avatar_url": "https://avatars2.githubusercontent.com/u/56201325?v=4", + "profile": "https://github.com/HCamberos", + "contributions": [ + "code" + ] + }, + { + "login": "mbarakaja", + "name": "José María Domínguez", + "avatar_url": "https://avatars0.githubusercontent.com/u/7861175?v=4", + "profile": "https://github.com/mbarakaja", + "contributions": [ + "infra" + ] + }, + { + "login": "destag", + "name": "Przemysław Pietras", + "avatar_url": "https://avatars2.githubusercontent.com/u/16159069?v=4", + "profile": "https://github.com/destag", + "contributions": [ + "infra", + "doc" + ] + }, + { + "login": "malexanderboyd", + "name": "M. Alex Boyd", + "avatar_url": "https://avatars2.githubusercontent.com/u/2465264?v=4", + "profile": "https://github.com/malexanderboyd", + "contributions": [ + "code", + "test" + ] + }, + { + "login": "Ginkooo", + "name": "Piotr Czajka", + "avatar_url": "https://avatars3.githubusercontent.com/u/11911709?v=4", + "profile": "https://github.com/Ginkooo", + "contributions": [ + "code", + "test" + ] + }, + { + "login": "toonarmycaptain", + "name": "toonarmycaptain", + "avatar_url": "https://avatars3.githubusercontent.com/u/29956894?v=4", + "profile": "https://github.com/toonarmycaptain", + "contributions": [ + "code", + "doc", + "design", + "ideas", + "maintenance" + ] + }, + { + "login": "schedutron", + "name": "Saurabh Chaturvedi", + "avatar_url": "https://avatars1.githubusercontent.com/u/22810216?v=4", + "profile": "https://stackoverflow.com/story/samchats", + "contributions": [ + "code" + ] + }, + { + "login": "AbdullahElagha", + "name": "AbdullahElagha", + "avatar_url": "https://avatars3.githubusercontent.com/u/20312723?v=4", + "profile": "https://github.com/AbdullahElagha", + "contributions": [ + "doc" + ] + } + ], + "contributorsPerLine": 7, + "projectName": "dionysus", + "projectOwner": "toonarmycaptain", + "repoType": "github", + "repoHost": "https://github.com", + "skipCi": true +} diff --git a/README.md b/README.md index 020e697e..8ed2e983 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ ## dionysus - Avatar chart generator -[![All Contributors](https://img.shields.io/badge/all_contributors-1-orange.svg?style=flat-square)](#contributors-) +[![All Contributors](https://img.shields.io/badge/all_contributors-7-orange.svg?style=flat-square)](#contributors-) [![Build Status](https://travis-ci.org/toonarmycaptain/dionysus.svg?branch=master)](https://travis-ci.org/toonarmycaptain/dionysus) [![CircleCI](https://circleci.com/gh/toonarmycaptain/dionysus/tree/master.svg?style=svg)](https://circleci.com/gh/toonarmycaptain/dionysus/tree/master) @@ -49,12 +49,20 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d - + + + + + + + +

Piotr Czajka
💻 ⚠️
toonarmycaptain
💻 📖 🎨 🤔 🚧
Przemysław Pietras
🚇 📖
Piotr Czajka
💻 ⚠️
Saurabh Chaturvedi
💻
M. Alex Boyd
💻 ⚠️
José María Domínguez
🚇
HCamberos
💻
AbdullahElagha
📖
+ -This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome! \ No newline at end of file +This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome! From b1263cf36f371e82a4dcd1d9d30985ff0bbb28a1 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 3 Feb 2020 11:12:47 +0000 Subject: [PATCH 04/44] Bump matplotlib from 3.1.2 to 3.1.3 Bumps [matplotlib](https://github.com/matplotlib/matplotlib) from 3.1.2 to 3.1.3. - [Release notes](https://github.com/matplotlib/matplotlib/releases) - [Commits](https://github.com/matplotlib/matplotlib/compare/v3.1.2...v3.1.3) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 65c901ba..31b231b2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ -matplotlib==3.1.2 +matplotlib==3.1.3 Pillow==7.0.0 setuptools==45.1.0 \ No newline at end of file From d44cd34c08181e405378fa60c221b67a5752dec3 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 10 Feb 2020 11:13:13 +0000 Subject: [PATCH 05/44] Bump setuptools from 45.1.0 to 45.2.0 Bumps [setuptools](https://github.com/pypa/setuptools) from 45.1.0 to 45.2.0. - [Release notes](https://github.com/pypa/setuptools/releases) - [Changelog](https://github.com/pypa/setuptools/blob/master/CHANGES.rst) - [Commits](https://github.com/pypa/setuptools/compare/v45.1.0...v45.2.0) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 31b231b2..98ee3093 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ matplotlib==3.1.3 Pillow==7.0.0 -setuptools==45.1.0 \ No newline at end of file +setuptools==45.2.0 \ No newline at end of file From 93b1ad8f474e7fae711b13cc8083aaf4dd2d60bb Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 17 Feb 2020 11:13:52 +0000 Subject: [PATCH 06/44] Bump coveralls from 1.10.0 to 1.11.1 Bumps [coveralls](https://github.com/coveralls-clients/coveralls-python) from 1.10.0 to 1.11.1. - [Release notes](https://github.com/coveralls-clients/coveralls-python/releases) - [Changelog](https://github.com/coveralls-clients/coveralls-python/blob/master/CHANGELOG.md) - [Commits](https://github.com/coveralls-clients/coveralls-python/compare/1.10.0...1.11.1) Signed-off-by: dependabot-preview[bot] --- requirements_dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_dev.txt b/requirements_dev.txt index 43872886..6d3f21f9 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -4,7 +4,7 @@ pytest==5.3.5 codacy-coverage==1.3.11 codecov==2.0.15 coverage==5.0.3 -coveralls==1.10.0 +coveralls==1.11.1 mypy==0.761 pytest-cov==2.8.1 pytest-mypy==0.4.2 From 749d20bb8698293e85695228c8c6b4fa7ed4b0ea Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 24 Feb 2020 11:12:56 +0000 Subject: [PATCH 07/44] Bump pytest-mypy from 0.4.2 to 0.5.0 Bumps [pytest-mypy](https://github.com/dbader/pytest-mypy) from 0.4.2 to 0.5.0. - [Release notes](https://github.com/dbader/pytest-mypy/releases) - [Changelog](https://github.com/dbader/pytest-mypy/blob/master/changelog.md) - [Commits](https://github.com/dbader/pytest-mypy/compare/v0.4.2...v0.5.0) Signed-off-by: dependabot-preview[bot] --- requirements_dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_dev.txt b/requirements_dev.txt index 6d3f21f9..c28a5dc1 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -7,4 +7,4 @@ coverage==5.0.3 coveralls==1.11.1 mypy==0.761 pytest-cov==2.8.1 -pytest-mypy==0.4.2 +pytest-mypy==0.5.0 From aaa13c8ed3396d274fda5192dad10e3f2fd1cf31 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Sun, 1 Mar 2020 12:42:03 -0600 Subject: [PATCH 08/44] Update codecov from 2.0.15 to 2.0.16 --- requirements_dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_dev.txt b/requirements_dev.txt index c28a5dc1..10fbbfd3 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -2,7 +2,7 @@ pytest==5.3.5 # Running tests on Travis codacy-coverage==1.3.11 -codecov==2.0.15 +codecov==2.0.16 coverage==5.0.3 coveralls==1.11.1 mypy==0.761 From 5a7ade9e2b159f0b96417815578a750fce8e4475 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 9 Mar 2020 11:12:23 +0000 Subject: [PATCH 09/44] Bump setuptools from 45.2.0 to 46.0.0 Bumps [setuptools](https://github.com/pypa/setuptools) from 45.2.0 to 46.0.0. - [Release notes](https://github.com/pypa/setuptools/releases) - [Changelog](https://github.com/pypa/setuptools/blob/master/CHANGES.rst) - [Commits](https://github.com/pypa/setuptools/compare/v45.2.0...v46.0.0) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 98ee3093..7d05a560 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ matplotlib==3.1.3 Pillow==7.0.0 -setuptools==45.2.0 \ No newline at end of file +setuptools==46.0.0 \ No newline at end of file From 10be8a2e310533dc4b9668740fa736dcbff19a35 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 16 Mar 2020 11:14:23 +0000 Subject: [PATCH 10/44] Bump pytest-mypy from 0.5.0 to 0.6.0 Bumps [pytest-mypy](https://github.com/dbader/pytest-mypy) from 0.5.0 to 0.6.0. - [Release notes](https://github.com/dbader/pytest-mypy/releases) - [Changelog](https://github.com/dbader/pytest-mypy/blob/master/changelog.md) - [Commits](https://github.com/dbader/pytest-mypy/compare/v0.5.0...v0.6.0) Signed-off-by: dependabot-preview[bot] --- requirements_dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_dev.txt b/requirements_dev.txt index 10fbbfd3..8373925a 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -7,4 +7,4 @@ coverage==5.0.3 coveralls==1.11.1 mypy==0.761 pytest-cov==2.8.1 -pytest-mypy==0.5.0 +pytest-mypy==0.6.0 From 4e993fad418612766e567094c0fe929faaa94ccd Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Thu, 19 Mar 2020 11:12:46 +0000 Subject: [PATCH 11/44] Bump codecov from 2.0.16 to 2.0.22 Bumps [codecov](https://github.com/codecov/codecov-python) from 2.0.16 to 2.0.22. - [Release notes](https://github.com/codecov/codecov-python/releases) - [Changelog](https://github.com/codecov/codecov-python/blob/v2.0.22/CHANGELOG.md) - [Commits](https://github.com/codecov/codecov-python/compare/v2.0.16...v2.0.22) Signed-off-by: dependabot-preview[bot] --- requirements_dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_dev.txt b/requirements_dev.txt index 10fbbfd3..471fd1eb 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -2,7 +2,7 @@ pytest==5.3.5 # Running tests on Travis codacy-coverage==1.3.11 -codecov==2.0.16 +codecov==2.0.22 coverage==5.0.3 coveralls==1.11.1 mypy==0.761 From e25e18200a98db705e094626468b83ed3bf3e3e2 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Fri, 20 Mar 2020 02:12:04 +0000 Subject: [PATCH 12/44] Bump mypy from 0.761 to 0.770 Bumps [mypy](https://github.com/python/mypy) from 0.761 to 0.770. - [Release notes](https://github.com/python/mypy/releases) - [Commits](https://github.com/python/mypy/compare/v0.761...v0.770) Signed-off-by: dependabot-preview[bot] --- requirements_dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_dev.txt b/requirements_dev.txt index c0789f1e..82527719 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -5,6 +5,6 @@ codacy-coverage==1.3.11 codecov==2.0.22 coverage==5.0.3 coveralls==1.11.1 -mypy==0.761 +mypy==0.770 pytest-cov==2.8.1 pytest-mypy==0.6.0 From d514e4be326b96cc59bca5fe2c4077f671523a14 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Fri, 20 Mar 2020 02:15:13 +0000 Subject: [PATCH 13/44] Bump matplotlib from 3.1.3 to 3.2.1 Bumps [matplotlib](https://github.com/matplotlib/matplotlib) from 3.1.3 to 3.2.1. - [Release notes](https://github.com/matplotlib/matplotlib/releases) - [Commits](https://github.com/matplotlib/matplotlib/compare/v3.1.3...v3.2.1) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 7d05a560..b35c1e72 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ -matplotlib==3.1.3 +matplotlib==3.2.1 Pillow==7.0.0 setuptools==46.0.0 \ No newline at end of file From e00657a527d1baee6330ab8d88c5e04348235d0f Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Fri, 20 Mar 2020 03:47:52 +0000 Subject: [PATCH 14/44] Bump coverage from 5.0.3 to 5.0.4 Bumps [coverage](https://github.com/nedbat/coveragepy) from 5.0.3 to 5.0.4. - [Release notes](https://github.com/nedbat/coveragepy/releases) - [Changelog](https://github.com/nedbat/coveragepy/blob/master/CHANGES.rst) - [Commits](https://github.com/nedbat/coveragepy/compare/coverage-5.0.3...coverage-5.0.4) Signed-off-by: dependabot-preview[bot] --- requirements_dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_dev.txt b/requirements_dev.txt index 82527719..77b0a31a 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -3,7 +3,7 @@ pytest==5.3.5 # Running tests on Travis codacy-coverage==1.3.11 codecov==2.0.22 -coverage==5.0.3 +coverage==5.0.4 coveralls==1.11.1 mypy==0.770 pytest-cov==2.8.1 From fdbcdae36413ffeb122ef4843ebd2537a2a7c774 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Fri, 20 Mar 2020 04:09:12 +0000 Subject: [PATCH 15/44] Bump pytest from 5.3.5 to 5.4.1 Bumps [pytest](https://github.com/pytest-dev/pytest) from 5.3.5 to 5.4.1. - [Release notes](https://github.com/pytest-dev/pytest/releases) - [Changelog](https://github.com/pytest-dev/pytest/blob/master/CHANGELOG.rst) - [Commits](https://github.com/pytest-dev/pytest/compare/5.3.5...5.4.1) Signed-off-by: dependabot-preview[bot] --- requirements_dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_dev.txt b/requirements_dev.txt index 77b0a31a..498b2718 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -1,4 +1,4 @@ -pytest==5.3.5 +pytest==5.4.1 # Running tests on Travis codacy-coverage==1.3.11 From bf916bc8fa51b8cf57f4c50cb7321d93ada714a2 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 23 Mar 2020 11:12:49 +0000 Subject: [PATCH 16/44] Bump setuptools from 46.0.0 to 46.1.1 Bumps [setuptools](https://github.com/pypa/setuptools) from 46.0.0 to 46.1.1. - [Release notes](https://github.com/pypa/setuptools/releases) - [Changelog](https://github.com/pypa/setuptools/blob/master/CHANGES.rst) - [Commits](https://github.com/pypa/setuptools/compare/v46.0.0...v46.1.1) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index b35c1e72..f084ee7e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ matplotlib==3.2.1 Pillow==7.0.0 -setuptools==46.0.0 \ No newline at end of file +setuptools==46.1.1 \ No newline at end of file From ca81c89d7fd8c75cd8bea10b885bf72bc18e2836 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Tue, 4 Feb 2020 15:14:37 -0600 Subject: [PATCH 17/44] Change write_classlist_to_file to not return a value. Change to allow uniformity in database write functions, as SQL databases won't have a Path to return. Return Path value is only currently used in data_version_conversion.py to provide visual feedback of file created. This is trivially removed, and the path constructed manually for display. Signed-off-by: toonarmycaptain --- data_version_conversion.py | 8 ++++++-- dionysus_app/class_functions.py | 3 +-- test_suite/test_class_functions.py | 4 ++-- test_suite/test_data_version_conversion.py | 4 +++- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/data_version_conversion.py b/data_version_conversion.py index 722d8739..05dda5ae 100644 --- a/data_version_conversion.py +++ b/data_version_conversion.py @@ -7,7 +7,7 @@ from dionysus_app.class_ import Class from dionysus_app.class_functions import write_classlist_to_file -from dionysus_app.data_folder import DataFolder +from dionysus_app.data_folder import DataFolder, CLASSLIST_DATA_FILE_TYPE from dionysus_app.file_functions import load_from_json_file from dionysus_app.student import Student from dionysus_app.UI_menus.UI_functions import select_file_dialogue @@ -112,7 +112,11 @@ def transform_old_cld_file(filepath: Path): class_name = filepath.stem new_class = transform_data(class_name, old_class_data) - new_class_data_path_name = write_classlist_to_file(new_class) + write_classlist_to_file(new_class) + + new_filename = class_name + CLASSLIST_DATA_FILE_TYPE + new_class_data_path_name = CLASSLIST_DATA_PATH.joinpath(class_name, new_filename) + print(f'Transformed {new_class.name} data file ' f'to new data format in {new_class_data_path_name}') diff --git a/dionysus_app/class_functions.py b/dionysus_app/class_functions.py index 6fee7f62..ead1ff3d 100644 --- a/dionysus_app/class_functions.py +++ b/dionysus_app/class_functions.py @@ -185,7 +185,7 @@ def avatar_file_exists(avatar_file: Union[str, Path]) -> bool: return Path(avatar_file).exists() -def write_classlist_to_file(current_class: Class) -> Path: +def write_classlist_to_file(current_class: Class) -> None: """ Write classlist data to disk as JSON dict, according to Class object's Class.json_dict and Class.to_json_str methods. @@ -208,7 +208,6 @@ def write_classlist_to_file(current_class: Class) -> Path: with open(classlist_data_path, 'w') as classlist_file: classlist_file.write(json_class_data) - return classlist_data_path def create_chart_with_new_class(classlist_name): diff --git a/test_suite/test_class_functions.py b/test_suite/test_class_functions.py index 7e3d64b6..880330cd 100644 --- a/test_suite/test_class_functions.py +++ b/test_suite/test_class_functions.py @@ -376,7 +376,7 @@ def test_write_classlist_to_file(self): # Assert precondition: assert not os.path.exists(self.test_class_data_file_path) - assert write_classlist_to_file(self.test_class_object) == self.test_class_data_file_path + assert write_classlist_to_file(self.test_class_object) is None assert isinstance(self.test_class_data_file_path, Path) @@ -417,7 +417,7 @@ def test_write_classlist_to_file_mocking_open(self): mocked_open = mock_open() with patch('dionysus_app.class_functions.open', mocked_open), \ patch('dionysus_app.class_functions.Path.mkdir', autospec=True) as mocked_mkdir: - assert write_classlist_to_file(self.test_class_object) == self.test_class_data_file_path + assert write_classlist_to_file(self.test_class_object) is None assert isinstance(self.test_class_data_file_path, Path) diff --git a/test_suite/test_data_version_conversion.py b/test_suite/test_data_version_conversion.py index f43c2eb0..ac571728 100644 --- a/test_suite/test_data_version_conversion.py +++ b/test_suite/test_data_version_conversion.py @@ -203,7 +203,8 @@ def mocked_data_is_new_format(json_dict): def test_transform_old_cld_file_genuine_old_file(self, monkeypatch, test_class_name_only, capsys): - test_filepath = Path(f'hi//I//do//{test_class_name_only.path_safe_name}.yay') + mocked_CLASSLIST_DATA_PATH = Path(f'hi//I//do//') + test_filepath = Path(mocked_CLASSLIST_DATA_PATH).joinpath(test_class_name_only.path_safe_name, f'{test_class_name_only.path_safe_name}.yay') new_data_filepath = test_filepath.parent.joinpath( test_class_name_only.path_safe_name + CLASSLIST_DATA_FILE_TYPE) @@ -241,6 +242,7 @@ def mocked_write_classlist_to_file(test_class): monkeypatch.setattr(data_version_conversion, 'data_is_new_format', mocked_data_is_new_format) monkeypatch.setattr(data_version_conversion, 'transform_data', mocked_transform_data) monkeypatch.setattr(data_version_conversion, 'write_classlist_to_file', mocked_write_classlist_to_file) + monkeypatch.setattr(data_version_conversion, 'CLASSLIST_DATA_PATH', mocked_CLASSLIST_DATA_PATH) assert transform_old_cld_file(test_filepath) is None From ccb1f597b7dd631d2996796360121aa4831ba98c Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Tue, 4 Feb 2020 20:56:14 -0600 Subject: [PATCH 18/44] PEP8 improvements, whitespace etc. Signed-off-by: toonarmycaptain --- test_suite/test_data_version_conversion.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test_suite/test_data_version_conversion.py b/test_suite/test_data_version_conversion.py index ac571728..230b632a 100644 --- a/test_suite/test_data_version_conversion.py +++ b/test_suite/test_data_version_conversion.py @@ -7,6 +7,7 @@ import pytest import data_version_conversion + from data_version_conversion import (CLASSLIST_DATA_PATH, data_is_new_format, file_from_gui_dialogue, @@ -114,7 +115,7 @@ def test_transform_all_old_data_files(self, monkeypatch): test_file_paths = [Path('first/path'), Path('second/path')] def mocked_path_glob(self, cld_pattern): - if cld_pattern != '**/*.cld': + if cld_pattern != '**/*.cld': raise ValueError return (test_path for test_path in test_file_paths) @@ -124,7 +125,6 @@ def mocked_transform_old_cld_file(arg): if arg not in test_file_paths: raise ValueError - monkeypatch.setattr(data_version_conversion.Path, 'glob', mocked_path_glob) monkeypatch.setattr(data_version_conversion, 'transform_old_cld_file', mocked_transform_old_cld_file) @@ -204,7 +204,8 @@ def mocked_data_is_new_format(json_dict): def test_transform_old_cld_file_genuine_old_file(self, monkeypatch, test_class_name_only, capsys): mocked_CLASSLIST_DATA_PATH = Path(f'hi//I//do//') - test_filepath = Path(mocked_CLASSLIST_DATA_PATH).joinpath(test_class_name_only.path_safe_name, f'{test_class_name_only.path_safe_name}.yay') + test_filepath = Path(mocked_CLASSLIST_DATA_PATH).joinpath(test_class_name_only.path_safe_name, + f'{test_class_name_only.path_safe_name}.yay') new_data_filepath = test_filepath.parent.joinpath( test_class_name_only.path_safe_name + CLASSLIST_DATA_FILE_TYPE) From bda8cfa3b37a89fe611a4987053631dcf6b148f5 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Sun, 16 Feb 2020 00:48:14 -0600 Subject: [PATCH 19/44] Separate UI/logic/persistence concerns in `create_classlist`. create_classlist_data takes a Class object. Add/improve docstrings. TestCreateClasslistData converted to Pytest test. Signed-off-by: toonarmycaptain --- CHANGELOG.md | 6 +++ dionysus_app/class_functions.py | 39 ++++++++++++---- test_suite/test_class_functions.py | 73 +++++++++++++++++------------- 3 files changed, 76 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c19f0a70..0ac32b08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] +### Changed +- Separate UI/logic/persistence concerns in `create_classlist`. +- `create_classlist_data` takes a `Class` object. +- More tests converted to Pytest style tests. + ## [0.5.0-alpha] - 2020-01-23 ### Added - Improved test coverage, type hinting across application. diff --git a/dionysus_app/class_functions.py b/dionysus_app/class_functions.py index ead1ff3d..b5bdb9da 100644 --- a/dionysus_app/class_functions.py +++ b/dionysus_app/class_functions.py @@ -33,11 +33,21 @@ DEFAULT_AVATAR_PATH = DataFolder.generate_rel_path(DataFolder.DEFAULT_AVATAR.value) -def create_classlist(): +def create_classlist() -> None: + """ + Create a new class, then give option to create a chart with new class. + + Calls UI elements to collect new class' data, then writes data to persistence. + + :return: None + """ classlist_name = take_classlist_name_input() # TODO: Option to cancel creation at class name entry stage - setup_class(classlist_name) - create_classlist_data(classlist_name) + new_class = compose_classlist_dialogue(classlist_name) + + create_classlist_data(new_class) # Future: Call to Database.create_class(new_class) + time.sleep(2) # Pause for user to look over feedback. + create_chart_with_new_class(classlist_name) @@ -48,9 +58,9 @@ def setup_class(classlist_name: str) -> None: Register class in class_registry index :param classlist_name: - :return: + :return: None """ - setup_class_data_storage(classlist_name) + setup_class_data_storage(classlist_name) # Database.create_class register_class(classlist_name) @@ -88,17 +98,24 @@ def setup_class_data_storage(classlist_name: str) -> None: user_chart_save_folder.mkdir(exist_ok=True, parents=True) -def create_classlist_data(class_name: str) -> None: - new_class = compose_classlist_dialogue(class_name) +def create_classlist_data(new_class: Class) -> None: + """ + Creates class data in persistence. + Calls setup_class to create any needed files, then writes data to file. - class_data_feedback(new_class) + :param new_class: Class object + :return: None + """ + setup_class(new_class.name) # -> functionality moves to (ie function called by) JSONDatabase.create_class write_classlist_to_file(new_class) - time.sleep(2) # Pause for user to look over feedback. def compose_classlist_dialogue(class_name: str) -> Class: """ - Create class object. + Call UI elements to collect new class data. + Provide feedback to user reflecting new class composition. + + Creates class object. If no students added to class, check if intended, making subsequent call to take_class_data_input UI if unintended blank class returned. @@ -117,6 +134,8 @@ def compose_classlist_dialogue(class_name: str) -> Class: continue # Line skipped from coverage due to peephole optimiser. break # class_data not empty + class_data_feedback(new_class) + return new_class diff --git a/test_suite/test_class_functions.py b/test_suite/test_class_functions.py index 880330cd..39b3bae6 100644 --- a/test_suite/test_class_functions.py +++ b/test_suite/test_class_functions.py @@ -42,26 +42,30 @@ class TestCreateClasslist: - def test_create_classlist(self, monkeypatch, test_classname='the_flying_circus'): - + def test_create_classlist(self, monkeypatch, test_full_class): def mocked_take_classlist_name_input(): - return test_classname + return test_full_class.name - def mocked_setup_class(test_class_name): - if test_class_name != test_classname: + def mocked_compose_classlist_dialogue(test_class_name): + if test_class_name != test_full_class.name: raise ValueError + return test_full_class - def mocked_create_classlist_data(test_class_name): - if test_class_name != test_classname: + def mocked_create_classlist_data(test_class): + if test_class != test_full_class: raise ValueError + def mocked_time_sleep(seconds): + pass + def mocked_create_chart_with_new_class(test_class_name): - if test_class_name != test_classname: + if test_class_name != test_full_class.name: raise ValueError monkeypatch.setattr(class_functions, 'take_classlist_name_input', mocked_take_classlist_name_input) - monkeypatch.setattr(class_functions, 'setup_class', mocked_setup_class) + monkeypatch.setattr(class_functions, 'compose_classlist_dialogue', mocked_compose_classlist_dialogue) monkeypatch.setattr(class_functions, 'create_classlist_data', mocked_create_classlist_data) + monkeypatch.setattr(class_functions.time, 'sleep', mocked_time_sleep) monkeypatch.setattr(class_functions, 'create_chart_with_new_class', mocked_create_chart_with_new_class) assert create_classlist() is None @@ -114,52 +118,57 @@ def test_setup_class_data_storage_raising_error(monkeypatch): setup_class_data_storage('some_class') -class TestCreateClasslistData(TestCase): - def setUp(self): - self.test_class_json_dict = test_full_class_data_set['json_dict_rep'] - self.test_classname = self.test_class_json_dict['name'] - self.test_class_object = Class.from_dict(self.test_class_json_dict) +class TestCreateClasslistData: + def test_create_classlist_data(self, monkeypatch, test_full_class): + def mocked_setup_class(test_class_name): + if test_class_name != test_full_class.name: + raise ValueError + + def mocked_write_classlist_to_file(test_class): + if test_class != test_full_class: + raise ValueError - @patch('dionysus_app.class_functions.compose_classlist_dialogue') - @patch('dionysus_app.class_functions.class_data_feedback') - @patch('dionysus_app.class_functions.write_classlist_to_file') - @patch('dionysus_app.class_functions.time.sleep') - def test_create_classlist_data(self, - mock_time_sleep, - mock_write_classlist_to_file, - mock_class_data_feedback, - mock_compose_classlist_dialogue): - mock_compose_classlist_dialogue.return_value = self.test_class_object - assert create_classlist_data(self.test_classname) is None + monkeypatch.setattr(class_functions, 'setup_class', mocked_setup_class) + monkeypatch.setattr(class_functions, 'write_classlist_to_file', mocked_write_classlist_to_file) - mock_compose_classlist_dialogue.assert_called_once_with(self.test_classname) - mock_class_data_feedback.assert_called_once_with(self.test_class_object) - mock_write_classlist_to_file.assert_called_once_with(self.test_class_object) - mock_time_sleep.assert_called_once_with(2) + assert create_classlist_data(test_full_class) is None class TestComposeClasslistDialogue: def test_compose_classlist_dialogue_full_class(self, monkeypatch, test_full_class): - def mocked_take_class_data_input(class_name): + def mocked_take_class_data_input(test_class_name): + if test_class_name != test_full_class.name: + raise ValueError return test_full_class def mocked_blank_class_dialogue(): raise ValueError # Should not be called. + def mocked_class_data_feedback(test_class): + if test_class != test_full_class: + raise ValueError + monkeypatch.setattr(class_functions, 'take_class_data_input', mocked_take_class_data_input) monkeypatch.setattr(class_functions, 'blank_class_dialogue', mocked_blank_class_dialogue) - + monkeypatch.setattr(class_functions, 'class_data_feedback', mocked_class_data_feedback) assert compose_classlist_dialogue(test_full_class.name).json_dict() == test_full_class.json_dict() def test_compose_classlist_dialogue_create_empty_class(self, monkeypatch, test_class_name_only): - def mocked_take_class_data_input(class_name): + def mocked_take_class_data_input(test_class_name): + if test_class_name != test_class_name_only.name: + raise ValueError return test_class_name_only def mocked_blank_class_dialogue(): return True + def mocked_class_data_feedback(test_class): + if test_class != test_class_name_only: + raise ValueError + monkeypatch.setattr(class_functions, 'take_class_data_input', mocked_take_class_data_input) monkeypatch.setattr(class_functions, 'blank_class_dialogue', mocked_blank_class_dialogue) + monkeypatch.setattr(class_functions, 'class_data_feedback', mocked_class_data_feedback) assert compose_classlist_dialogue(test_class_name_only.name).json_dict() == test_class_name_only.json_dict() From 981ef8c607a11db31e2ab7f60d23fbd26abff2cb Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Mon, 9 Mar 2020 01:32:21 -0500 Subject: [PATCH 20/44] Fix visual indentation in list comp. Signed-off-by: toonarmycaptain --- dionysus_app/UI_menus/UI_functions.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dionysus_app/UI_menus/UI_functions.py b/dionysus_app/UI_menus/UI_functions.py index f60d7afb..f553bd89 100644 --- a/dionysus_app/UI_menus/UI_functions.py +++ b/dionysus_app/UI_menus/UI_functions.py @@ -70,9 +70,8 @@ def scrub_candidate_filename(dirty_string: str): :return: str """ allowed_special_characters = [' ', '_', '-', ] - cleaned_string = "".join([c - if c.isalnum() - or c in allowed_special_characters + cleaned_string = "".join([c if c.isalnum() + or c in allowed_special_characters else '_' for c in dirty_string ]).rstrip() From b0c89ee9b21eb8396828ae92b2d42b3e56dc721b Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Sat, 14 Mar 2020 00:40:47 -0500 Subject: [PATCH 21/44] Change UI_functions to add types, accept/return Paths, change to pytest. Also improve docstrings, documenting that if None/unresolvable path is passed to `filedialog` dialogues, recent versions of Windows default to a recently used directory, or a Windows default, and on older Windows/ other OS defaults to current working directory. Signed-off-by: toonarmycaptain --- dionysus_app/UI_menus/UI_functions.py | 65 +- .../UI_menus_tests/test_UI_functions.py | 767 +++++++----------- 2 files changed, 337 insertions(+), 495 deletions(-) diff --git a/dionysus_app/UI_menus/UI_functions.py b/dionysus_app/UI_menus/UI_functions.py index f553bd89..a4cd6ab6 100644 --- a/dionysus_app/UI_menus/UI_functions.py +++ b/dionysus_app/UI_menus/UI_functions.py @@ -6,9 +6,10 @@ from pathlib import Path from tkinter import filedialog +from typing import Optional, Union, List, Tuple -def clear_screen(num_lines=50): +def clear_screen(num_lines: int = 50) -> None: clear_seq = '\n' * num_lines print(clear_seq) @@ -17,7 +18,7 @@ def clear_screen(num_lines=50): # User input and string processing functions: -def input_is_essentially_blank(subject_string: str): +def input_is_essentially_blank(subject_string: str) -> bool: """ Return True if string is empty or primarily composed of spaces, underscores, special characters (eg brackets, punctuation). @@ -35,7 +36,7 @@ def input_is_essentially_blank(subject_string: str): return False -def clean_for_filename(some_string: str): +def clean_for_filename(some_string: str) -> str: """ Cleans a string for use as a filename. eg student or classlist name @@ -52,7 +53,7 @@ def clean_for_filename(some_string: str): return cleaned_filename -def scrub_candidate_filename(dirty_string: str): +def scrub_candidate_filename(dirty_string: str) -> str: """ Cleans string of non-alpha-numeric characters, but leaves spaces, dashes, and underscores, stripping trailing spaces. @@ -79,12 +80,12 @@ def scrub_candidate_filename(dirty_string: str): return cleaned_string -def save_as_dialogue(title_str=None, - default_file_extension=None, - filetypes=None, - suggested_filename=None, - start_dir=None - ): +def save_as_dialogue(title_str: str = None, + default_file_extension: str = None, + filetypes: List[Tuple[str, str]] = None, + suggested_filename: str = None, + start_dir: Union[Path, str] = '..' + ) -> Optional[Path]: """ Prompts user to select a directory and filename to save a file to. Calls tkinter filedialog.asksaveasfilename with title (if provided), and @@ -113,7 +114,10 @@ def save_as_dialogue(title_str=None, Returns None instead of empty string if no file is selected. - + Default starting directory is directory above application directory. + If start_dir is unresolvable, or '' or None, the dialog will default to + starting at the last directory selected. + See https://www.tcl.tk/man/tcl8.6/TkCmd/chooseDirectory.htm NB When default_file_extension is given, but not in filetypes, if the first filetype in filetypes is NOT ("all files", "*.*"), that @@ -124,9 +128,9 @@ def save_as_dialogue(title_str=None, :param title_str: str :param default_file_extension: list :param suggested_filename: str - :param filetypes: list - :param start_dir: str - :return: str + :param filetypes: List[Tuple[str, str]] + :param start_dir: Path or str + :return: Path """ root = tk.Tk() root.withdraw() @@ -149,13 +153,13 @@ def save_as_dialogue(title_str=None, if filepath_str == '': return None - return filepath_str + return Path(filepath_str) -def select_file_dialogue(title_str=None, - filetypes=None, - start_dir=None, - ): +def select_file_dialogue(title_str: str = None, + filetypes: List[Tuple[str, str]] = None, + start_dir: Union[Path, str] = '..', + ) -> Optional[Path]: """ Prompts user to select a file. Calls tkinter filedialog.askopenfilename with title (if provided), and filetype argument (if provided) eg '*.png'. @@ -163,15 +167,21 @@ def select_file_dialogue(title_str=None, filetypes is a list of tuples with 2 values, a label and a pattern eg for png and all files: [('.png', '*.png'), ("all files", "*.*")] + Default starting directory is directory above application directory. + If start_dir is unresolvable, or '' or None, the dialog will default to + starting at the last directory selected on recent versions of Windows, + current working directory on old Windows/other OS. + See https://www.tcl.tk/man/tcl8.6/TkCmd/chooseDirectory.htm + Returns None instead of empty string if no file is selected. NB If no title is passed to filedialog.askopenfilename, the window title will be "Open". :param title_str: str - :param filetypes: list + :param filetypes: List[Tuple[str, str]] :param start_dir: str - :return: str + :return: Path or None """ root = tk.Tk() root.withdraw() @@ -189,19 +199,22 @@ def select_file_dialogue(title_str=None, return Path(filepath_str) -def select_folder_dialogue(title_str=None, start_dir='..'): +def select_folder_dialogue(title_str: str = None, start_dir: Union[Path, str] = '..'): """ Prompts user to select a file. Calls tkinter filedialog.askopenfilename with title (if provided), and filetype argument (if provided) eg '*.png'. - filetypes is a list of tuples with 2 values, a label and a pattern - eg for png and all files: [('.png', '*.png'), ("all files", "*.*")] + Default starting directory is directory above application directory. + If start_dir is unresolvable, or '' or None, the dialog will default to + starting at the last directory selected on recent versions of Windows, + current working directory on old Windows/other OS. + See https://www.tcl.tk/man/tcl8.6/TkCmd/chooseDirectory.htm Returns None instead of empty string if no file is selected. :param title_str: str - :param start_dir: str - Path for dialogue to start in. - :return: str + :param start_dir: Path or str - Path for dialogue to start in. + :return: Path """ root = tk.Tk() root.withdraw() diff --git a/test_suite/UI_menus_tests/test_UI_functions.py b/test_suite/UI_menus_tests/test_UI_functions.py index c4800d1b..2f47cd66 100644 --- a/test_suite/UI_menus_tests/test_UI_functions.py +++ b/test_suite/UI_menus_tests/test_UI_functions.py @@ -3,9 +3,9 @@ import pytest from pathlib import Path -from unittest import TestCase, mock from unittest.mock import patch +from dionysus_app.UI_menus import UI_functions from dionysus_app.UI_menus.UI_functions import (clean_for_filename, clear_screen, input_is_essentially_blank, @@ -16,138 +16,109 @@ ) -class TestClearScreen(TestCase): - def setUp(self): - self.test_arguments = 1, 5, 99 - self.expected_print_calls = ['\n', - 5 * '\n', - 99 * '\n', - ] - self.default_argument = 50 - self.default_argument_print_call = 50 * '\n' - - def test_clear_screen(self): +class TestClearScree: + @pytest.mark.parametrize( + 'test_input, expected_print_output', + [(50, 50 * '\n'), + (1, '\n'), + (5, 5 * '\n'), + (99, 99 * '\n'), + ]) + def test_clear_screen(self, test_input, expected_print_output): with patch('dionysus_app.UI_menus.UI_functions.print') as mocked_print: - for argument in self.test_arguments: - clear_screen(argument) - - print_calls = [mock.call(printed_str) for printed_str in self.expected_print_calls] + clear_screen(test_input) + + mocked_print.assert_called_once_with(expected_print_output) + + +class TestInputIsEssentiallyBlank: + @pytest.mark.parametrize( + 'test_input, expected_return_value', + [('', True), # empty string, ie no input. + # Spaces + (' ', True), # single space + (' ', True), # 2_spaces + (' ', True), # 3_spaces + (' ', True), # 5_spaces + # Underscores + ('_', True), # single underscore + ('__', True), # 2 underscores + ('___', True), # 3 underscores + ('_____', True), # 5 underscores + + ('''~`!@#$%^&*()-_+{}[]|\\:;"',.<>?/''', True), # only_special_characters + (' test', False), # singe leading space + (' _ _ _', True), # spaces underscores combo + (' test', False), # leading spaces + ('test ', False), # singe trailing space + ('test ', False), # trailing spaces + (' test ', False), # leading and trailing space + ('test', False), # no spaces + ('not the Spanish inquisition', False), # sentence + (' not the spanish inquisition ', False), # sentence leading and trailing spaces + # test_combination underscore spaces + (" because nobody_expects_the _spanish_ inquisition the 2nd time", False), + # combination underscore spaces special characters + (" because nobody_expects_the !@#$%ing _spanish_ inquisition the 2nd ?~)*% time", False), + ]) + def test_input_is_essentially_blank(self, test_input, expected_return_value): + assert input_is_essentially_blank(test_input) == expected_return_value - mocked_print.assert_has_calls(print_calls) # Test calls. - assert mocked_print.call_args_list == print_calls # Test calls made in order. - def test_clear_screen_default_argument(self): - with patch('dionysus_app.UI_menus.UI_functions.print') as mocked_print: - clear_screen() - - print_call = self.default_argument_print_call - mocked_print.assert_called_once_with(print_call) - - -class TestInputIsEssentiallyBlank(TestCase): - def setUp(self): - """"Test cases: (input_value, expected_return_value)""" - self.test_cases = { - 'test_empty_string': ('', True), # ie no input - # Spaces - 'test_single_space': (' ', True), - 'test_2_spaces': (' ', True), - 'test_3_spaces': (' ', True), - 'test_5_spaces': (' ', True), - # Underscores - 'test_single_underscore': ('_', True), - 'test_2_underscores': ('__', True), - 'test_3_underscores': ('___', True), - 'test_5_underscores': ('_____', True), - - 'test_only_special_characters': ('''~`!@#$%^&*()-_+{}[]|\\:;"',.<>?/''', True), - 'test_singe_leading_space': (' test', False), - 'test_spaces_underscores_combo': (' _ _ _', True), - 'test_leading_spaces': (' test', False), - 'test_singe_trailing_space': ('test ', False), - 'test_trailing_spaces': ('test ', False), - 'test_leading_and_trailing_space': (' test ', False), - 'test_no_spaces': ('test', False), - 'test_sentence': ('not the Spanish inquisition', False), - 'test_sentence_leading_and_trailing_spaces': (' not the spanish inquisition ', False), - 'test_combination_underscore_spaces': ( - " because nobody_expects_the _spanish_ inquisition the 2nd time", False), - 'test_combination_underscore_spaces_special_characters': ( - " because nobody_expects_the !@#$%ing _spanish_ inquisition the 2nd ?~)*% time", False), - } - - def test_input_is_essentially_blank(self): - for test_case in self.test_cases: - with self.subTest(i=self.test_cases[test_case]): - test_input = self.test_cases[test_case][0] - expected_output = self.test_cases[test_case][1] - - assert input_is_essentially_blank(test_input) == expected_output - - -class TestCleanForFilename(TestCase): - def setUp(self): - """Test cases: (input_value, expected_return_value)""" - self.test_cases = { - 'test_empty_string': ('', ''), # ie no input - # Spaces - 'test_single_space': (' ', ''), # .rstrip() will remove trailing space. - 'test_2_spaces': (' ', ''), - 'test_3_spaces': (' ', ''), - 'test_5_spaces': (' ', ''), - # Underscores - 'test_single_underscore': ('_', '_'), - 'test_double_underscore': ('__', '__'), - 'test_triple_underscore': ('___', '___'), - 'test_spaces_underscores_combo': (' _ _ _', '______'), - 'test_spaces_underscores_combo_trailing_space': (' _ _ _ ', '______'), - 'test_leading_space': (' test', '_test'), - 'test_leading_spaces': (' test', '___test'), - 'test_singe_trailing_space': ('test ', 'test'), - 'test_trailing_spaces': ('test ', 'test'), - 'test_leading_and_trailing_space': (' test ', '_test'), - 'test_no_spaces': ('test', 'test'), - 'test_sentence': ('not the Spanish inquisition', 'not_the_Spanish_inquisition'), - 'test_sentence_leading_and_trailing_spaces': (' not the spanish inquisition ', - '_not_the_spanish_inquisition'), - 'test_combination_underscore_spaces': ( - ' because nobody_expects_the _spanish_ inquisition the 2nd time', - '_because_nobody_expects_the__spanish__inquisition_the_2nd_time'), - 'test_combination_underscore_spaces_special_characters': ( - ' because nobody_expects_the !@#$%ing _spanish_ inquisition the 2nd ?~)*% time', - '_because_nobody_expects_the______ing__spanish__inquisition_the_2nd_______time'), - # Explicitly test OS/Filesystem format prohibited characters: - 'Test prohibited chars FAT32': (r'" * / : < > ? \ | + , . ; = [ ] ! @ ; ', - '_____________________________________'), - 'Test prohibited characters Windows': (r'\/:*?"<>|.', '__________'), - 'Test prohibited *nix/OSX chars': (r'*?!/.', '_____'), - } - - @patch('dionysus_app.UI_menus.UI_functions.scrub_candidate_filename') - def test_clean_for_filename_mocking_call(self, mocked_scrub_candidate_filename): +@pytest.mark.parametrize( + 'test_input, expected_return_value', + [('', ''), # empty string, ie no input + # Spaces + (' ', ''), # .rstrip() will remove trailing space, single_space + (' ', ''), # 2 spaces + (' ', ''), # 3 spaces + (' ', ''), # 5 spaces + # Underscores + ('_', '_'), # single underscore + ('__', '__'), # double underscore + ('___', '___'), # triple underscore + (' _ _ _', '______'), # spaces underscores combo + (' _ _ _ ', '______'), # spaces underscores combo trailing space + (' test', '_test'), # leading space + (' test', '___test'), # leading spaces + ('test ', 'test'), # singe trailing space + ('test ', 'test'), # trailing spaces + (' test ', '_test'), # leading and trailing space + ('test', 'test'), # no spaces + ('not the Spanish inquisition', 'not_the_Spanish_inquisition'), # sentence + (' not the spanish inquisition ', # sentence leading and trailing spaces + '_not_the_spanish_inquisition'), + # Combination underscore spaces. + (' because nobody_expects_the _spanish_ inquisition the 2nd time', + '_because_nobody_expects_the__spanish__inquisition_the_2nd_time'), + # Combination underscore spaces special characters. + (' because nobody_expects_the !@#$%ing _spanish_ inquisition the 2nd ?~)*% time', + '_because_nobody_expects_the______ing__spanish__inquisition_the_2nd_______time'), + # Explicitly test OS/Filesystem format prohibited characters: + # Prohibited chars FAT32: + (r'" * / : < > ? \ | + , . ; = [ ] ! @ ; ', '_____________________________________'), + # Prohibited characters Windows: + (r'\/:*?"<>|.', '__________'), + # Prohibited characters *nix/OSX chars: + (r'*?!/.', '_____'), + ]) +class TestCleanForFilename: + def test_clean_for_filename_mocking_call(self, monkeypatch, + test_input, expected_return_value): """ Mock call to scrub_candidate_filename. - Essentially an input .replace(' ', '_') operation. + Essentially an input .replace(' ', '_') operation, unless behaviour changes. """ - for test_case in self.test_cases: - with self.subTest(i=self.test_cases[test_case]): - test_input = self.test_cases[test_case][0] - expected_output = test_input.replace(' ', '_') + def mocked_scrub_candidate_filename(test_string): + assert test_string == test_input + return test_input - # Have scrub_candidate_filename return input_string. - mocked_scrub_candidate_filename.return_value = test_input - assert clean_for_filename(test_input) == expected_output + monkeypatch.setattr(UI_functions, 'scrub_candidate_filename', mocked_scrub_candidate_filename) - mocked_scrub_candidate_filename.assert_called_once_with(test_input) - mocked_scrub_candidate_filename.reset_mock(return_value=True, side_effect=True) + assert clean_for_filename(test_input) == test_input.replace(' ', '_') - def test_clean_for_filename_unmocked(self): - for test_case in self.test_cases: - with self.subTest(i=self.test_cases[test_case]): - test_input = self.test_cases[test_case][0] - expected_output = self.test_cases[test_case][1] - - assert clean_for_filename(test_input) == expected_output + def test_clean_for_filename_unmocked(self, test_input, expected_return_value): + assert clean_for_filename(test_input) == expected_return_value @pytest.mark.parametrize( @@ -188,353 +159,211 @@ def test_scrub_candidate_filename(test_input, expected_output): assert scrub_candidate_filename(test_input) == expected_output +# Patching Tk doesn't affect test but not doing so slows down test 10x. +# Pass to test func as mock_tk @patch('dionysus_app.UI_menus.UI_functions.tk.Tk') -class TestSaveAsDialogue(TestCase): - def setUp(self): - self.default_filetypes = [("all files", "*.*")] - self.test_returned_filepath_str = "my save file" - - def test_save_as_dialogue_no_arguments(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - save_as_filedialog.return_value = self.test_returned_filepath_str - assert save_as_filedialog.return_value == save_as_dialogue() - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=None, - defaultextension=None, - filetypes=self.default_filetypes, - initialdir=None, - initialfile=None - ) - - def test_save_as_dialogue_all_None_arguments(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - save_as_filedialog.return_value = self.test_returned_filepath_str - - assert save_as_dialogue(title_str=None, - default_file_extension=None, - filetypes=None, - suggested_filename=None, - start_dir=None - ) == save_as_filedialog.return_value - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=None, - defaultextension=None, - filetypes=self.default_filetypes, - initialdir=None, - initialfile=None, - ) - - def test_save_as_dialogue_with_title_str(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - title_string = "Save your super_file_as:" - - save_as_filedialog.return_value = self.test_returned_filepath_str - - assert save_as_dialogue(title_str=title_string) == save_as_filedialog.return_value - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=title_string, - defaultextension=None, - filetypes=self.default_filetypes, - initialdir=None, - initialfile=None, - ) - - def test_save_as_dialogue_with_default_extension_some_ext(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - test_default_extension = '.some_ext' - - save_as_filedialog.return_value = self.test_returned_filepath_str - - assert save_as_dialogue(default_file_extension=test_default_extension) == save_as_filedialog.return_value - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=None, - defaultextension='.some_ext', - filetypes=self.default_filetypes, - initialdir=None, - initialfile=None - ) - - def test_save_as_dialogue_with_filetypes_all_files(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - test_filetypes = [("all files", "*.*")] - - save_as_filedialog.return_value = self.test_returned_filepath_str - - assert save_as_dialogue(filetypes=test_filetypes) == save_as_filedialog.return_value - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=None, - defaultextension=None, - filetypes=self.default_filetypes, - initialdir=None, - initialfile=None - ) - - def test_save_as_dialogue_with_filetypes_some_ext(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - test_filetypes = [('some ext', '*.some_ext')] - test_default_extension = '.some_ext' - - save_as_filedialog.return_value = self.test_returned_filepath_str - - assert save_as_dialogue(default_file_extension=test_default_extension, - filetypes=test_filetypes - ) == self.test_returned_filepath_str - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=None, - defaultextension=test_default_extension, - filetypes=test_filetypes, - initialdir=None, - initialfile=None, - ) - - def test_save_as_dialogue_with_filetypes_all_files_plus_some_ext(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - test_filetypes = [('all files', '*.*'), ('some ext', '*.some_ext')] - - save_as_filedialog.return_value = self.test_returned_filepath_str - - assert save_as_dialogue(filetypes=test_filetypes) == self.test_returned_filepath_str - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=None, - defaultextension=None, - filetypes=test_filetypes, - initialdir=None, - initialfile=None, - ) - - def test_save_as_dialogue_with_filetypes_some_ext_plus_all_files(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - test_filetypes = [('some ext', '*.some_ext'), ('all files', '*.*')] - test_default_extension = '.some_ext' - - save_as_filedialog.return_value = self.test_returned_filepath_str - - assert save_as_dialogue(default_file_extension=test_default_extension, - filetypes=test_filetypes, - ) == save_as_filedialog.return_value - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=None, - defaultextension=test_default_extension, - filetypes=test_filetypes, - initialdir=None, - initialfile=None, - ) - - def test_save_as_dialogue_with_filetypes_some_ext_no_default(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - test_filetypes = [('some ext', '*.some_ext'), ('all files', '*.*')] - test_default_extension = '.some_ext' - - save_as_filedialog.return_value = self.test_returned_filepath_str - - assert save_as_dialogue(filetypes=test_filetypes, - ) == save_as_filedialog.return_value - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=None, - defaultextension=test_default_extension, - filetypes=test_filetypes, - initialdir=None, - initialfile=None, - ) - - def test_save_as_dialogue_with_suggested_filename(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - test_suggested_filename = 'you should call your save file THIS' - - save_as_filedialog.return_value = self.test_returned_filepath_str - - assert save_as_dialogue(suggested_filename=test_suggested_filename) == self.test_returned_filepath_str - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=None, - defaultextension=None, - filetypes=self.default_filetypes, - initialdir=None, - initialfile=test_suggested_filename, - ) - - def test_save_as_dialogue_with_start_dir_str(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - test_start_dir = 'where to start?' - - save_as_filedialog.return_value = self.test_returned_filepath_str - - assert save_as_dialogue(start_dir=test_start_dir) == self.test_returned_filepath_str - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=None, - defaultextension=None, - filetypes=self.default_filetypes, - initialdir=test_start_dir, - initialfile=None, - ) - - def test_save_as_dialogue_with_start_dir_Path(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - test_start_dir = 'where to start?' - test_start_dir_path = Path(test_start_dir) - - save_as_filedialog.return_value = self.test_returned_filepath_str - - assert save_as_dialogue(start_dir=test_start_dir_path) == self.test_returned_filepath_str - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=None, - defaultextension=None, - filetypes=self.default_filetypes, - initialdir=test_start_dir_path, - initialfile=None, - ) - - def test_save_as_dialogue_no_input_returns_None(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.asksaveasfilename') as save_as_filedialog: - save_as_filedialog.return_value = '' - - assert save_as_dialogue() is None - - mock_tkinter.assert_called() - save_as_filedialog.assert_called_once_with(title=None, - defaultextension=None, - filetypes=self.default_filetypes, - initialdir=None, - initialfile=None, - ) - - +class TestSaveAsDialogue: + @pytest.mark.parametrize( + 'save_as_dialog_args, expected_filedialog_args, test_filename, returned_filepath', + [({}, {}, "my save file", Path("my save file")), # No args. + pytest.param({}, {'filetypes': 'should be the default', 'initialdir': 'starting directory: ".." '}, + "my_save_file", Path("my_save_file"), + marks=pytest.mark.xfail(reason="Test diagnostic.")), + ({'title_str': None, # None args passed to save_as_dialog + 'default_file_extension': None, + 'filetypes': None, + 'suggested_filename': None, + 'start_dir': None}, + {'initialdir': None}, # None dir passed on, default_filetypes subst by func. + "my_save_file", Path("my_save_file")), + # Title string. + ({'title_str': "Save your super_file_as:"}, + {'title': "Save your super_file_as:"}, + "my_save_file", Path("my_save_file")), + # Default extension. + ({'default_file_extension': '.some_ext'}, + {'defaultextension': '.some_ext'}, + "my_save_file", Path("my_save_file.some_ext")), + # Passing all files works same as no arg, since all files is default. + ({'filetypes': [("all files", "*.*")]}, + {}, + "my_save_file", Path("my_save_file")), + # Pass a filetype with no default, and it becomes default extension. + ({'filetypes': [('some ext', '*.some_ext')]}, + {'filetypes': [('some ext', '*.some_ext')], + 'defaultextension': '.some_ext'}, + "my_save_file", Path("my_save_file.some_ext")), + # Test default extension and filetype + ({'filetypes': [('some ext', '*.some_ext')], + 'default_file_extension': '.some_ext'}, + {'filetypes': [('some ext', '*.some_ext')], + 'defaultextension': '.some_ext'}, + "my_save_file", Path("my_save_file.some_ext")), + # Test default extension different from first filetype + ({'filetypes': [('a ext', '*.a_ext'), ('b ext', '*.b_ext')], + 'default_file_extension': '.b_ext'}, + {'filetypes': [('a ext', '*.a_ext'), ('b ext', '*.b_ext')], + 'defaultextension': '.b_ext'}, + "my_save_file", Path("my_save_file.b_ext")), + # Test default extension is default/first of multiple filetypes. + ({'filetypes': [('a ext', '*.a_ext'), ('b ext', '*.b_ext')]}, + {'filetypes': [('a ext', '*.a_ext'), ('b ext', '*.b_ext')], + 'defaultextension': '.a_ext'}, + "my_save_file", Path("my_save_file.a_ext")), + # Test no default extension when multiple filetypes passed with all files first. + ({'filetypes': [("all files", "*.*"), ('a ext', '*.a_ext'), ('b ext', '*.b_ext')]}, + {'filetypes': [("all files", "*.*"), ('a ext', '*.a_ext'), ('b ext', '*.b_ext')], + 'defaultextension': None}, + "my_save_file", Path("my_save_file")), + # Test default extension when multiple filetypes passed with all files not first. + ({'filetypes': [('a ext', '*.a_ext'), ("all files", "*.*"), ('b ext', '*.b_ext')]}, + {'filetypes': [('a ext', '*.a_ext'), ("all files", "*.*"), ('b ext', '*.b_ext')], + 'defaultextension': '.a_ext'}, + "my_save_file", Path("my_save_file.a_ext")), + # Test passing default filename, with user using default. + ({'suggested_filename': 'save as this'}, + {'initialfile': 'save as this'}, + "save as this", Path("save as this")), + # Test passing default filename, but returning another. + ({'suggested_filename': 'save as this'}, + {'initialfile': 'save as this'}, + "user filename", Path("user filename")), + # Test passing str starting directory. + ({'start_dir': 'some start dir'}, + {'initialdir': 'some start dir'}, + "my_save_file", Path("my_save_file")), + # Test passing Path starting directory. + ({'start_dir': Path('some start dir')}, + {'initialdir': Path('some start dir')}, + "my_save_file", Path("my_save_file")), + # Test no user input returns None. + ({}, {}, '', None), + ]) + def test_save_as_dialogue(self, mock_tk, monkeypatch, + save_as_dialog_args, + expected_filedialog_args, + test_filename, + returned_filepath): + default_filetypes = [("all files", "*.*")] + default_start_dir = '..' + + def mocked_filedialog_asksaveasfilename(title, + defaultextension, + filetypes, + initialfile, + initialdir, + ): + """ + Test arguments passed to filedialog are as expected, subbing with defaults save_as_dialogue + will supply if no value is passed (equivalent to expecting the default). + """ + assert title == expected_filedialog_args.get('title', None) + assert defaultextension == expected_filedialog_args.get('defaultextension', None) + assert filetypes == expected_filedialog_args.get('filetypes', default_filetypes) + assert initialfile == expected_filedialog_args.get('initialfile', None) + assert initialdir == expected_filedialog_args.get('initialdir', default_start_dir) + + return test_filename + (expected_filedialog_args.get('defaultextension', '') or '') + + monkeypatch.setattr(UI_functions.filedialog, 'asksaveasfilename', mocked_filedialog_asksaveasfilename) + + assert returned_filepath == save_as_dialogue(**save_as_dialog_args) + + +# Patching Tk doesn't affect test but not doing so slows down test 10x. +# Pass to test func as mock_tk @patch('dionysus_app.UI_menus.UI_functions.tk.Tk') -class TestSelectFileDialogue(TestCase): - def setUp(self): - self.test_default_filetypes = [('all files', '*.*')] - - self.test_title_str = 'First you must answer three questions' - self.test_filetypes = [('some ext', '*.some_ext'), ('all files', '*.*')] - self.test_start_dir = 'What\\is\\your\\quest' - - self.test_returned_filepath_path = Path('strange women lying in ponds distributing swords') - - def test_select_file_dialogue_called_without_arguments(self, mock_tkinter): - # Tests filedialog called with default_filetypes. - with patch('dionysus_app.UI_menus.UI_functions.filedialog.askopenfilename') as select_filedialog: - select_filedialog.return_value = self.test_returned_filepath_path - - assert select_file_dialogue() == self.test_returned_filepath_path - - assert isinstance(self.test_returned_filepath_path, Path) - - mock_tkinter.assert_called() - select_filedialog.assert_called_once_with(title=None, - filetype=self.test_default_filetypes, - initialdir=None) - - def test_select_file_dialogue_all_None_arguments(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.askopenfilename') as select_filedialog: - select_filedialog.return_value = self.test_returned_filepath_path - - assert select_file_dialogue(title_str=None, - filetypes=None, - start_dir=None, - ) == self.test_returned_filepath_path - - assert isinstance(self.test_returned_filepath_path, Path) - - mock_tkinter.assert_called() - select_filedialog.assert_called_once_with(title=None, - filetype=self.test_default_filetypes, - initialdir=None) - - def test_select_file_dialogue_called_with_all_arguments(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.askopenfilename') as select_filedialog: - select_filedialog.return_value = self.test_returned_filepath_path - - assert select_file_dialogue(title_str=self.test_title_str, - filetypes=self.test_filetypes, - start_dir=self.test_start_dir, - ) == self.test_returned_filepath_path - - assert isinstance(self.test_returned_filepath_path, Path) - - mock_tkinter.assert_called() - select_filedialog.assert_called_once_with(title=self.test_title_str, - filetype=self.test_filetypes, - initialdir=self.test_start_dir) - - def test_select_file_dialogue_no_input_returns_None(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.askopenfilename') as select_filedialog: - select_filedialog.return_value = '' - - assert select_file_dialogue() is None - - mock_tkinter.assert_called() - select_filedialog.assert_called_once_with(title=None, - filetype=self.test_default_filetypes, - initialdir=None) +class TestSelectFileDialogue: + @pytest.mark.parametrize( + 'select_file_dialogue_args, expected_askopenfilename_args, test_filename, returned_filepath', + [({}, {}, "my save file", Path("my save file")), # No args. + pytest.param({}, {'filetype': 'should be the default', 'initialdir': 'starting directory: ".." '}, + "my_save_file", Path("some_other_file"), + marks=pytest.mark.xfail(reason="Test diagnostic.")), + ({'title_str': None, # None args passed to select_file_dialog + 'filetypes': None, + 'start_dir': None}, + {'initialdir': None}, # None dir passed on, default_filetypes subst by func. + "my_save_file", Path("my_save_file")), + # All args passed to select_file_dialog + ({'title_str': 'Some title string', + 'filetypes': [('some ext', '*.some_ext'), ('all files', '*.*')], + 'start_dir': Path(r'start/here')}, + {'title': 'Some title string', + 'filetype': [('some ext', '*.some_ext'), ('all files', '*.*')], + 'initialdir': Path(r'start/here')}, + "my save file", Path("my save file")), + # No user input/cancel (ie filename = '') returns None + ({}, {}, '', None) + ]) + def test_select_file_dialogue(self, mock_tk, monkeypatch, + select_file_dialogue_args, + expected_askopenfilename_args, + test_filename, + returned_filepath): + default_filetypes = [("all files", "*.*")] + default_start_dir = '..' + + def mocked_filedialog_askopenfilename(title, + filetype, + initialdir, + ): + """ + Test arguments passed to filedialog are as expected, subbing with defaults save_as_dialogue + will supply if no value is passed (equivalent to expecting the default). + """ + assert title == expected_askopenfilename_args.get('title', None) + assert filetype == expected_askopenfilename_args.get('filetype', default_filetypes) + assert initialdir == expected_askopenfilename_args.get('initialdir', default_start_dir) + + return test_filename + (expected_askopenfilename_args.get('defaultextension', '') or '') + + monkeypatch.setattr(UI_functions.filedialog, 'askopenfilename', mocked_filedialog_askopenfilename) + + assert returned_filepath == select_file_dialogue(**select_file_dialogue_args) @patch('dionysus_app.UI_menus.UI_functions.tk.Tk') -class TestSelectFolderDialogue(TestCase): - def setUp(self): - self.test_default_start_dir = '..' - - self.test_title_str = 'First you must answer three questions' - self.test_start_dir = 'What\\is\\your\\quest' - - self.test_returned_folderpath_path = Path('strange women lying in ponds distributing swords') - - def test_select_folder_dialogue_called_without_arguments(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.askdirectory') as select_directorydialog: - select_directorydialog.return_value = self.test_returned_folderpath_path - - assert select_folder_dialogue() == self.test_returned_folderpath_path - - assert isinstance(self.test_returned_folderpath_path, Path) - - mock_tkinter.assert_called() - select_directorydialog.assert_called_once_with(title=None, - initialdir=self.test_default_start_dir) - - def test_select_folder_dialogue_all_None_arguments(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.askdirectory') as select_directorydialog: - select_directorydialog.return_value = self.test_returned_folderpath_path - - assert select_folder_dialogue(title_str=None, - start_dir=None, - ) == self.test_returned_folderpath_path - - assert isinstance(self.test_returned_folderpath_path, Path) - - mock_tkinter.assert_called() - select_directorydialog.assert_called_once_with(title=None, - initialdir=None) - - def test_select_folder_dialogue_called_with_all_arguments(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.askdirectory') as select_directorydialog: - select_directorydialog.return_value = self.test_returned_folderpath_path - - assert select_folder_dialogue(title_str=self.test_title_str, - start_dir=self.test_start_dir, - ) == self.test_returned_folderpath_path - - assert isinstance(self.test_returned_folderpath_path, Path) - - mock_tkinter.assert_called() - select_directorydialog.assert_called_once_with(title=self.test_title_str, - initialdir=self.test_start_dir) - - def test_select_folder_dialogue_no_input_returns_None(self, mock_tkinter): - with patch('dionysus_app.UI_menus.UI_functions.filedialog.askdirectory') as select_directorydialog: - select_directorydialog.return_value = '' - - assert select_folder_dialogue() is None - - mock_tkinter.assert_called() - select_directorydialog.assert_called_once_with(title=None, - initialdir=self.test_default_start_dir) +class TestSelectFolderDialogue: + @pytest.mark.parametrize( + 'select_folder_dialogue_args, expected_askdirectory_args, test_path, returned_path', + [({}, {}, "my save dir", Path("my save dir")), # No args. + pytest.param({}, {'initialdir': 'starting directory: ".." '}, + "my_save_dir", Path("some_other_dir"), + marks=pytest.mark.xfail(reason="Test diagnostic.")), + # None args passed to select_file_dialog + ({'title_str': None, + 'start_dir': None}, + {'initialdir': None}, # None dir passed on. + "my_save_dir", Path("my_save_dir")), + # # All args passed to select_file_dialog + ({'title_str': 'Some title string', + 'start_dir': Path(r'start/here')}, + {'title': 'Some title string', + 'initialdir': Path(r'start/here')}, + "my save dir", Path("my save dir")), + # No user input/cancel (ie filename = '') returns None + ({}, {}, '', None) + ]) + def test_select_folder_dialogue(self, mock_tk, monkeypatch, + select_folder_dialogue_args, + expected_askdirectory_args, + test_path, + returned_path): + default_start_dir = '..' + + def mocked_filedialog_askdirectory(title, + initialdir, + ): + """ + Test arguments passed to filedialog are as expected, subbing with defaults save_as_dialogue + will supply if no value is passed (equivalent to expecting the default). + """ + assert title == expected_askdirectory_args.get('title', None) + assert initialdir == expected_askdirectory_args.get('initialdir', default_start_dir) + + return test_path + + monkeypatch.setattr(UI_functions.filedialog, 'askdirectory', mocked_filedialog_askdirectory) + + assert returned_path == select_folder_dialogue(**select_folder_dialogue_args) From 4f2913512efd69ec83727de357872f613a87a487 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Sat, 14 Mar 2020 00:42:59 -0500 Subject: [PATCH 22/44] Convert file_functions tests to Pytest style tests. Signed-off-by: toonarmycaptain --- test_suite/test_file_functions.py | 251 ++++++++++++++---------------- 1 file changed, 120 insertions(+), 131 deletions(-) diff --git a/test_suite/test_file_functions.py b/test_suite/test_file_functions.py index 5cdb6b07..a11d1d28 100644 --- a/test_suite/test_file_functions.py +++ b/test_suite/test_file_functions.py @@ -1,10 +1,9 @@ -import os -import shutil - from pathlib import Path -from unittest import TestCase from unittest.mock import patch, mock_open +import pytest + +from dionysus_app import file_functions from dionysus_app.file_functions import (convert_to_json, load_from_json, load_from_json_file, @@ -13,169 +12,159 @@ from test_suite.testing_class_data import test_full_class_data_set as test_json_class_data -class TestConvertToJson(TestCase): - def setUp(self): - self.data_to_convert = {1: 'a', 'b': 2, 3: 'c', 'd': 4} - self.json_converted_data = '{\n "1": "a",\n "b": 2,\n "3": "c",\n "d": 4\n}' - - def test_convert_to_json(self): - assert convert_to_json(self.data_to_convert) == self.json_converted_data - - -class TestLoadFromJson(TestCase): - def setUp(self): - self.json_data_to_convert = '{\n "1": "a",\n "b": 2,\n "3": "c",\n "d": 4\n}' - self.converted_json_data = {"1": 'a', 'b': 2, "3": 'c', 'd': 4} - self.test_json_class_data = test_json_class_data - - def test_load_from_json(self): - assert load_from_json(self.json_data_to_convert) == self.converted_json_data - - def test_load_from_json_test_class_data(self): - assert load_from_json(self.test_json_class_data['json_str_rep']) == self.test_json_class_data['json_dict_rep'] +@pytest.fixture +def test_file(): + """ + Return function that creates an arbitrary at a text file, test_file.txt in + a given directory, return the path of the file. + """ + def create_file(test_path): + test_filepath = Path(test_path, 'test_file.txt') + with open(test_filepath, 'w') as test_file: + test_file.write('This is a placeholder file.') + if not test_filepath.exists(): + raise FileNotFoundError('This file should exist now.') + return test_filepath -class TestLoadFromJsonFile(TestCase): - def setUp(self): - self.test_file_json_data_to_convert = '{\n "1": "a",\n "b": 2,\n "3": "c",\n "d": 4\n}' - self.mock_file_path = Path('test_file_path') - self.converted_json_data = {"1": 'a', 'b': 2, "3": 'c', 'd': 4} + return create_file - def test_load_from_json_file(self): - with patch('dionysus_app.file_functions.open', mock_open(read_data=self.test_file_json_data_to_convert)): - assert load_from_json_file(self.mock_file_path) == self.converted_json_data +def test_test_file_fixture(tmpdir, test_file): + tf = test_file(tmpdir) -class TestCopyFile(TestCase): - def setUp(self): - self.original_filename = 'just_a_naughty_boy.png' - self.new_folder_name = 'not_the_messiah' + assert isinstance(tf, Path) + assert tf.exists() + assert open(tf).read() == 'This is a placeholder file.' - self.original_path = self.original_filename - self.destination_path = os.path.join(self.new_folder_name, self.original_filename) - with open(self.original_filename, 'w+') as good_file: - pass +class TestConvertToJson: + @pytest.mark.parametrize( + 'loaded_object, json_str', + [({"1": 'a', 'b': 2, "3": 'c', 'd': 4}, '{\n "1": "a",\n "b": 2,\n "3": "c",\n "d": 4\n}'), + (test_json_class_data['json_dict_rep'], test_json_class_data['json_str_rep']), + ]) + def test_convert_to_json(self, loaded_object, json_str): + assert convert_to_json(loaded_object) == json_str - os.mkdir(self.new_folder_name) - def test_copy_file(self): - assert os.path.exists(self.original_filename) - assert not os.path.exists(self.destination_path) - copy_file(self.original_path, self.destination_path) - assert os.path.exists(self.destination_path) +class TestLoadFromJson: + @pytest.mark.parametrize( + 'json_str, loaded_object', + [('{\n "1": "a",\n "b": 2,\n "3": "c",\n "d": 4\n}', + {"1": 'a', 'b': 2, "3": 'c', 'd': 4}), + (test_json_class_data['json_str_rep'], + test_json_class_data['json_dict_rep']), + ]) + def test_load_from_json(self, json_str, loaded_object): + assert load_from_json(json_str) == loaded_object - def tearDown(self): - os.remove(self.original_filename) # remove new file - assert not os.path.exists(self.original_filename) - shutil.rmtree(self.new_folder_name) # remove new dir - assert not os.path.exists(self.new_folder_name) +class TestLoadFromJsonFile: + @pytest.mark.parametrize( + 'json_file_data, loaded_object', + [('{\n "1": "a",\n "b": 2,\n "3": "c",\n "d": 4\n}', + {"1": 'a', 'b': 2, "3": 'c', 'd': 4}), + (test_json_class_data['json_str_rep'], + test_json_class_data['json_dict_rep']), + ]) + def test_load_from_json_file(self, json_file_data, loaded_object): + mock_file_path = Path('test_file_path') + with patch('dionysus_app.file_functions.open', mock_open(read_data=json_file_data)): + assert load_from_json_file(mock_file_path) == loaded_object -class TestCopyFileMockingCopyfile(TestCase): - def setUp(self): - self.original_filename = 'just_a_naughty_boy.png' - self.new_folder_name = 'not_the_messiah' - self.original_path = self.original_filename - self.destination_path = os.path.join(self.new_folder_name, self.original_filename) +class TestCopyFile: + def test_copy_file(self, tmpdir, test_file, monkeypatch): + def mocked_copyfile(origin, destination): + # Test copyfile called with expected arguments. + assert (origin, destination) == (str(original_filepath), str(destination_filepath)) - @patch('dionysus_app.file_functions.copyfile') - @patch('dionysus_app.file_functions.Path.exists') - def test_copy_file_mocking_copyfile(self, mock_path_exists, mock_copyfile): - mock_path_exists.return_value = True # Assume the path exists. + original_filepath = test_file(tmpdir) - copy_file(self.original_path, self.destination_path) + destination_filepath = Path('some destination') + monkeypatch.setattr(file_functions, 'copyfile', mocked_copyfile) - mock_copyfile.assert_called_once_with(self.original_path, str(self.destination_path)) + assert not Path.exists(destination_filepath) + copy_file(original_filepath, destination_filepath) + def test_copy_file_copies_file(self, tmpdir, test_file): + original_filepath = test_file(tmpdir) + original_filename = original_filepath.name -class TestMoveFile(TestCase): - def setUp(self): - self.original_filename = 'just_a_naughty_boy.png' - self.new_folder_name = 'not_the_messiah' + destination_dir = Path(tmpdir, 'new_directory') + Path.mkdir(destination_dir) + destination_filepath = Path(destination_dir, original_filename) - self.original_path = self.original_filename - self.destination_path = os.path.join(self.new_folder_name, self.original_filename) + assert not Path.exists(destination_filepath) + copy_file(original_filepath, destination_filepath) - with open(self.original_filename, 'w+') as test_file: - pass + # Assert file in destination and still in origin. + assert Path.exists(destination_filepath) and Path.exists(original_filepath) - os.mkdir(self.new_folder_name) + def test_copy_file_non_existent_original(self, monkeypatch): + def mocked_copyfile(origin, destination): + # Should not be called. + raise NotImplementedError - def test_move_file(self): - assert os.path.exists(self.original_filename) - assert not os.path.exists(self.destination_path) - move_file(self.original_path, self.destination_path) - assert os.path.exists(self.destination_path) - assert not os.path.exists(self.original_path) + monkeypatch.setattr(file_functions, 'copyfile', mocked_copyfile) - def tearDown(self): - shutil.rmtree(self.new_folder_name) - assert not os.path.exists(self.new_folder_name) - assert not os.path.exists(self.original_filename) + copy_file('Non-existent origin', 'No destination') -class TestMoveFileMockingMove(TestCase): - def setUp(self): - self.original_filename = 'just_a_naughty_boy.png' - self.new_folder_name = 'not_the_messiah' +class TestMoveFile: + def test_move_file(self, tmpdir, test_file, monkeypatch): + def mocked_move(origin, destination): + # Test copyfile called with expected arguments. + assert (origin, destination) == (str(original_filepath), str(destination_filepath)) - self.original_path = self.original_filename - self.destination_path = os.path.join(self.new_folder_name, self.original_filename) + original_filepath = test_file(tmpdir) + destination_filepath = Path('some destination') - @patch('dionysus_app.file_functions.move') - @patch('dionysus_app.file_functions.Path.exists') - def test_move_file_mocking_move(self, mock_path_exists, mock_move): - mock_path_exists.return_value = True # Assume the path exists. + monkeypatch.setattr(file_functions, 'move', mocked_move) - move_file(self.original_path, self.destination_path) - mock_move.assert_called_once_with(self.original_path, self.destination_path) + assert not Path.exists(destination_filepath) + move_file(original_filepath, destination_filepath) - @patch('dionysus_app.file_functions.move') - @patch('dionysus_app.file_functions.Path.exists') - def test_move_file_mocking_move_non_existent_origin(self, mock_path_exists, mock_move): - mock_path_exists.return_value = False # Assume the path exists. + def test_move_file_moves_file(self, tmpdir, test_file): + original_filepath = test_file(tmpdir) + original_filename = original_filepath.name - move_file(self.original_path, self.destination_path) - mock_move.assert_not_called() + destination_dir = Path(tmpdir, 'new_directory') + Path.mkdir(destination_dir) + destination_filepath = Path(destination_dir, original_filename) + assert not Path.exists(destination_filepath) + move_file(original_filepath, destination_filepath) -class TestMoveDirectoryWithFileInIt(TestCase): - def setUp(self): - # Origin file, folder. - self.original_filename = 'just_a_naughty_boy.png' - self.original_folder_name = 'not_the_messiah' + # Assert file in destination and not in origin. + assert Path.exists(destination_filepath) and not Path.exists(original_filepath) - self.original_file_path = os.path.join(self.original_folder_name, self.original_filename) + def test_move_file_non_existent_original(self, monkeypatch): + def mocked_move(origin, destination): + # Should not be called. + raise NotImplementedError - # Destination folder, filepath. - self.destination_folder_name = 'a_boy_named_brian' - self.destination_path = os.path.join(self.destination_folder_name, self.original_file_path) + monkeypatch.setattr(file_functions, 'move', mocked_move) - # Make origin folder, file, destination folder. - os.mkdir(self.original_folder_name) - with open(self.original_file_path, 'w+') as test_file: - pass - os.mkdir(self.destination_folder_name) + move_file('Non-existent origin', 'No destination') - # test setUp - # confirm original files and folders exist - assert os.path.exists(self.original_folder_name) - assert os.path.exists(self.original_file_path) - assert os.path.exists(self.destination_folder_name) + def test_move_file_directory_containing_file(self, tmpdir, test_file): + # Create directory with file in it. + original_dirname = 'original_dir' + original_dir = Path(tmpdir, original_dirname) + Path.mkdir(original_dir) + original_filepath = test_file(original_dir) + original_filename = original_filepath.name - # Confirm target not in destination folder: - # Folder in new location. - assert not os.path.exists(os.path.join(self.destination_folder_name, self.original_folder_name)) - assert not os.path.exists(self.destination_path) # File in destination_folder/original_folder/file. + destination_dir = Path(tmpdir, 'new_directory') + Path.mkdir(destination_dir) + destination_filepath = Path(destination_dir, original_dirname, original_filename) - def test_move_file_directory_containing_file(self): - move_file(self.original_folder_name, self.destination_folder_name) - assert os.path.exists(self.destination_path) - assert not os.path.exists(self.original_file_path) - assert not os.path.exists(self.original_folder_name) + assert not Path.exists(destination_filepath) + # Move original folder with file in it. + move_file(original_dir, destination_dir) - def tearDown(self): - shutil.rmtree(self.destination_folder_name) - assert not os.path.exists(self.destination_folder_name) + # Assert original directory no longer exists, file in destination and not in origin. + assert not Path.exists(original_dir) + assert Path.exists(destination_filepath) and not Path.exists(original_filepath) From a45dbdac4013692f62927ee550d4915f1226cc29 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Sun, 22 Mar 2020 23:51:07 -0500 Subject: [PATCH 23/44] Use Path in initialise_app, Change test_initialise_app to pytest tests. Signed-off-by: toonarmycaptain --- dionysus_app/initialise_app.py | 8 ++- test_suite/test_initialise_app.py | 89 +++++++++++++++---------------- 2 files changed, 48 insertions(+), 49 deletions(-) diff --git a/dionysus_app/initialise_app.py b/dionysus_app/initialise_app.py index 286ec8cf..bcde984d 100644 --- a/dionysus_app/initialise_app.py +++ b/dionysus_app/initialise_app.py @@ -1,4 +1,4 @@ -import os +from pathlib import Path from dionysus_app.data_folder import DataFolder from dionysus_app.settings_functions import APP_SETTINGS_FILE, app_start_set_default_chart_save_location @@ -11,7 +11,7 @@ def app_config() -> None: Set user default chart save folder. """ - if not os.path.exists(APP_SETTINGS_FILE): + if not Path.exists(APP_SETTINGS_FILE): app_start_set_default_chart_save_location() @@ -38,3 +38,7 @@ def app_init() -> None: if __name__ == '__main__': pass + + +### Add temp to DataFolder? check empty on startup if [list comp files in temp]: delete them. +# Create/clear and create temp folder on startup. Clear/clear and recreate on exit. \ No newline at end of file diff --git a/test_suite/test_initialise_app.py b/test_suite/test_initialise_app.py index 4148ac73..8a4a26b7 100644 --- a/test_suite/test_initialise_app.py +++ b/test_suite/test_initialise_app.py @@ -1,70 +1,65 @@ -import os -from unittest import TestCase -from unittest.mock import patch +import pytest -from dionysus_app.data_folder import DataFolder +from pathlib import Path + +from dionysus_app import data_folder, initialise_app from dionysus_app.initialise_app import (app_config, data_folder_check, app_init, ) -class TestAppConfig(TestCase): - def setUp(self): - pass +class TestAppConfig: + def test_app_config_no_settings_file(self, monkeypatch): + def mocked_path_exists(path): + return False - @patch('dionysus_app.initialise_app.app_start_set_default_chart_save_location') - @patch('dionysus_app.initialise_app.os.path.exists') - def test_app_config_no_settings_file(self, - mocked_os_path_exists, - mocked_app_start_set_default_chart_save_location, - ): - mocked_os_path_exists.return_value = True + def mocked_app_start_set_default_chart_save_location(): + pass + monkeypatch.setattr(initialise_app.Path, 'exists', mocked_path_exists) + monkeypatch.setattr(initialise_app, 'app_start_set_default_chart_save_location', + mocked_app_start_set_default_chart_save_location) assert app_config() is None - assert mocked_app_start_set_default_chart_save_location.called_once_with() + def test_app_config_settings_file_exists(self, monkeypatch): + def mocked_path_exists(path): + return True + + def mocked_app_start_set_default_chart_save_location(): + raise ValueError('Should not be called if settings file exists.') - @patch('dionysus_app.initialise_app.app_start_set_default_chart_save_location') - @patch('dionysus_app.initialise_app.os.path.exists') - def test_app_config_settings_file_exists(self, - mocked_os_path_exists, - mocked_app_start_set_default_chart_save_location, - ): - mocked_os_path_exists.return_value = False + monkeypatch.setattr(initialise_app.Path, 'exists', mocked_path_exists) + monkeypatch.setattr(initialise_app, 'app_start_set_default_chart_save_location', mocked_app_start_set_default_chart_save_location) assert app_config() is None - assert mocked_app_start_set_default_chart_save_location.not_called() +class TestDataFolderCheck: + @pytest.mark.parametrize( + 'default_path', + [r'./dionysus_app/app_data', + r'./dionysus_app/app_data/class_data', + ]) + def test_data_folder_check_default(self, monkeypatch, tmpdir, + default_path): + monkeypatch.setattr(data_folder, 'ROOT_DIR', tmpdir) + assert data_folder_check() is None + assert Path(tmpdir, default_path).exists() -class TestDataFolderCheck(TestCase): - def setUp(self): - self.default_paths = [ - r'./dionysus_app/app_data', - r'./dionysus_app/app_data/class_data', - ] +class TestAppInit: + def test_app_init(self, monkeypatch): + data_folder_check_mock, app_config_mock = {'called': False}, {'called': False} - def test_data_folder_check_default(self): - os.chdir(os.path.join(os.getcwd(), '.')) - data_folder_check() - for path in self.default_paths: - data_folder_path = DataFolder.generate_rel_path(path) - assert os.path.exists(data_folder_path) + def mocked_data_folder_check(): + data_folder_check_mock['called'] = True + def mocked_app_config(): + app_config_mock['called'] = True -class TestAppInit(TestCase): - def setUp(self): - pass + monkeypatch.setattr(initialise_app, 'app_config', mocked_app_config) + monkeypatch.setattr(initialise_app, 'data_folder_check', mocked_data_folder_check) - @patch('dionysus_app.initialise_app.app_config') - @patch('dionysus_app.initialise_app.data_folder_check') - def test_app_init(self, - mocked_data_folder_check, - mocked_app_config, - ): assert app_init() is None - - mocked_data_folder_check.assert_called_once_with() - mocked_app_config.assert_called_once_with() + assert data_folder_check_mock['called'] and app_config_mock['called'] From 3fad3ca4bbc4a2a8a2b8d95ec9b7d6e9641c5ace Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Mon, 23 Mar 2020 00:08:50 -0500 Subject: [PATCH 24/44] Add Optional type, change TestDataFolder to pytest. Signed-off-by: toonarmycaptain --- dionysus_app/data_folder.py | 3 ++- dionysus_app/initialise_app.py | 4 --- test_suite/test_data_folder.py | 47 +++++++++++++++------------------- 3 files changed, 22 insertions(+), 32 deletions(-) diff --git a/dionysus_app/data_folder.py b/dionysus_app/data_folder.py index edf34a4e..6dde27e7 100644 --- a/dionysus_app/data_folder.py +++ b/dionysus_app/data_folder.py @@ -1,5 +1,6 @@ from enum import Enum from pathlib import Path +from typing import Optional from definitions import ROOT_DIR @@ -24,7 +25,7 @@ class DataFolder(Enum): DEFAULT_AVATAR = CHART_GENERATOR + 'default_avatar.png' @staticmethod - def generate_rel_path(path: str) -> Path: + def generate_rel_path(path: Optional[str]) -> Path: """ Returns a abs path from relative path. eg for APP_DATA use: diff --git a/dionysus_app/initialise_app.py b/dionysus_app/initialise_app.py index bcde984d..448bb12d 100644 --- a/dionysus_app/initialise_app.py +++ b/dionysus_app/initialise_app.py @@ -38,7 +38,3 @@ def app_init() -> None: if __name__ == '__main__': pass - - -### Add temp to DataFolder? check empty on startup if [list comp files in temp]: delete them. -# Create/clear and create temp folder on startup. Clear/clear and recreate on exit. \ No newline at end of file diff --git a/test_suite/test_data_folder.py b/test_suite/test_data_folder.py index c437f036..a148dacb 100644 --- a/test_suite/test_data_folder.py +++ b/test_suite/test_data_folder.py @@ -1,5 +1,7 @@ import os +import pytest + from pathlib import Path from unittest import TestCase @@ -10,38 +12,29 @@ from dionysus_app.settings_functions import write_settings_to_file -class TestDataFolder(TestCase): - - def setUp(self): - # set correct cwd: - os.chdir(ROOT_DIR) - self.default_paths = [ - r'/dionysus_app' - r'/dionysus_app/app_data', - r'/dionysus_app/app_data/class_data', - r'/dionysus_app/app_data/class/registry.index', - r'/dionysus_app/app_data/settings.py', - r'/dionysus_app/chart_generator', - r'/dionysus_app/chart_generator/default_avatar.png', - ] - - def test_generate_data_path_defaults(self): +class TestDataFolder: + @pytest.mark.parametrize('relative_path_str', + [r'/dionysus_app' + r'/dionysus_app/app_data', + r'/dionysus_app/app_data/class_data', + r'/dionysus_app/app_data/class/registry.index', + r'/dionysus_app/app_data/settings.py', + r'/dionysus_app/chart_generator', + r'/dionysus_app/chart_generator/default_avatar.png', + ]) + def test_generate_data_path_defaults(self, relative_path_str): os.chdir(ROOT_DIR) cwd_path = Path.cwd() - for rel_path_str in self.default_paths: - path_result = DataFolder.generate_rel_path(rel_path_str) - - # Assert relative app paths in generated absolute paths: - assert rel_path_str in path_result.as_uri() - # Assert cwd in generated absolute paths: - assert cwd_path.as_uri() in path_result.as_uri() + path_result = DataFolder.generate_rel_path(relative_path_str) + + # Assert relative app paths in generated absolute paths: + assert relative_path_str in path_result.as_uri() + # Assert cwd in generated absolute paths: + assert cwd_path.as_uri() in path_result.as_uri() def test_generate_data_path_None(self): # Should return current working directory: - none_path_result = DataFolder.generate_rel_path(None) - cwd_path = Path.cwd() - - assert cwd_path == none_path_result + assert DataFolder.generate_rel_path(None) == Path.cwd() class TestDataFolderPathsExist(TestCase): From c5e63a13ad91d669a67f2d5f202de53bb1f78e1a Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Mon, 23 Mar 2020 00:18:44 -0500 Subject: [PATCH 25/44] Implement temp directory in app_data, tests. Initial use: to cache added avatars to be written database with class. initialise_app -> data_folder_check() will ensure dir exists at runtime. Add path to DataFolder. Add variable TEMP_DIR to settings_functions.py. Signed-off-by: toonarmycaptain --- dionysus_app/data_folder.py | 2 ++ dionysus_app/initialise_app.py | 1 + dionysus_app/settings_functions.py | 1 + test_suite/test_initialise_app.py | 1 + 4 files changed, 5 insertions(+) diff --git a/dionysus_app/data_folder.py b/dionysus_app/data_folder.py index 6dde27e7..d1297387 100644 --- a/dionysus_app/data_folder.py +++ b/dionysus_app/data_folder.py @@ -16,6 +16,8 @@ class DataFolder(Enum): CLASS_DATA = APP_DATA + 'class_data/' + TEMP_DIR = APP_DATA + 'temp/' + CLASS_REGISTRY = APP_DATA + 'class_registry.index' APP_SETTINGS = APP_DATA + 'settings.py' diff --git a/dionysus_app/initialise_app.py b/dionysus_app/initialise_app.py index 448bb12d..6185fc28 100644 --- a/dionysus_app/initialise_app.py +++ b/dionysus_app/initialise_app.py @@ -25,6 +25,7 @@ def data_folder_check() -> None: data_folders = { DataFolder.APP_DATA: DataFolder.generate_rel_path(DataFolder.APP_DATA.value), DataFolder.CLASS_DATA: DataFolder.generate_rel_path(DataFolder.CLASS_DATA.value), + DataFolder.TEMP_DIR: DataFolder.generate_rel_path(DataFolder.TEMP_DIR.value) } for key in data_folders: diff --git a/dionysus_app/settings_functions.py b/dionysus_app/settings_functions.py index 175fd3e8..ab49aa06 100644 --- a/dionysus_app/settings_functions.py +++ b/dionysus_app/settings_functions.py @@ -21,6 +21,7 @@ ) APP_DATA = DataFolder.generate_rel_path(DataFolder.APP_DATA.value) +TEMP_DIR = DataFolder.generate_rel_path(DataFolder.TEMP_DIR.value) APP_DEFAULT_CHART_SAVE_FOLDER = DataFolder.generate_rel_path(DataFolder.APP_DEFAULT_CHART_SAVE_FOLDER.value) APP_SETTINGS_FILE = DataFolder.generate_rel_path(DataFolder.APP_SETTINGS.value) diff --git a/test_suite/test_initialise_app.py b/test_suite/test_initialise_app.py index 8a4a26b7..aed2ca4e 100644 --- a/test_suite/test_initialise_app.py +++ b/test_suite/test_initialise_app.py @@ -40,6 +40,7 @@ class TestDataFolderCheck: 'default_path', [r'./dionysus_app/app_data', r'./dionysus_app/app_data/class_data', + r'./dionysus_app/app_data/temp' ]) def test_data_folder_check_default(self, monkeypatch, tmpdir, default_path): From 75a4b5f10ca4c3dea1ecdbdcddbf70b2a14b3b74 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Tue, 24 Mar 2020 00:25:16 -0500 Subject: [PATCH 26/44] Convert test_app_main to pytest tests. Signed-off-by: toonarmycaptain --- test_suite/test_app_main.py | 93 ++++++++++++++++++++----------------- 1 file changed, 51 insertions(+), 42 deletions(-) diff --git a/test_suite/test_app_main.py b/test_suite/test_app_main.py index 9e3fbeb9..44699fe4 100644 --- a/test_suite/test_app_main.py +++ b/test_suite/test_app_main.py @@ -1,53 +1,62 @@ """Test app_main.""" -import os +import sys -from unittest import TestCase -from unittest.mock import patch +import app_main from app_main import quit_app, run_app -class TestQuitApp(TestCase): - @patch('app_main.sys.exit') - @patch('app_main.check_registry_on_exit') - def test_quit_app(self, mock_check_registry_on_exit, mock_exit_call): +class TestQuitApp: + def test_quit_app(self, monkeypatch): + check_registry_on_exit_mock, exit_mock = {'called': False}, {'called': False} + + def mocked_check_registry_on_exit(): + check_registry_on_exit_mock['called'] = True + + def mocked_sys_exit(): + exit_mock['called'] = True + + monkeypatch.setattr(app_main, 'check_registry_on_exit', mocked_check_registry_on_exit) + monkeypatch.setattr(app_main.sys, 'exit', mocked_sys_exit) + assert quit_app() is None - mock_check_registry_on_exit.assert_called_once() - mock_exit_call.assert_called_once() - - -class TestRunApp(TestCase): - def setUp(self): - self.initial_cwd = os.getcwd() # For resetting after tests. - self.app_main_cwd = os.path.abspath('.//') # Current location of app_main. - - @patch('app_main.app_init') - @patch('app_main.cache_class_registry') - @patch('app_main.load_chart_save_folder') - @patch('app_main.run_main_menu') - @patch('app_main.quit_app') - def test_run_app_sets_cwd_correctly(self, - mocked_app_init, - mocked_cache_class_registry, - mocked_load_chart_save_folder, - mocked_run_main_menu, - mocked_quit_app, - ): - """ - Check that run_app sets cwd to location of app_main. - At present this is the folder above test_suite where - test_app_main.py is located, hence testing against '..//'. - """ + assert exit_mock['called'] and check_registry_on_exit_mock['called'] - assert run_app() is None - assert os.getcwd() == self.app_main_cwd # Ensure run_app changed cwd to location of app_main. +class TestRunApp: + def test_run_app_sets_cwd_correctly(self, monkeypatch): + os_chdir_mock, app_init_mock = {'called': False}, {'called': False} + cache_class_registry_mock, load_chart_save_folder_mock = {'called': False}, {'called': False} + run_main_menu_mock, quit_app_mock = {'called': False}, {'called': False} + + def mocked_os_chdir(path): + if path is not sys.path[0]: + raise ValueError('run_app did not change cwd to dir containing app_main.') + os_chdir_mock['called'] = True + + def mocked_app_init(): + app_init_mock['called'] = True - mocked_app_init.assert_called_once_with() - mocked_cache_class_registry.assert_called_once_with() - mocked_load_chart_save_folder.assert_called_once_with() - mocked_run_main_menu.assert_called_once_with() - mocked_quit_app.assert_called_once_with() + def mocked_cache_class_registry(): + cache_class_registry_mock['called'] = True + + def mocked_load_chart_save_folder(): + load_chart_save_folder_mock['called'] = True + + def mocked_run_main_menu(): + run_main_menu_mock['called'] = True + + def mocked_quit_app(): + quit_app_mock['called'] = True + + monkeypatch.setattr(app_main.os, 'chdir', mocked_os_chdir) + monkeypatch.setattr(app_main, 'app_init', mocked_app_init) + monkeypatch.setattr(app_main, 'cache_class_registry', mocked_cache_class_registry) + monkeypatch.setattr(app_main, 'load_chart_save_folder', mocked_load_chart_save_folder) + monkeypatch.setattr(app_main, 'run_main_menu', mocked_run_main_menu) + monkeypatch.setattr(app_main, 'quit_app', mocked_quit_app) + + assert run_app() is None - def tearDown(self): - os.chdir(self.initial_cwd) # Reset cwd if changed by tests. + assert all([os_chdir_mock['called'], app_init_mock['called'], cache_class_registry_mock['called'], + load_chart_save_folder_mock['called'], run_main_menu_mock['called'], quit_app_mock['called']]) From 13abcb868b0d632f6ebe482fadfe63bf1d174044 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Tue, 24 Mar 2020 01:06:13 -0500 Subject: [PATCH 27/44] Add clear_temp func, remove temp dir on start/exit if it contains files. Signed-off-by: toonarmycaptain --- CHANGELOG.md | 7 ++++- app_main.py | 3 +- dionysus_app/initialise_app.py | 37 ++++++++++++++++++++++-- test_suite/test_app_main.py | 16 ++++++++--- test_suite/test_initialise_app.py | 48 +++++++++++++++++++++++++++++-- 5 files changed, 100 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac32b08..3375bd1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,9 +5,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added +- `temp` directory created in `APP_DATA` by data_folder_check() on app start and removed on app exit (if it contains files). +- `NewClass` subclass of `Class` using `temp` directory to hold files before writing to database. + - Initially holds avatars. ### Changed - Separate UI/logic/persistence concerns in `create_classlist`. -- `create_classlist_data` takes a `Class` object. + - New functions `move_avatars_to_class_data`, `move_avatar_to_class_data` utilise `NewClass` to move all of the avatars from temp to database. +- `create_classlist_data` takes a `NewClass` object. - More tests converted to Pytest style tests. ## [0.5.0-alpha] - 2020-01-23 diff --git a/app_main.py b/app_main.py index 4cda1ba5..c1616367 100644 --- a/app_main.py +++ b/app_main.py @@ -7,7 +7,7 @@ import definitions from dionysus_app.class_registry_functions import cache_class_registry, check_registry_on_exit -from dionysus_app.initialise_app import app_init +from dionysus_app.initialise_app import app_init, clear_temp from dionysus_app.UI_menus.main_menu import run_main_menu from dionysus_app.settings_functions import load_chart_save_folder @@ -20,6 +20,7 @@ def quit_app(): :return: None """ check_registry_on_exit() # Dump cached registry to disk if different class_registry.index. + clear_temp() # Clear temp files. sys.exit() diff --git a/dionysus_app/initialise_app.py b/dionysus_app/initialise_app.py index 6185fc28..e952fdd2 100644 --- a/dionysus_app/initialise_app.py +++ b/dionysus_app/initialise_app.py @@ -1,7 +1,12 @@ +import os +import shutil + from pathlib import Path from dionysus_app.data_folder import DataFolder -from dionysus_app.settings_functions import APP_SETTINGS_FILE, app_start_set_default_chart_save_location +from dionysus_app.settings_functions import (APP_SETTINGS_FILE, + app_start_set_default_chart_save_location, + TEMP_DIR) def app_config() -> None: @@ -26,14 +31,40 @@ def data_folder_check() -> None: DataFolder.APP_DATA: DataFolder.generate_rel_path(DataFolder.APP_DATA.value), DataFolder.CLASS_DATA: DataFolder.generate_rel_path(DataFolder.CLASS_DATA.value), DataFolder.TEMP_DIR: DataFolder.generate_rel_path(DataFolder.TEMP_DIR.value) - } + } for key in data_folders: data_folders[key].mkdir(parents=True, exist_ok=True) +def clear_temp() -> None: + """ + Removes temp folder if there are files in it. + + Should not throw an error if temp dir doesn't exist. + + This is a candidate to run using atexit module, see + https://docs.python.org/3/library/atexit.html - but doing so without + implementing logging to log any errors would remove any files in temp that + might assist in debugging. + + :return: None + """ + if TEMP_DIR.exists() and os.listdir(TEMP_DIR): + # Log any files if logging is implemented. + shutil.rmtree(TEMP_DIR) + + def app_init() -> None: - data_folder_check() # data paths need to exist before creating/editing settings file. + """ + Clear temp folder if it contains files. + Ensures existence of key data folders. + Run app_config. + + :return: None + """ + clear_temp() + data_folder_check() # Data paths need to exist before creating/editing settings file. app_config() diff --git a/test_suite/test_app_main.py b/test_suite/test_app_main.py index 44699fe4..105046e8 100644 --- a/test_suite/test_app_main.py +++ b/test_suite/test_app_main.py @@ -8,19 +8,23 @@ class TestQuitApp: def test_quit_app(self, monkeypatch): - check_registry_on_exit_mock, exit_mock = {'called': False}, {'called': False} + check_registry_on_exit_mock, clear_temp_mock, exit_mock = {'called': False}, {'called': False}, {'called': False} def mocked_check_registry_on_exit(): check_registry_on_exit_mock['called'] = True + def mocked_clear_temp(): + clear_temp_mock['called'] = True + def mocked_sys_exit(): exit_mock['called'] = True monkeypatch.setattr(app_main, 'check_registry_on_exit', mocked_check_registry_on_exit) + monkeypatch.setattr(app_main, 'clear_temp', mocked_clear_temp) monkeypatch.setattr(app_main.sys, 'exit', mocked_sys_exit) assert quit_app() is None - assert exit_mock['called'] and check_registry_on_exit_mock['called'] + assert exit_mock['called'] and clear_temp_mock['called'] and check_registry_on_exit_mock['called'] class TestRunApp: @@ -58,5 +62,9 @@ def mocked_quit_app(): assert run_app() is None - assert all([os_chdir_mock['called'], app_init_mock['called'], cache_class_registry_mock['called'], - load_chart_save_folder_mock['called'], run_main_menu_mock['called'], quit_app_mock['called']]) + assert all([os_chdir_mock['called'], + app_init_mock['called'], + cache_class_registry_mock['called'], + load_chart_save_folder_mock['called'], + run_main_menu_mock['called'], quit_app_mock['called'] + ]) \ No newline at end of file diff --git a/test_suite/test_initialise_app.py b/test_suite/test_initialise_app.py index aed2ca4e..b12f3661 100644 --- a/test_suite/test_initialise_app.py +++ b/test_suite/test_initialise_app.py @@ -1,11 +1,14 @@ +import os + import pytest from pathlib import Path from dionysus_app import data_folder, initialise_app from dionysus_app.initialise_app import (app_config, - data_folder_check, app_init, + clear_temp, + data_folder_check, ) @@ -30,7 +33,8 @@ def mocked_app_start_set_default_chart_save_location(): raise ValueError('Should not be called if settings file exists.') monkeypatch.setattr(initialise_app.Path, 'exists', mocked_path_exists) - monkeypatch.setattr(initialise_app, 'app_start_set_default_chart_save_location', mocked_app_start_set_default_chart_save_location) + monkeypatch.setattr(initialise_app, 'app_start_set_default_chart_save_location', + mocked_app_start_set_default_chart_save_location) assert app_config() is None @@ -49,6 +53,46 @@ def test_data_folder_check_default(self, monkeypatch, tmpdir, assert Path(tmpdir, default_path).exists() +class TestClearTemp: + def test_clear_temp_files_cleared(self, monkeypatch, tmpdir): + test_temp_dir = Path(tmpdir, 'temp_dir') + test_temp_dir.mkdir(parents=True) # Make temp_dir. + Path(test_temp_dir, 'some_dir').mkdir(parents=True) # Make file in test_temp_dir. + assert os.listdir(test_temp_dir) # File in test_temp_dir. + + monkeypatch.setattr(initialise_app, 'TEMP_DIR', test_temp_dir) + + assert clear_temp() is None + assert not test_temp_dir.exists() + + def test_clear_temp_dir_nonexistent(self, monkeypatch, tmpdir): + test_temp_dir = Path(tmpdir, 'temp_dir') + assert not test_temp_dir.exists() # No temp dir. + + def mocked_rmtree(path): + raise ValueError("rmtree should not be called temp directory doesn't exist.") + + monkeypatch.setattr(initialise_app, 'TEMP_DIR', test_temp_dir) + monkeypatch.setattr(initialise_app.shutil, 'rmtree', mocked_rmtree) + + assert clear_temp() is None + assert not test_temp_dir.exists() + + def test_clear_temp_no_files_not_removed(self, monkeypatch, tmpdir): + test_temp_dir = Path(tmpdir, 'temp_dir') + test_temp_dir.mkdir(parents=True) # Make temp_dir + assert not os.listdir(test_temp_dir) # No files in test_temp_dir. + + def mocked_rmtree(path): + raise ValueError("rmtree should not be called when no files in temp directory.") + + monkeypatch.setattr(initialise_app, 'TEMP_DIR', test_temp_dir) + monkeypatch.setattr(initialise_app.shutil, 'rmtree', mocked_rmtree) + + assert clear_temp() is None + assert test_temp_dir.exists() # Directory not removed as already empty. + + class TestAppInit: def test_app_init(self, monkeypatch): data_folder_check_mock, app_config_mock = {'called': False}, {'called': False} From 1ac06fc680d41d3745e2c1dcc857c7607213a82c Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Wed, 25 Mar 2020 23:37:57 -0500 Subject: [PATCH 28/44] Convert TestClassNamePathSafeName to pytest style tests. Signed-off-by: toonarmycaptain --- test_suite/test_class.py | 83 +++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 44 deletions(-) diff --git a/test_suite/test_class.py b/test_suite/test_class.py index 6b86e9c8..bdc17d07 100644 --- a/test_suite/test_class.py +++ b/test_suite/test_class.py @@ -1,9 +1,7 @@ """Tests for class.py""" import pytest -from unittest import TestCase -from unittest.mock import patch - +from dionysus_app import class_ from dionysus_app.class_ import Class from dionysus_app.file_functions import convert_to_json from dionysus_app.student import Student @@ -55,65 +53,62 @@ def test_class_student_list_instantiation_no_list_arg(): class TestClassNamePathSafeName: - """ - Test Class name and path_safe_name properties. - """ + """Test Class name and path_safe_name properties.""" - def setup_method(self): - """ - Setup class name and name for change. - """ + def test_name_getter(self): + test_name = "The Knights of the Round-table: we don't say 'Ni!'" + assert Class(test_name).name == test_name + + def test_path_safe_name_getter(self): + assert Class("The Knights of the Round-table: we don't say 'Ni!'" + ).path_safe_name == "The_Knights_of_the_Round-table__we_don_t_say__Ni__" - self.test_name = "The Knights of the Round-table: we don't say 'Ni!'" - self.test_path_safe_name = "The_Knights_of_the_Round-table__we_don_t_say__Ni__" + def test_path_safe_name_getter_mocking_calls(self, monkeypatch): + mock_path_safe_name = "Something completely different." - self.test_changed_name = 'Adaptable Knights: We now say Ni!, but we dont have to.' - self.test_changed_path_safe_name = 'Adaptable_Knights__We_now_say_Ni___but_we_dont_have_to_' + def mocked_clean_for_filename(class_name): + return mock_path_safe_name - def test_name_getter(self, test_class_name_only): - assert test_class_name_only.name == self.test_name + monkeypatch.setattr(class_, 'clean_for_filename', mocked_clean_for_filename) - def test_path_safe_name_getter(self, test_class_name_only): - assert test_class_name_only.path_safe_name == self.test_path_safe_name + assert Class("The Knights of the Round-table: we don't say 'Ni!'").path_safe_name == mock_path_safe_name - def test_name_setter_unmocked(self, test_class_name_only): - assert test_class_name_only.name != self.test_changed_name - assert test_class_name_only.path_safe_name != self.test_changed_path_safe_name + def test_name_setter(self): + test_name = "The Knights of the Round-table: we don't say 'Ni!'" + + test_changed_name = 'Adaptable Knights: We now say Ni!, but we dont have to.' + test_changed_path_safe_name = 'Adaptable_Knights__We_now_say_Ni___but_we_dont_have_to_' + + test_class = Class(test_name) + + # Original test_class attributes not equal to changed: + assert (test_class.name, test_class.path_safe_name) != (test_changed_name, test_changed_path_safe_name) # Change name - test_class_name_only.name = self.test_changed_name + test_class.name = test_changed_name - assert test_class_name_only.name == self.test_changed_name - # Assert name change carried over to path_safe_name attribute. - assert test_class_name_only.path_safe_name == self.test_changed_path_safe_name + assert (test_class.name, test_class.path_safe_name) == (test_changed_name, test_changed_path_safe_name) + def test_name_setter_mocking_calls(self, monkeypatch): + test_name = "The Knights of the Round-table: we don't say 'Ni!'" -class TestClassNameMocked(TestCase): - def setUp(self): - self.test_name = "The Knights of the Round-table: we don't say 'Ni!'" - self.test_path_safe_name = "The_Knights_of_the_Round-table_we_don't_say__Ni__" + test_changed_name = 'Adaptable Knights: We now say Ni!, but we dont have to.' + mock_changed_path_safe_name = "Adaptable_Knights: We're Niiiearly completely un!safe?!$" - self.test_class = Class(self.test_name) + test_class = Class(test_name) - self.test_changed_name = 'Adaptable Knights: We now say Ni!, but we dont have to.' - self.test_changed_path_safe_name = 'Adaptable_Knights__We_now_say_Ni___but_we_dont_have_to_' + # Original test_class attributes not equal to changed: + assert (test_class.name, test_class.path_safe_name) != (test_changed_name, mock_changed_path_safe_name) - @patch('dionysus_app.class_.clean_for_filename') - def test_name_setter_mocked(self, mocked_clean_for_filename): - # Assert name, path_safe_name initial value != changed value - assert self.test_class.name != self.test_changed_name - assert self.test_class.path_safe_name != self.test_changed_path_safe_name + def mocked_clean_for_filename(class_name): + return mock_changed_path_safe_name - mocked_clean_for_filename.return_value = self.test_changed_path_safe_name + monkeypatch.setattr(class_, 'clean_for_filename', mocked_clean_for_filename) # Change name - self.test_class.name = self.test_changed_name - - assert self.test_class.name == self.test_changed_name - # Assert name change carried over to path_safe_name attribute. - assert self.test_class.path_safe_name == self.test_changed_path_safe_name + test_class.name = test_changed_name - mocked_clean_for_filename.assert_called_once_with(self.test_changed_name) + assert (test_class.name, test_class.path_safe_name) == (test_changed_name, mock_changed_path_safe_name) class TestContainsMethod: From fd7468f6137977e4efb0c44a708c7dca28d7fa47 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Thu, 26 Mar 2020 00:34:38 -0500 Subject: [PATCH 29/44] Add NewClass subclass of Class. This subclass implements temp dir for class in app temp directory, with 'avatars' dir for holding user avatars before writing to database. Signed-off-by: toonarmycaptain --- dionysus_app/class_.py | 52 ++++++++++++++++++++++++++++++++++++++++ test_suite/test_class.py | 37 +++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/dionysus_app/class_.py b/dionysus_app/class_.py index b93c62ef..4a375b2b 100644 --- a/dionysus_app/class_.py +++ b/dionysus_app/class_.py @@ -1,11 +1,14 @@ """Class for class data.""" import json +import shutil +import tempfile from pathlib import Path from typing import Any, List, Union from dionysus_app.file_functions import convert_to_json from dionysus_app.student import Student +from dionysus_app.settings_functions import TEMP_DIR from dionysus_app.UI_menus.UI_functions import clean_for_filename @@ -236,3 +239,52 @@ def __str__(self): students_stmt = 'containing 0 students' return f'Class {self.name}, {students_stmt}.' + + +class NewClass(Class): + """ + Subclass of Class for creating new classes, machinery to facilitate + creation of new classes in database. + + Adds temp directory for class to cache files before writing to database. + This temp directory is garbage collected with the object upon object + destruction. + + Temp directory is prefixed with the class' name, plus a hash, and contains + a subdirectory named 'avatars' to hold avatars before writing to database: + + TEMP_DIR + ├── class_name+hash + | ├── avatars + + NB: In future may be subclassed or housed in Database objects to allow database + specific implementation. + ... + + Attributes + ---------- + As for baseclass Class. + + temp_dir: Path + Path to class' temp directory. + + temp_avatars_dir: Path + Path to avatars folder in class' temp directory. + """ + def __init__(self, name: str, students: List[Student] = None): + super().__init__(name, students) + + # Create class temp directory. + Path.mkdir(TEMP_DIR, exist_ok=True, parents=True) # Ensure path exists. + self.temp_dir: Path = Path(tempfile.mkdtemp(prefix=self.name, dir=TEMP_DIR)) + # Create avatars directory within class temp directory. + self.temp_avatars_dir: Path = self.temp_dir.joinpath('avatars') + Path.mkdir(self.temp_avatars_dir) + + def __del__(self) -> None: + """ + Temp folder garbage collected with object. + + :return: None + """ + shutil.rmtree(self.temp_dir) diff --git a/test_suite/test_class.py b/test_suite/test_class.py index bdc17d07..2bc3d9b2 100644 --- a/test_suite/test_class.py +++ b/test_suite/test_class.py @@ -1,8 +1,12 @@ """Tests for class.py""" +import os + import pytest +from pathlib import Path + from dionysus_app import class_ -from dionysus_app.class_ import Class +from dionysus_app.class_ import Class, NewClass from dionysus_app.file_functions import convert_to_json from dionysus_app.student import Student @@ -363,3 +367,34 @@ class TestClassStr: ]) def test_str(self, class_object, expected_str): assert str(class_object) == expected_str + + +class TestNewClass: + def test_new_class_temp_dir_created(self, monkeypatch, tmpdir): + test_temp_dir = Path(tmpdir, 'temp_dir') + test_temp_dir.mkdir(parents=True) # Make temp_dir. + assert not os.listdir(test_temp_dir) # Nothing in test_temp_dir. + + monkeypatch.setattr(class_, 'TEMP_DIR', test_temp_dir) + + test_class = NewClass("Sir Robin's baboons") + + assert test_class.temp_dir.exists() + assert test_class.temp_dir.name in os.listdir(test_temp_dir) + # Check temp_avatars_dir + assert test_class.temp_avatars_dir.exists() + # noinspection PyTypeChecker + assert test_class.temp_avatars_dir.name in os.listdir(test_class.temp_dir) + + def test_new_class_temp_dir_deleted_on_deletion(self, monkeypatch, tmpdir): + test_temp_dir = Path(tmpdir, 'temp_dir') + test_temp_dir.mkdir(parents=True) # Make temp_dir. + assert not os.listdir(test_temp_dir) # Nothing in test_temp_dir. + + monkeypatch.setattr(class_, 'TEMP_DIR', test_temp_dir) + + test_class = NewClass("Sir Robin's baboons") + assert test_class.temp_dir.exists() and os.listdir(test_temp_dir) # Class temp dir in test_temp_dir. + del test_class # NB May throw an (ignored) Exception because the class is garbage collected before this line. + # No class temp dir in test_temp_dir: + assert not os.listdir(test_temp_dir) From 4459f1b9dfc787aa5c7e98219e16ccb3853b7ef8 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Thu, 26 Mar 2020 00:41:09 -0500 Subject: [PATCH 30/44] Implement NewClass in create_classlist_data. Cache avatars in temp folder, then move to class_data/database when class is written. take_student_avatar now takes NewClass instead of a class name, copies avatars to NewClass.temp_avatars_dir using file_functions.copy_file. create_classlist_data copies cached avatars after writing class to file. Added functions: copy_avatars_to_class_data, copy_avatar_to_class_data Remove functions: copy_avatar_to_app_data Note this solves issue caused by creating folders etc after taking user input, as user adding avatar was trying to copy to new class folders that didn't exist yet due to previous change that separated logic from writing to disk/database, postponing folder creation until writing, where previously avatars were copied to created folder one by one as user supplied them for each added student. NB Above issue is a good candidate for end-to-testing if/when this is implemented. Signed-off-by: toonarmycaptain --- dionysus_app/class_functions.py | 67 ++++++---- test_suite/test_class_functions.py | 207 +++++++++++++++-------------- 2 files changed, 145 insertions(+), 129 deletions(-) diff --git a/dionysus_app/class_functions.py b/dionysus_app/class_functions.py index b5bdb9da..aee3a6b5 100644 --- a/dionysus_app/class_functions.py +++ b/dionysus_app/class_functions.py @@ -9,13 +9,13 @@ import definitions -from dionysus_app.class_ import Class +from dionysus_app.class_ import Class, NewClass from dionysus_app.student import Student from dionysus_app.class_registry_functions import register_class from dionysus_app.data_folder import DataFolder, CLASSLIST_DATA_FILE_TYPE from dionysus_app.file_functions import (copy_file, load_from_json_file, - ) + move_file) from dionysus_app.UI_menus.class_functions_UI import (blank_class_dialogue, class_data_feedback, create_chart_with_new_class_dialogue, @@ -48,7 +48,6 @@ def create_classlist() -> None: create_classlist_data(new_class) # Future: Call to Database.create_class(new_class) time.sleep(2) # Pause for user to look over feedback. - create_chart_with_new_class(classlist_name) @@ -98,7 +97,7 @@ def setup_class_data_storage(classlist_name: str) -> None: user_chart_save_folder.mkdir(exist_ok=True, parents=True) -def create_classlist_data(new_class: Class) -> None: +def create_classlist_data(new_class: NewClass) -> None: """ Creates class data in persistence. Calls setup_class to create any needed files, then writes data to file. @@ -108,9 +107,38 @@ def create_classlist_data(new_class: Class) -> None: """ setup_class(new_class.name) # -> functionality moves to (ie function called by) JSONDatabase.create_class write_classlist_to_file(new_class) + move_avatars_to_class_data(new_class) + + +def move_avatars_to_class_data(new_class: NewClass) -> None: + """ + Move avatars from NewClass.temp_dir to new class' avatars directory. + + :param new_class: NewClass + :return: None + """ + for avatar_file in [student.avatar_filename for student in new_class.students + if student.avatar_filename]: + move_avatar_to_class_data(new_class, avatar_file) + +def move_avatar_to_class_data(new_class: NewClass, avatar_filename: str) -> None: + """ + Moves avatar from NewClass.temp_dir to new class' avatars directory. + Will not repeat moves of same filename if image already exists in avatars + directory, to avoid repeated overwrite with same file. -def compose_classlist_dialogue(class_name: str) -> Class: + :param new_class: NewClass + :param avatar_filename: str + :return: None + """ + origin_path = new_class.temp_avatars_dir.joinpath(avatar_filename) + destination_path = CLASSLIST_DATA_PATH.joinpath(new_class.name, 'avatars', avatar_filename) + if not destination_path.exists(): # Avatar not already in database/class data. + move_file(origin_path, destination_path) + + +def compose_classlist_dialogue(class_name: str) -> NewClass: """ Call UI elements to collect new class data. Provide feedback to user reflecting new class composition. @@ -139,28 +167,30 @@ def compose_classlist_dialogue(class_name: str) -> Class: return new_class -def take_class_data_input(class_name: str) -> Class: +def take_class_data_input(class_name: str) -> NewClass: """ Take student names, avatars, return Class object. :param class_name: str :return: Class """ - new_class = Class(name=class_name) + new_class = NewClass(name=class_name) while True: student_name = take_student_name_input(new_class) if student_name.upper() == 'END': break - avatar_filename = take_student_avatar(class_name, student_name) + avatar_filename = take_student_avatar(new_class, student_name) new_class.add_student(name=student_name, avatar_filename=avatar_filename) return new_class -def take_student_avatar(class_name: str, student_name: str) -> Optional[str]: +def take_student_avatar(new_class: NewClass, student_name: str) -> Optional[str]: """ + Get avatar for student: Prompts user for path to avatar file. + Copies avatar file to temp dir for class. - :param class_name: str + :param new_class: NewClass class object :param student_name: str :return: str or None """ @@ -175,25 +205,10 @@ def take_student_avatar(class_name: str, student_name: str) -> Optional[str]: # TODO: process_student_avatar() # TODO: convert to png - copy_avatar_to_app_data(class_name, avatar_file, target_avatar_filename) - + copy_file(avatar_file, new_class.temp_avatars_dir.joinpath(target_avatar_filename)) return target_avatar_filename -def copy_avatar_to_app_data(classlist_name: str, avatar_filename: str, save_filename: str) -> None: - """ - Copies given avatar image to classlist_name/avatars/ with given save_filename. - No need to pre-check if file exists because it could not be selected if it did not exist. - - :param classlist_name: str - :param avatar_filename: str or Path - :param save_filename: str or Path - :return: None - """ - save_avatar_path = CLASSLIST_DATA_PATH.joinpath(classlist_name, 'avatars', save_filename) - copy_file(avatar_filename, save_avatar_path) - - def avatar_file_exists(avatar_file: Union[str, Path]) -> bool: """ Checks if provided file exists. diff --git a/test_suite/test_class_functions.py b/test_suite/test_class_functions.py index 39b3bae6..0a7a7570 100644 --- a/test_suite/test_class_functions.py +++ b/test_suite/test_class_functions.py @@ -12,10 +12,9 @@ from dionysus_app import class_functions, class_registry_functions from dionysus_app.chart_generator import create_chart -from dionysus_app.class_ import Class +from dionysus_app.class_ import Class, NewClass from dionysus_app.class_functions import (avatar_path_from_string, compose_classlist_dialogue, - copy_avatar_to_app_data, create_chart_with_new_class, create_classlist, create_classlist_data, @@ -24,6 +23,8 @@ get_avatar_path, load_chart_data, load_class_from_disk, + move_avatar_to_class_data, + move_avatars_to_class_data, select_classlist, select_student, setup_class, @@ -128,12 +129,93 @@ def mocked_write_classlist_to_file(test_class): if test_class != test_full_class: raise ValueError + def mocked_copy_avatars_to_class_data(test_class): + if test_class != test_full_class: + raise ValueError + monkeypatch.setattr(class_functions, 'setup_class', mocked_setup_class) monkeypatch.setattr(class_functions, 'write_classlist_to_file', mocked_write_classlist_to_file) + monkeypatch.setattr(class_functions, 'move_avatars_to_class_data', mocked_copy_avatars_to_class_data) assert create_classlist_data(test_full_class) is None +class TestMoveAvatarsToClassData: + @pytest.mark.parametrize( + 'test_new_class, calls_to_mock_move_avatars_to_class_data', + [(NewClass('test empty class'), []), # No students + # All students have avatar_filename + (NewClass.from_dict({'name': 'test_class', + 'students': [{'name': 'Cali', 'avatar_filename': 'Cali_avatar.png'}, + {'name': 'Zach', 'avatar_filename': 'Zach_avatar.png'}, + {'name': 'Ashley', 'avatar_filename': 'Ashley_avatar.png'}, + {'name': 'Danielle', 'avatar_filename': 'Danielle.png'}, ]}), + ['Cali_avatar.png', + 'Zach_avatar.png', + 'Ashley_avatar.png', + 'Danielle.png', + ]), + # Some students have avatar_filename + (NewClass.from_dict(test_full_class_data_set['json_dict_rep']), ['Cali_avatar.png', + 'Zach_avatar.png', + 'Ashley_avatar.png', + 'Danielle.png', + ] + ), + # No students have avatar_filename + (NewClass.from_dict({'name': 'test_class', + 'students': [{'name': 'Cali'}, + {'name': 'Zach'}, + {'name': 'Ashley'}, + {'name': 'Danielle'}, ]}), + []), + ]) + def test_move_avatars_to_class_data(self, monkeypatch, + test_new_class, calls_to_mock_move_avatars_to_class_data): + moved_avatars = [] + + def mock_move_avatar_to_class_data(test_class: NewClass, avatar_filename: str): + moved_avatars.append(avatar_filename) + + monkeypatch.setattr(class_functions, 'move_avatar_to_class_data', mock_move_avatar_to_class_data) + + assert move_avatars_to_class_data(test_new_class) is None + assert moved_avatars == calls_to_mock_move_avatars_to_class_data + + +class TestMoveAvatarToClassData: + def test_move_avatar_to_class_data(self, monkeypatch, tmpdir): + test_class = NewClass('test_class') + test_filename = 'test avatar filename' + + def mock_move_file(origin_path: Path, destination_path: Path): + if origin_path != Path(test_class.temp_dir, 'avatars', test_filename): + raise ValueError("Origin path incorrect.") + if destination_path != Path(tmpdir, test_class.name, 'avatars', test_filename): + raise ValueError("Destination path incorrect") + + monkeypatch.setattr(class_functions, 'move_file', mock_move_file) + monkeypatch.setattr(class_functions, 'CLASSLIST_DATA_PATH', Path(tmpdir)) + + assert move_avatar_to_class_data(test_class, test_filename) is None + + def test_move_avatar_to_class_data_avatar_preexisting(self, monkeypatch, tmpdir): + test_class = NewClass('test_class') + # Make existing avatar in tmpdir test_class class data: + destination_avatar_path = Path(tmpdir, test_class.name, 'avatars', 'test_avatar_filename') + Path.mkdir(destination_avatar_path.parent, parents=True) + with open(destination_avatar_path, 'w'): + pass + + def mock_move_file(origin_path: Path, destination_path: Path): + raise ValueError("Move file should not be called.") + + monkeypatch.setattr(class_functions, 'move_file', mock_move_file) + monkeypatch.setattr(class_functions, 'CLASSLIST_DATA_PATH', Path(tmpdir)) + + assert move_avatar_to_class_data(test_class, destination_avatar_path.name) is None + + class TestComposeClasslistDialogue: def test_compose_classlist_dialogue_full_class(self, monkeypatch, test_full_class): def mocked_take_class_data_input(test_class_name): @@ -216,6 +298,8 @@ def test_take_class_data_input(self, mock_take_student_name_input, class TestTakeStudentAvatar: def test_take_student_avatar_no_avatar(self, monkeypatch): + test_class = NewClass('some_class') + def mocked_select_avatar_file_dialogue(): return None # No file selected @@ -223,140 +307,57 @@ def mocked_select_avatar_file_dialogue(): # Ensure calls to other funcs will cause error. monkeypatch.delattr(class_functions, 'clean_for_filename') - monkeypatch.delattr(class_functions, 'copy_avatar_to_app_data') + monkeypatch.delattr(class_functions, 'copy_file') - assert take_student_avatar('some class', 'some student') is None + assert take_student_avatar(test_class, 'some student') is None def test_take_student_avatar_pre_clean_name(self, monkeypatch): - test_class_name = 'some class' + test_class = NewClass('some_class') test_student_name = 'clean name' - test_avatar_filename = 'avatar file name' + test_avatar_filepath = Path('avatar file name') cleaned_student_name = 'file name was already clean' returned_filename = f'{cleaned_student_name}.png' def mocked_select_avatar_file_dialogue(): - return test_avatar_filename + return test_avatar_filepath def mocked_clean_for_filename(student_name): if student_name != test_student_name: raise ValueError # Ensure called with correct arg. return cleaned_student_name - def mocked_copy_avatar_to_app_data(class_name, avatar_filename, target_filename): - if (class_name, avatar_filename, target_filename) != ( - test_class_name, test_avatar_filename, returned_filename): + def mocked_copy_file(avatar_filepath, destination_filepath): + if (avatar_filepath, destination_filepath) != ( + test_avatar_filepath, test_class.temp_avatars_dir.joinpath(returned_filename)): raise ValueError # Ensure called with correct args. return None monkeypatch.setattr(class_functions, 'select_avatar_file_dialogue', mocked_select_avatar_file_dialogue) monkeypatch.setattr(class_functions, 'clean_for_filename', mocked_clean_for_filename) - monkeypatch.setattr(class_functions, 'copy_avatar_to_app_data', mocked_copy_avatar_to_app_data) + monkeypatch.setattr(class_functions, 'copy_file', mocked_copy_file) - assert take_student_avatar(test_class_name, test_student_name) == returned_filename + assert take_student_avatar(test_class, test_student_name) == returned_filename def test_take_student_avatar_dirty_name(self, monkeypatch): - test_class_name = 'some class' + test_class = NewClass('some_class') test_student_name = r'very unsafe */^@ :$ name' - test_avatar_filename = 'avatar file name' + test_avatar_filepath = Path('avatar file name') returned_filename = f'{clean_for_filename(test_student_name)}.png' def mocked_select_avatar_file_dialogue(): - return test_avatar_filename + return test_avatar_filepath - def mocked_copy_avatar_to_app_data(class_name, avatar_filename, target_filename): - if (class_name, avatar_filename, target_filename) != ( - test_class_name, test_avatar_filename, returned_filename): + def mocked_copy_file(avatar_filepath, destination_filepath): + if (avatar_filepath, destination_filepath) != ( + test_avatar_filepath, test_class.temp_avatars_dir.joinpath(returned_filename)): raise ValueError # Ensure called with correct args. return None monkeypatch.setattr(class_functions, 'select_avatar_file_dialogue', mocked_select_avatar_file_dialogue) - monkeypatch.setattr(class_functions, 'copy_avatar_to_app_data', mocked_copy_avatar_to_app_data) - - assert take_student_avatar(test_class_name, test_student_name) == returned_filename - - -class TestCopyAvatarToAppData(TestCase): - mock_CLASSLIST_DATA_PATH = Path('.') - mock_DEFAULT_CHART_SAVE_FOLDER = Path('my_charts') - - # Need to mock globals in setUp call of setup_class_data_storage - @patch('dionysus_app.class_functions.CLASSLIST_DATA_PATH', mock_CLASSLIST_DATA_PATH) - @patch('dionysus_app.class_functions.definitions.DEFAULT_CHART_SAVE_FOLDER', mock_DEFAULT_CHART_SAVE_FOLDER) - def setUp(self): - self.mock_CLASSLIST_DATA_PATH = Path('.') - self.mock_DEFAULT_CHART_SAVE_FOLDER = Path('my_charts') - - # arguments to copy_avatar_to_app_data - self.test_classlist_name = 'arthurs_knights' - self.test_avatar_filename = 'sir_lancelot_the_looker.image' - self.copied_avatar_save_filename = 'sir_lancelot.png' - - # create test file and structure. - with open(self.test_avatar_filename, 'w+') as avatar_file: - pass + monkeypatch.setattr(class_functions, 'copy_file', mocked_copy_file) - # Setup test class storage, - setup_class_data_storage(self.test_classlist_name) - self.test_class_datafolder_path = self.mock_CLASSLIST_DATA_PATH.joinpath(self.test_classlist_name) - self.test_class_avatar_subfolder_path = self.test_class_datafolder_path.joinpath('avatars') - self.test_class_chart_data_subfolder_path = self.test_class_datafolder_path.joinpath('chart_data') - - self.copied_avatar_filepath = self.test_class_avatar_subfolder_path.joinpath(self.copied_avatar_save_filename) - - # assert test preconditions met - assert os.path.exists(self.test_avatar_filename) - assert os.path.exists(self.test_class_avatar_subfolder_path) - - assert not os.path.exists(self.copied_avatar_filepath) - - @patch('dionysus_app.class_functions.CLASSLIST_DATA_PATH', mock_CLASSLIST_DATA_PATH) - @patch('dionysus_app.class_functions.definitions.DEFAULT_CHART_SAVE_FOLDER', mock_DEFAULT_CHART_SAVE_FOLDER) - def test_copy_avatar_to_app_data(self): - copy_avatar_to_app_data(self.test_classlist_name, self.test_avatar_filename, self.copied_avatar_save_filename) - assert os.path.exists(self.copied_avatar_filepath) - - def tearDown(self): - os.remove(self.test_avatar_filename) # Remove test avatar file - assert not os.path.exists(self.test_avatar_filename) - - # Remove tree created in setup_class_data_storage - shutil.rmtree(self.test_class_datafolder_path) - assert not os.path.exists(self.test_class_datafolder_path) - shutil.rmtree(self.mock_DEFAULT_CHART_SAVE_FOLDER) - assert not os.path.exists(self.mock_DEFAULT_CHART_SAVE_FOLDER) - - -class TestCopyAvatarToAppDataMockingCopyfile(TestCase): - mock_CLASSLIST_DATA_PATH = Path('.') - mock_DEFAULT_CHART_SAVE_FOLDER = Path('my_charts') - - def setUp(self): - self.mock_CLASSLIST_DATA_PATH = Path('.') - self.mock_DEFAULT_CHART_SAVE_FOLDER = Path('my_charts') - - # arguments to copy_avatar_to_app_data - self.test_classlist_name = 'arthurs_knights' - self.test_avatar_filename = 'sir_lancelot_the_looker.image' - self.copied_avatar_save_filename = 'sir_lancelot.png' - - # Setup test class storage paths - self.test_class_datafolder_path = self.mock_CLASSLIST_DATA_PATH.joinpath(self.test_classlist_name) - self.test_class_avatar_subfolder_path = self.test_class_datafolder_path.joinpath('avatars') - self.test_class_chart_data_subfolder_path = self.test_class_datafolder_path.joinpath('chart_data') - - self.copied_avatar_filepath = self.test_class_avatar_subfolder_path.joinpath(self.copied_avatar_save_filename) - - # assert test preconditions met - assert not os.path.exists(self.copied_avatar_filepath) - - @patch('definitions.DEFAULT_CHART_SAVE_FOLDER', mock_DEFAULT_CHART_SAVE_FOLDER) - @patch('dionysus_app.class_functions.CLASSLIST_DATA_PATH', mock_CLASSLIST_DATA_PATH) - def test_copy_avatar_to_app_data_copy_file_mocked(self): - with patch('dionysus_app.class_functions.copy_file') as mocked_copy_file: - copy_avatar_to_app_data(self.test_classlist_name, self.test_avatar_filename, - self.copied_avatar_save_filename) - mocked_copy_file.assert_called_once_with(self.test_avatar_filename, self.copied_avatar_filepath) + assert take_student_avatar(test_class, test_student_name) == returned_filename class TestWriteClasslistToFile(TestCase): From d38f840672ab3b407bceec7296b535ea267cae86 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Thu, 26 Mar 2020 00:41:58 -0500 Subject: [PATCH 31/44] Typing and code style improvements. Signed-off-by: toonarmycaptain --- dionysus_app/UI_menus/class_functions_UI.py | 5 +++-- dionysus_app/class_functions.py | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dionysus_app/UI_menus/class_functions_UI.py b/dionysus_app/UI_menus/class_functions_UI.py index 3933e8c9..3ada9776 100644 --- a/dionysus_app/UI_menus/class_functions_UI.py +++ b/dionysus_app/UI_menus/class_functions_UI.py @@ -1,5 +1,6 @@ """UI elements for class_functions""" -from typing import Union +from pathlib import Path +from typing import Optional, Union from dionysus_app.class_ import Class from dionysus_app.class_registry_functions import classlist_exists @@ -183,7 +184,7 @@ def take_student_selection(student_options: dict): return selected_student -def select_avatar_file_dialogue(): +def select_avatar_file_dialogue() -> Optional[Path]: """ Prompts user to select an avatar file. Currently only displays PNG files by default. diff --git a/dionysus_app/class_functions.py b/dionysus_app/class_functions.py index aee3a6b5..852bf6b6 100644 --- a/dionysus_app/class_functions.py +++ b/dionysus_app/class_functions.py @@ -243,7 +243,6 @@ def write_classlist_to_file(current_class: Class) -> None: classlist_file.write(json_class_data) - def create_chart_with_new_class(classlist_name): if create_chart_with_new_class_dialogue(): from dionysus_app.chart_generator.create_chart import new_chart From b4372252a10f4f8f9114aba1d097a9aa9bc120dd Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Thu, 26 Mar 2020 11:13:51 +0000 Subject: [PATCH 32/44] Bump setuptools from 46.1.1 to 46.1.3 Bumps [setuptools](https://github.com/pypa/setuptools) from 46.1.1 to 46.1.3. - [Release notes](https://github.com/pypa/setuptools/releases) - [Changelog](https://github.com/pypa/setuptools/blob/master/CHANGES.rst) - [Commits](https://github.com/pypa/setuptools/compare/v46.1.1...v46.1.3) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index f084ee7e..b6a9fa55 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ matplotlib==3.2.1 Pillow==7.0.0 -setuptools==46.1.1 \ No newline at end of file +setuptools==46.1.3 \ No newline at end of file From efbc8a5ec1d0d61305c500a5b2f53aecd2dea319 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Sat, 28 Mar 2020 00:39:41 -0500 Subject: [PATCH 33/44] Typing and code style improvements. Signed-off-by: toonarmycaptain --- dionysus_app/class_functions.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dionysus_app/class_functions.py b/dionysus_app/class_functions.py index 852bf6b6..de8c54cb 100644 --- a/dionysus_app/class_functions.py +++ b/dionysus_app/class_functions.py @@ -243,7 +243,14 @@ def write_classlist_to_file(current_class: Class) -> None: classlist_file.write(json_class_data) -def create_chart_with_new_class(classlist_name): +def create_chart_with_new_class(classlist_name: str) -> None: + """ + Prompt to create a chart with a newly created class, call new_chart with + newly created class if user desires. + + :param classlist_name: str + :return: None + """ if create_chart_with_new_class_dialogue(): from dionysus_app.chart_generator.create_chart import new_chart new_chart(classlist_name) From 7fa1ef183435a04c99480025f43f3266753c4f7f Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Sat, 28 Mar 2020 13:21:15 -0500 Subject: [PATCH 34/44] Rename test to reflect function. Signed-off-by: toonarmycaptain --- test_suite/UI_menus_tests/test_class_functions_UI.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_suite/UI_menus_tests/test_class_functions_UI.py b/test_suite/UI_menus_tests/test_class_functions_UI.py index cba4f9eb..756e1a47 100644 --- a/test_suite/UI_menus_tests/test_class_functions_UI.py +++ b/test_suite/UI_menus_tests/test_class_functions_UI.py @@ -275,7 +275,7 @@ def test_class_data_feedback_with_empty_class(self, test_class_name_only, capsys assert captured == ''.join(printed_strings) -class TestCreateChartWithNewClass: +class TestCreateChartWithNewClassDialogue: @pytest.mark.parametrize( 'inputs, returned_value', [([bad_input, good_input], return_value) @@ -286,7 +286,7 @@ class TestCreateChartWithNewClass: ('Y', True), ] ] ) - def test_create_chart_with_new_class(self, inputs, returned_value): + def test_create_chart_with_new_class_dialogue(self, inputs, returned_value): with mock.patch('builtins.input', side_effect=inputs): assert create_chart_with_new_class_dialogue() is returned_value From 525979548469624d359be64fd60d0993ab117209 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Sat, 28 Mar 2020 21:47:36 -0500 Subject: [PATCH 35/44] Add test_new_class_name_only, test_full_new_class fixtures. Signed-off-by: toonarmycaptain --- test_suite/test_class.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test_suite/test_class.py b/test_suite/test_class.py index 2bc3d9b2..55d2339c 100644 --- a/test_suite/test_class.py +++ b/test_suite/test_class.py @@ -38,6 +38,31 @@ def test_full_class(): return test_full_class +@pytest.fixture() +def test_new_class_name_only(): + """Returns empty NewClass instantiated with name only.""" + test_new_class_name_only = NewClass(test_class_name_only_data_set['json_dict_rep']['name']) + + # Add attributes to test expected output. + test_new_class_name_only.json_str_rep = test_class_name_only_data_set['json_str_rep'] + test_new_class_name_only.json_dict_rep = test_class_name_only_data_set['json_dict_rep'] + + return test_new_class_name_only + + +@pytest.fixture() +def test_full_new_class(): + """Returns empty NewClass instantiated with students.""" + test_full_new_class = NewClass(test_full_class_data_set['json_dict_rep']['name']) + for student in test_full_class_data_set['json_dict_rep']['students']: + test_full_new_class.add_student(Student(**student)) + + test_full_new_class.json_str_rep = test_full_class_data_set['json_str_rep'] + test_full_new_class.json_dict_rep = test_full_class_data_set['json_dict_rep'] + + return test_full_new_class + + @pytest.mark.parametrize( 'class_name, class_list_arg, list_of_students', [('slightly silly', ['silly', 'sillier', 'silliest'], ['silly', 'sillier', 'silliest']), From d72e5f13dd95978df0b0443992e059f2c5463727 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Sat, 28 Mar 2020 22:00:43 -0500 Subject: [PATCH 36/44] Change NewClass to use `path_safe_name`, not `name` to create temp_dir. This fixes bug where name with disallowed characters would be used as a directory name. Also added test ensuring path_safe_name is used. Signed-off-by: toonarmycaptain --- dionysus_app/class_.py | 13 +++++++------ test_suite/test_class.py | 10 ++++++++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/dionysus_app/class_.py b/dionysus_app/class_.py index 4a375b2b..53523338 100644 --- a/dionysus_app/class_.py +++ b/dionysus_app/class_.py @@ -250,12 +250,13 @@ class NewClass(Class): This temp directory is garbage collected with the object upon object destruction. - Temp directory is prefixed with the class' name, plus a hash, and contains - a subdirectory named 'avatars' to hold avatars before writing to database: + Temp directory is prefixed with the class' path_safe_name, plus a hash, and + contains a subdirectory named 'avatars' to hold avatars before writing to + database: - TEMP_DIR - ├── class_name+hash - | ├── avatars + TEMP_DIR + ├── class_path_safe_name+hash + | ├── avatars NB: In future may be subclassed or housed in Database objects to allow database specific implementation. @@ -276,7 +277,7 @@ def __init__(self, name: str, students: List[Student] = None): # Create class temp directory. Path.mkdir(TEMP_DIR, exist_ok=True, parents=True) # Ensure path exists. - self.temp_dir: Path = Path(tempfile.mkdtemp(prefix=self.name, dir=TEMP_DIR)) + self.temp_dir: Path = Path(tempfile.mkdtemp(prefix=self._path_safe_name, dir=TEMP_DIR)) # Create avatars directory within class temp directory. self.temp_avatars_dir: Path = self.temp_dir.joinpath('avatars') Path.mkdir(self.temp_avatars_dir) diff --git a/test_suite/test_class.py b/test_suite/test_class.py index 55d2339c..3fccbc99 100644 --- a/test_suite/test_class.py +++ b/test_suite/test_class.py @@ -423,3 +423,13 @@ def test_new_class_temp_dir_deleted_on_deletion(self, monkeypatch, tmpdir): del test_class # NB May throw an (ignored) Exception because the class is garbage collected before this line. # No class temp dir in test_temp_dir: assert not os.listdir(test_temp_dir) + + def test_new_class_uses_path_safe_name(self): + # Ensure class name has disallowed characters - validate test. + test_new_class = NewClass("S|r Røbin's ß@boon$") + assert test_new_class.name != test_new_class.path_safe_name + + # Ensure class_name_with_disallowed chars in temp dir path. + assert test_new_class.name not in str(test_new_class.temp_dir) + # Ensure path_safe_name is in temp dir path. + assert test_new_class.path_safe_name in str(test_new_class.temp_dir) From 9faeab1080df79b5f127a34bf6cb2d770699eb91 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Sat, 28 Mar 2020 22:59:38 -0500 Subject: [PATCH 37/44] Change `new_chart`/`assemble_chart_data` to take `Class`/`NewClass` obj. Change `new_chart`/`assemble_chart_data` to take `Class`/`NewClass` object instead of a class name. - Supports passing in class directly from `create_class` without reloading from database. - Move logic prompting user to choose a class from `assemble_chart_data` to `new_chart`. Signed-off-by: toonarmycaptain --- CHANGELOG.md | 9 ++- dionysus_app/chart_generator/create_chart.py | 37 +++++----- dionysus_app/class_functions.py | 10 +-- .../test_chart_generator/test_create_chart.py | 68 +++++++++---------- test_suite/test_class_functions.py | 2 +- 5 files changed, 64 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3375bd1c..cb571f73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,13 +6,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added -- `temp` directory created in `APP_DATA` by data_folder_check() on app start and removed on app exit (if it contains files). +- Implemented `temp` directory created in `APP_DATA` by data_folder_check() on app start and removed on app exit (if it contains files). - `NewClass` subclass of `Class` using `temp` directory to hold files before writing to database. - - Initially holds avatars. + - Initially holds avatars as user enters during class creation. ### Changed - Separate UI/logic/persistence concerns in `create_classlist`. - - New functions `move_avatars_to_class_data`, `move_avatar_to_class_data` utilise `NewClass` to move all of the avatars from temp to database. + - New functions `move_avatars_to_class_data`, `move_avatar_to_class_data` utilise `NewClass` to move avatars from `temp` to database. - `create_classlist_data` takes a `NewClass` object. +- Change `new_chart`/`assemble_chart_data` to take `Class`/`NewClass` object instead of a class name. + - Supports passing in class directly from `create_class` without reloading from database. + - Move logic prompting user to choose a class from `assemble_chart_data` to `new_chart`. - More tests converted to Pytest style tests. ## [0.5.0-alpha] - 2020-01-23 diff --git a/dionysus_app/chart_generator/create_chart.py b/dionysus_app/chart_generator/create_chart.py index 9bd212b9..0f9ab19b 100644 --- a/dionysus_app/chart_generator/create_chart.py +++ b/dionysus_app/chart_generator/create_chart.py @@ -14,6 +14,7 @@ from dionysus_app.chart_generator.generate_image import generate_chart_image from dionysus_app.chart_generator.process_chart_data import DEFAULT_CHART_PARAMS +from dionysus_app.class_ import Class from dionysus_app.class_functions import select_classlist, load_class_from_disk from dionysus_app.data_folder import CHART_DATA_FILE_TYPE, DataFolder from dionysus_app.file_functions import convert_to_json, copy_file @@ -29,10 +30,11 @@ CLASSLIST_DATA_PATH = DataFolder.generate_rel_path(DataFolder.CLASS_DATA.value) -def new_chart(class_name: str = None): +def new_chart(loaded_class: Class = None): """ - Take class name selection, chart name, score data, chart parameters from - assemble_chart_data, form into chart_data_dict with key-value format: + Prompts user to select class if not provided by caller. + Take chart name, score data, chart parameters from assemble_chart_data, + form into chart_data_dict with key-value format: chart_data_dict = { 'class_name': class_name, # str 'chart_name': chart_name, # str @@ -43,14 +45,16 @@ def new_chart(class_name: str = None): Then write this data to disk as *.cdf (ChartDataFile), generate and save the chart. - NB Skips class name selection if class_name provided. - - :param class_name: str = None + :param loaded_class: Class = None :return: None """ - class_name, chart_name, chart_default_filename, student_scores, chart_params = assemble_chart_data(class_name) + if not loaded_class: + class_name = select_classlist() # TODO: warn for empty classlist + loaded_class = load_class_from_disk(class_name) + + chart_name, chart_default_filename, student_scores, chart_params = assemble_chart_data(loaded_class) - chart_data_dict = {'class_name': class_name, # str + chart_data_dict = {'class_name': loaded_class.name, # str 'chart_name': chart_name, # str 'chart_default_filename': chart_default_filename, # str 'chart_params': chart_params, # dict @@ -66,7 +70,7 @@ def new_chart(class_name: str = None): user_save_chart_image(chart_data_dict, chart_image_location) -def assemble_chart_data(class_name: str = None): +def assemble_chart_data(loaded_class: Class): """ Collect data/user input for new chart. @@ -74,20 +78,15 @@ def assemble_chart_data(class_name: str = None): take chart data from user. Return values for chart_data_dict assembly: - class_name: str + loaded_class: Class chart_name: str chart_filename: str student_scores: dict chart_params: dict - :param class_name: str = None - :return: tuple(str, str, str, dict, dict) + :param loaded_class: str = None + :return: tuple(str, str, dict, dict) """ - if not class_name: - class_name = select_classlist() # TODO: warn for empty classlist - - loaded_class = load_class_from_disk(class_name) - student_scores: dict = take_score_data(loaded_class) chart_name = take_chart_name() @@ -97,7 +96,7 @@ def assemble_chart_data(class_name: str = None): chart_params = set_chart_params() # chart options here or before score entry, setting chart params, min, max scores etc - return class_name, chart_name, chart_filename, student_scores, chart_params + return chart_name, chart_filename, student_scores, chart_params def write_chart_data_to_file(chart_data_dict: dict): @@ -134,7 +133,7 @@ def write_chart_data_to_file(chart_data_dict: dict): chart_filename = file_chart_data_dict['chart_default_filename'] chart_data_file = chart_filename + CHART_DATA_FILE_TYPE chart_data_filepath = CLASSLIST_DATA_PATH.joinpath( - file_chart_data_dict['class_name'], 'chart_data', chart_data_file) + file_chart_data_dict['class_name'], 'chart_data', chart_data_file) # Convert data_dict to JSON-safe form. json_safe_chart_data_dict = sanitise_avatar_path_objects(file_chart_data_dict) diff --git a/dionysus_app/class_functions.py b/dionysus_app/class_functions.py index de8c54cb..f7abd216 100644 --- a/dionysus_app/class_functions.py +++ b/dionysus_app/class_functions.py @@ -43,12 +43,12 @@ def create_classlist() -> None: """ classlist_name = take_classlist_name_input() # TODO: Option to cancel creation at class name entry stage - new_class = compose_classlist_dialogue(classlist_name) + new_class: NewClass = compose_classlist_dialogue(classlist_name) create_classlist_data(new_class) # Future: Call to Database.create_class(new_class) time.sleep(2) # Pause for user to look over feedback. - create_chart_with_new_class(classlist_name) + create_chart_with_new_class(new_class) def setup_class(classlist_name: str) -> None: @@ -243,17 +243,17 @@ def write_classlist_to_file(current_class: Class) -> None: classlist_file.write(json_class_data) -def create_chart_with_new_class(classlist_name: str) -> None: +def create_chart_with_new_class(new_class: NewClass) -> None: """ Prompt to create a chart with a newly created class, call new_chart with newly created class if user desires. - :param classlist_name: str + :param new_class: NewClass :return: None """ if create_chart_with_new_class_dialogue(): from dionysus_app.chart_generator.create_chart import new_chart - new_chart(classlist_name) + new_chart(new_class) def select_classlist() -> str: diff --git a/test_suite/test_chart_generator/test_create_chart.py b/test_suite/test_chart_generator/test_create_chart.py index f49a286a..ab0dc6a7 100644 --- a/test_suite/test_chart_generator/test_create_chart.py +++ b/test_suite/test_chart_generator/test_create_chart.py @@ -3,7 +3,7 @@ import pytest import definitions -from definitions import DEFAULT_CHART_SAVE_FOLDER + from dionysus_app.chart_generator import create_chart from dionysus_app.chart_generator.create_chart import (assemble_chart_data, copy_image_to_user_save_loc, @@ -19,25 +19,29 @@ user_save_chart_image, ) from dionysus_app.chart_generator.process_chart_data import DEFAULT_CHART_PARAMS -from dionysus_app.class_ import Class +from dionysus_app.class_ import Class, NewClass from dionysus_app.data_folder import CHART_DATA_FILE_TYPE +from test_suite.test_class import test_full_class from test_suite.testing_class_data import test_full_class_data_set class TestNewChart: - @pytest.mark.parametrize('classname_from_create_class', + @pytest.mark.parametrize('class_from_create_class', [None, - 'test_class_name', # Pass in test_class_name. + Class.from_dict(test_full_class_data_set['json_dict_rep']), # Pass in test_class + NewClass.from_dict(test_full_class_data_set['json_dict_rep']) # NewClass obj ]) - def test_new_chart(self, monkeypatch, classname_from_create_class): - test_class_name = 'test_class_name' + def test_new_chart(self, monkeypatch, class_from_create_class, test_full_class): + # Test class either passed, or instantiated if None is passed. + test_class = class_from_create_class or test_full_class + test_chart_name = 'test_chart_name' test_chart_default_filename = 'test_chart_default_filename' test_chart_params = {'test_chart_params': 'some chart params'} test_score_avatar_dict = {'test_student_scores': 'test student avatars'} - test_chart_data_dict = {'class_name': test_class_name, + test_chart_data_dict = {'class_name': test_class.name, 'chart_name': test_chart_name, 'chart_default_filename': test_chart_default_filename, 'chart_params': test_chart_params, @@ -46,11 +50,18 @@ def test_new_chart(self, monkeypatch, classname_from_create_class): test_chart_image_location = 'some image location' - def mocked_assemble_chart_data(class_name): - if class_name is not classname_from_create_class: - raise ValueError('None or a classname should be passed in here.') - return (test_class_name, - test_chart_name, + def mocked_select_classlist(): + if class_from_create_class: + raise ValueError('Function should not be called if class is passed.') + return test_class.name + + def mocked_load_class_from_disk(class_name): + if class_from_create_class: + raise ValueError('Function should not be called if class is passed.') + return test_class + + def mocked_assemble_chart_data(class_obj): + return (test_chart_name, test_chart_default_filename, test_score_avatar_dict, test_chart_params) @@ -74,40 +85,30 @@ def mocked_user_save_chart_image(chart_data_dict, chart_image_location): if chart_image_location != test_chart_image_location: raise ValueError + monkeypatch.setattr(create_chart, 'select_classlist', mocked_select_classlist) + monkeypatch.setattr(create_chart, 'load_class_from_disk', mocked_load_class_from_disk) monkeypatch.setattr(create_chart, 'assemble_chart_data', mocked_assemble_chart_data) monkeypatch.setattr(create_chart, 'write_chart_data_to_file', mocked_write_chart_data_to_file) monkeypatch.setattr(create_chart, 'generate_chart_image', mocked_generate_chart_image) monkeypatch.setattr(create_chart, 'show_image', mocked_show_image) monkeypatch.setattr(create_chart, 'user_save_chart_image', mocked_user_save_chart_image) - assert new_chart(classname_from_create_class) is None + assert new_chart(class_from_create_class) is None class TestAssembleChartData: - @pytest.mark.parametrize('classname_from_create_class', - [None, - test_full_class_data_set['json_dict_rep']['name'], # Pass in name from test class. + @pytest.mark.parametrize('class_from_create_class', + [Class.from_dict(test_full_class_data_set['json_dict_rep']), # Pass in test_class + NewClass.from_dict(test_full_class_data_set['json_dict_rep']) # NewClass obj ]) - def test_assemble_chart_data(self, monkeypatch, classname_from_create_class): - test_class = Class.from_dict(test_full_class_data_set['json_dict_rep']) - test_class_name = test_class.name + def test_assemble_chart_data(self, monkeypatch, class_from_create_class): test_chart_name = 'my_chart' test_chart_filename = test_chart_name mock_score_avatar_dict = {'scores': 'list of avatars'} mock_chart_params = {'some': 'chart_params'} - def mocked_select_classlist(): - if classname_from_create_class: - raise ValueError('Select classlist called despite class already given.') - return test_class_name - - def mocked_load_class_from_disk(class_name): - if class_name != test_class_name: - raise ValueError - return test_class - def mocked_take_score_data(class_obj): - if class_obj is not test_class: + if class_obj is not class_from_create_class: raise ValueError return mock_score_avatar_dict @@ -122,15 +123,13 @@ def mocked_clean_for_filename(chart_name): def mocked_set_chart_params(): return mock_chart_params - monkeypatch.setattr(create_chart, 'select_classlist', mocked_select_classlist) - monkeypatch.setattr(create_chart, 'load_class_from_disk', mocked_load_class_from_disk) monkeypatch.setattr(create_chart, 'take_score_data', mocked_take_score_data) monkeypatch.setattr(create_chart, 'take_chart_name', mocked_take_chart_name) monkeypatch.setattr(create_chart, 'clean_for_filename', mocked_clean_for_filename) monkeypatch.setattr(create_chart, 'set_chart_params', mocked_set_chart_params) - assert assemble_chart_data(classname_from_create_class) == ( - test_class_name, test_chart_name, test_chart_filename, mock_score_avatar_dict, mock_chart_params) + assert assemble_chart_data(class_from_create_class) == ( + test_chart_name, test_chart_filename, mock_score_avatar_dict, mock_chart_params) class TestWriteChartDataToFile: @@ -348,6 +347,7 @@ def test_get_class_save_folder_path(monkeypatch): with pytest.raises(ValueError): get_class_save_folder_path('some_class') + class TestShowImage: def test_show_image(self, monkeypatch): test_image_location = Path('my/test/image/path') diff --git a/test_suite/test_class_functions.py b/test_suite/test_class_functions.py index 0a7a7570..db92b40c 100644 --- a/test_suite/test_class_functions.py +++ b/test_suite/test_class_functions.py @@ -60,7 +60,7 @@ def mocked_time_sleep(seconds): pass def mocked_create_chart_with_new_class(test_class_name): - if test_class_name != test_full_class.name: + if test_class_name != test_full_class: raise ValueError monkeypatch.setattr(class_functions, 'take_classlist_name_input', mocked_take_classlist_name_input) From 921bc319db53c317e29386179dc658c53284c99e Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Mon, 30 Mar 2020 00:39:39 -0500 Subject: [PATCH 38/44] Improve docstring for cache_class_registry. Signed-off-by: toonarmycaptain --- dionysus_app/class_registry_functions.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/dionysus_app/class_registry_functions.py b/dionysus_app/class_registry_functions.py index 5c65d8b7..6f24606f 100644 --- a/dionysus_app/class_registry_functions.py +++ b/dionysus_app/class_registry_functions.py @@ -13,10 +13,12 @@ def cache_class_registry() -> List[str]: """ - Initialises CLASS_REGISTRY global variable and writes registry to - disk. + Generate registry from class files on disk, then write to disk, returning + freshly generated registry. - :return: list + Typically used on app start to get value for CLASS_REGISTRY global variable + + :return: List[str] """ registry = generate_registry_from_filesystem() From 52080a6ba6b5fa6a2bfa198811f23ed3e9d832cb Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Mon, 30 Mar 2020 00:40:37 -0500 Subject: [PATCH 39/44] Write tests for class_registry_functions.py. Signed-off-by: toonarmycaptain --- test_suite/test_class_registry_functions.py | 178 +++++++++++--------- 1 file changed, 103 insertions(+), 75 deletions(-) diff --git a/test_suite/test_class_registry_functions.py b/test_suite/test_class_registry_functions.py index a06a91b7..0a893f61 100644 --- a/test_suite/test_class_registry_functions.py +++ b/test_suite/test_class_registry_functions.py @@ -1,81 +1,78 @@ -import unittest +from pathlib import Path import pytest -import dionysus_app.class_registry_functions as class_registry_functions +import definitions -from dionysus_app.class_registry_functions import (register_class, +from dionysus_app import class_registry_functions +from dionysus_app.class_registry_functions import (cache_class_registry, check_registry_on_exit, classlist_exists, + register_class, ) -class TestLoadNonexistentRegistryWithNoClassData(unittest.TestCase): - pass - # setup: - # dummy CLASS_REGISTRY_PATH? - # - # confirm blank registry created on startup when no data exists - # confirm empty cached registry dict/global - - -class TestLoadExistingRegistry(unittest.TestCase): - pass - # setup: - # dummy CLASS_REGISTRY_PATH? - # dummy class data (existent classes) - # - # confirm registry file created on startup - # confirm registry dict/global created on startup - # confirm created registry file has correct data - # confirm created registry dict/global has correct data - - -class TestLoadNonexistentRegistryWithClassData: - pass - # setup: - # dummy CLASS_REGISTRY_PATH? - # dummy class data (existent classes) - # - # confirm registry file created on startup - # confirm registry dict/global created on startup - # confirm created registry file has correct data - - -class TestAddClassToNewRegistry(unittest.TestCase): - pass - # setup: - # dummy CLASS_REGISTRY_PATH? - # dummy classlist_name - # - # test add class to registry - # confirm registry created - # confirm classlist_name in cached registry dict - # confirm classlist_name in registry file - - -class TestAddClassExistingRegistry(unittest.TestCase): - pass - # setup: - # dummy CLASS_REGISTRY_PATH? - # dummy existing class registry - # dummy classlist_name - # - # test add class to registry - # confirm classlist_name in cached registry dict - # confirm classlist_name in registry file +class TestCacheClassRegistry: + def test_cache_class_registry(self, monkeypatch): + mock_registry_object = 'mock_registry_object' + + def mocked_generate_registry_from_filesystem(): + return mock_registry_object + + def mocked_write_registry_to_disk(registry): + if registry != mock_registry_object: + raise ValueError + + monkeypatch.setattr(class_registry_functions, 'generate_registry_from_filesystem', + mocked_generate_registry_from_filesystem) + monkeypatch.setattr(class_registry_functions, 'write_registry_to_disk', mocked_write_registry_to_disk) + + assert cache_class_registry() == mock_registry_object + + def test_cache_class_registry_no_classes(self, monkeypatch, tmpdir): + monkeypatch.setattr(class_registry_functions, 'CLASS_REGISTRY_PATH', Path(tmpdir, 'class_registry.index')) + monkeypatch.setattr(class_registry_functions, 'CLASSLIST_DATA_PATH', Path(tmpdir, 'test_APP_DATA/CLASS_DATA')) + # No class registry. + assert not class_registry_functions.CLASS_REGISTRY_PATH.exists() + + assert cache_class_registry() == list() # As opposed to None, this compares to empty list. + assert class_registry_functions.CLASS_REGISTRY_PATH.exists() # class_registry file is created. + assert open(class_registry_functions.CLASS_REGISTRY_PATH, 'r').readlines() == list() # Registry is empty list. + + def test_cache_class_registry_some_classes(self, monkeypatch, tmpdir): + """This also tests existing registry where no classes exist, as """ + mock_registry_object = ['mock_registry_object', 'contains two class names'] + + def mocked_generate_registry_from_filesystem(): + return mock_registry_object + + # def mocked_write_registry_to_disk(registry): + # if registry != mock_registry_object: + # raise ValueError + + monkeypatch.setattr(class_registry_functions, 'CLASS_REGISTRY_PATH', Path(tmpdir, 'class_registry.index')) + monkeypatch.setattr(class_registry_functions, 'CLASSLIST_DATA_PATH', Path(tmpdir, 'test_APP_DATA/CLASS_DATA')) + monkeypatch.setattr(class_registry_functions, 'generate_registry_from_filesystem', + mocked_generate_registry_from_filesystem) + # monkeypatch.setattr(class_registry_functions, 'write_registry_to_disk', mocked_write_registry_to_disk) + + assert cache_class_registry() == mock_registry_object # As opposed to None, this compares to empty list. + assert class_registry_functions.CLASS_REGISTRY_PATH.exists() # class_registry file is created. + # Number of classes is same as mock object. + assert len(open(class_registry_functions.CLASS_REGISTRY_PATH, 'r').readlines()) == len(mock_registry_object) + class TestRegisterClass: - # @pytest.mark.parametrize('mock_registry', - # [(['a class', 'some_class', 'some_other_class']), - # (['a class', 'some_other_class']), - # ([]), - # pytest.param(None, marks=pytest.mark.xfail), - # ]) - # def test_register_class(mock_registry, - # monkeypatch): - # pass + def test_register_class(self, monkeypatch, tmpdir): + test_class_name = 'Arthur_s Knights' + monkeypatch.setattr(class_registry_functions, 'CLASS_REGISTRY_PATH', Path(tmpdir, 'class_registry.index')) + monkeypatch.setattr(definitions, 'REGISTRY', []) + assert register_class(test_class_name) is None + # Class name in registry file: + assert f'{test_class_name}\n' in open(class_registry_functions.CLASS_REGISTRY_PATH).readlines() + # Class name in cached registry: + assert test_class_name in definitions.REGISTRY def test_register_class_raising_error_uninitialised_registry(self, monkeypatch): monkeypatch.setattr(class_registry_functions.definitions, 'REGISTRY', None) @@ -103,16 +100,47 @@ def test_classlist_exists_uninitialised_registry_raising_error(self, monkeypatch class TestCheckRegistryOnExit: - # @pytest.mark.parametrize('mock_registry', - # [(['a class', 'some_class', 'some_other_class']), - # (['a class', 'some_other_class']), - # ([]), - # pytest.param(None, marks=pytest.mark.xfail), - # ]) - # def test_check_registry_on_exit(mock_registry, - # monkeypatch): - # pass + @pytest.mark.parametrize('mock_registry', + [(['a class', 'some_class', 'some_other_class']), + (['a class', 'some_other_class']), + ([]), + pytest.param(None, marks=pytest.mark.xfail), + ]) + def test_check_registry_on_exit_writing_registry(self, monkeypatch, tmpdir, + mock_registry, + ): + mocked_class_registry_path = Path(tmpdir, 'mocked_registry') + + def mocked_write_registry_to_disk(registry): + if registry != mock_registry: + raise ValueError("Registry written incorrectly.") + + monkeypatch.setattr(class_registry_functions, 'write_registry_to_disk', mocked_write_registry_to_disk) + monkeypatch.setattr(class_registry_functions.definitions, 'REGISTRY', mock_registry) + monkeypatch.setattr(class_registry_functions, 'CLASS_REGISTRY_PATH', mocked_class_registry_path) + + assert check_registry_on_exit() + + @pytest.mark.parametrize('mock_registry', + [(['a class', 'some_class', 'some_other_class']), + (['a class', 'some_other_class']), + ([]), + pytest.param(None, marks=pytest.mark.xfail), + ]) + def test_check_registry_on_exit_writing_registry(self, monkeypatch, tmpdir, + mock_registry, + ): + mocked_class_registry_path = Path(tmpdir, 'mock_registry') + + with open(mocked_class_registry_path, 'w') as mock_registry_file: + mock_registry_file.write('definitely not a correct registry!') + + monkeypatch.setattr(class_registry_functions.definitions, 'REGISTRY', mock_registry) + monkeypatch.setattr(class_registry_functions, 'CLASS_REGISTRY_PATH', mocked_class_registry_path) + + assert check_registry_on_exit() is None + assert open(mocked_class_registry_path).readlines() == [f'{classname}\n' for classname in mock_registry] def test_check_registry_on_exit_raising_error_uninitialised_registry(self, monkeypatch): monkeypatch.setattr(class_registry_functions.definitions, 'REGISTRY', None) From 6b7eb4e4058b95c80dd7276e83587279c1f217a5 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Mon, 30 Mar 2020 22:23:23 -0500 Subject: [PATCH 40/44] Correct docstring - returns None. Signed-off-by: toonarmycaptain --- dionysus_app/class_functions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dionysus_app/class_functions.py b/dionysus_app/class_functions.py index f7abd216..ef138cf3 100644 --- a/dionysus_app/class_functions.py +++ b/dionysus_app/class_functions.py @@ -228,7 +228,7 @@ def write_classlist_to_file(current_class: Class) -> None: keep them as strings when loading. :param current_class: Class object - :return: Path + :return: None """ class_name = current_class.name data_filename = class_name + CLASSLIST_DATA_FILE_TYPE From 0d73a4518eaff77c5fce6fe900ce9dabceca2c3b Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Tue, 31 Mar 2020 21:31:49 -0500 Subject: [PATCH 41/44] Update and clarify CHANGELOG.md. Signed-off-by: toonarmycaptain --- CHANGELOG.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb571f73..0993d9f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,17 +6,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added -- Implemented `temp` directory created in `APP_DATA` by data_folder_check() on app start and removed on app exit (if it contains files). +- Implemented `temp` directory created in `APP_DATA` by `data_folder_check` on app start and removed on app exit (if it contains files). - `NewClass` subclass of `Class` using `temp` directory to hold files before writing to database. - Initially holds avatars as user enters during class creation. +- Add [AllContributors](https://allcontributors.org/) badge to `README.md`, recognising project contributors, `.all-contributorsrc` with contributer data. ### Changed - Separate UI/logic/persistence concerns in `create_classlist`. - New functions `move_avatars_to_class_data`, `move_avatar_to_class_data` utilise `NewClass` to move avatars from `temp` to database. + - `write_classlist_to_file` now returns `None` instead of the path written to. Future persistence layer may be a Database rather than a path. - `create_classlist_data` takes a `NewClass` object. -- Change `new_chart`/`assemble_chart_data` to take `Class`/`NewClass` object instead of a class name. - - Supports passing in class directly from `create_class` without reloading from database. +- Refactor `new_chart`/`assemble_chart_data` to take `Class`/`NewClass` object instead of a class name. + - Supports passing in class directly from `create_class` without reloading class from database. - Move logic prompting user to choose a class from `assemble_chart_data` to `new_chart`. -- More tests converted to Pytest style tests. +- More tests converted to Pytest style tests, test coverage improved (eg `class_registry_functions.py`). +- Improved type hinting, extend passing/use of `Path`s rather than strings/`os/path` (eg in `UI_functions.py`, `initialise_app.py`). ## [0.5.0-alpha] - 2020-01-23 ### Added From e4dcf95d310fd354ecbd58dbc31617c6b025756f Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Tue, 31 Mar 2020 21:51:53 -0500 Subject: [PATCH 42/44] Iterate version to 0.6.0-alpha Signed-off-by: toonarmycaptain --- CHANGELOG.md | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0993d9f8..49c869ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [0.6.0-alpha] - 2020-03-31 ### Added - Implemented `temp` directory created in `APP_DATA` by `data_folder_check` on app start and removed on app exit (if it contains files). - `NewClass` subclass of `Class` using `temp` directory to hold files before writing to database. diff --git a/setup.py b/setup.py index 181321d4..57bcdf4c 100644 --- a/setup.py +++ b/setup.py @@ -1,7 +1,7 @@ from setuptools import setup, find_packages setup(name='dionysus_app', - version='0.5.0-alpha', + version='0.6.0-alpha', description='Avatar chart generator', author='David Antonini', author_email='toonarmycaptain@hotmail.com', From b9b9d19a3f30a7c71fa8179349425d300b83cf9e Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Tue, 31 Mar 2020 22:36:28 -0500 Subject: [PATCH 43/44] Remove superfluous whitespace. Signed-off-by: toonarmycaptain --- data_version_conversion.py | 1 - 1 file changed, 1 deletion(-) diff --git a/data_version_conversion.py b/data_version_conversion.py index 05dda5ae..1e207319 100644 --- a/data_version_conversion.py +++ b/data_version_conversion.py @@ -117,7 +117,6 @@ def transform_old_cld_file(filepath: Path): new_filename = class_name + CLASSLIST_DATA_FILE_TYPE new_class_data_path_name = CLASSLIST_DATA_PATH.joinpath(class_name, new_filename) - print(f'Transformed {new_class.name} data file ' f'to new data format in {new_class_data_path_name}') From 83c1000b1258f32aaf531e475a02ede2ddac2145 Mon Sep 17 00:00:00 2001 From: toonarmycaptain Date: Wed, 1 Apr 2020 08:52:11 -0500 Subject: [PATCH 44/44] Correct/iterate date. Signed-off-by: toonarmycaptain --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49c869ba..a4380410 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [0.6.0-alpha] - 2020-03-31 +## [0.6.0-alpha] - 2020-04-01 ### Added - Implemented `temp` directory created in `APP_DATA` by `data_folder_check` on app start and removed on app exit (if it contains files). - `NewClass` subclass of `Class` using `temp` directory to hold files before writing to database.