From 72843c06f9c7bbebd9c50bd4169d7a9caf487d53 Mon Sep 17 00:00:00 2001 From: "alex.soh" Date: Tue, 23 Nov 2021 16:25:09 +0800 Subject: [PATCH] refactor: add code coverage, cover CLI commands with tests --- .coveragerc | 42 ++++++++ .gitignore | 4 + Makefile | 19 +++- bin/main.py | 6 +- docs/conf.py | 27 ++--- requirements/dev.in | 1 + requirements/dev.txt | 2 + setup.py | 13 +-- tests/commands/__init__.py | 0 tests/commands/test_cli.py | 26 +++++ tests/commands/test_config.py | 116 ++++++++++++++++++++++ tests/commands/test_context.py | 18 ++++ tests/commands/test_dev.py | 20 ++++ tests/commands/test_images.py | 176 +++++++++++++++++++++++++++++++++ tests/commands/test_k8s.py | 13 +++ tests/commands/test_local.py | 25 +++++ tests/commands/test_plugins.py | 64 ++++++++++++ tests/helpers.py | 45 +++++++++ tests/test_config.py | 37 +++++-- tests/test_env.py | 14 ++- tests/test_images.py | 1 + tests/test_jobs.py | 91 +++++++++++++++++ tests/test_plugins.py | 12 +-- tests/test_utils.py | 117 +++++++++++++++++++++- tutor/commands/cli.py | 14 +-- tutor/commands/compose.py | 4 +- tutor/commands/config.py | 4 +- tutor/commands/context.py | 2 +- tutor/commands/images.py | 10 +- tutor/commands/k8s.py | 6 +- tutor/commands/local.py | 4 +- tutor/commands/plugins.py | 6 +- tutor/config.py | 17 ++-- tutor/env.py | 3 +- tutor/plugins.py | 2 +- tutor/utils.py | 12 +-- 36 files changed, 881 insertions(+), 92 deletions(-) create mode 100644 .coveragerc create mode 100644 tests/commands/__init__.py create mode 100644 tests/commands/test_cli.py create mode 100644 tests/commands/test_config.py create mode 100644 tests/commands/test_context.py create mode 100644 tests/commands/test_dev.py create mode 100644 tests/commands/test_images.py create mode 100644 tests/commands/test_k8s.py create mode 100644 tests/commands/test_local.py create mode 100644 tests/commands/test_plugins.py create mode 100644 tests/helpers.py create mode 100644 tests/test_jobs.py diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 0000000..606d5fa --- /dev/null +++ b/.coveragerc @@ -0,0 +1,42 @@ +# .coveragerc to control coverage.py +[run] +branch = True +source = + ./tutor + ./bin +omit = + */templates/* + +[report] +# Regexes for lines to exclude from consideration +exclude_lines = + # Have to re-enable the standard pragma + pragma: no cover + + # Don't complain about missing debug-only code: + def __repr__ + if self\.debug + + # Don't complain if tests don't hit defensive assertion code: + raise AssertionError + raise NotImplementedError + + # Don't complain if non-runnable code isn't run: + if 0: + if __name__ == .__main__.: + + # Don't complain about abstract methods, they aren't run: + @(abc\.)?abstractmethod + +ignore_errors = True +show_missing = True +skip_empty = True +precision = 2 + +[html] +skip_empty = True +show_contexts = True + +[json] +pretty_print = True +show_contexts = True \ No newline at end of file diff --git a/.gitignore b/.gitignore index 8e55cc4..0b40c3b 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,7 @@ __pycache__ /build/ /dist/ /release_description.md + +# Unit test/ coverage reports +.coverage +/htmlcov/ diff --git a/Makefile b/Makefile index a7268ba..4340d00 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ .DEFAULT_GOAL := help .PHONY: docs -SRC_DIRS = ./tutor ./tests ./bin +SRC_DIRS = ./tutor ./tests ./bin ./docs BLACK_OPTS = --exclude templates ${SRC_DIRS} ###### Development @@ -53,6 +53,23 @@ bootstrap-dev: ## Install dev requirements bootstrap-dev-plugins: bootstrap-dev ## Install dev requirement and all supported plugins pip install -r requirements/plugins.txt +###### Code coverage + +coverage: ## Run unit-tests before analyzing code coverage and generate report + $(MAKE) --keep-going coverage-tests coverage-report + +coverage-tests: ## Run unit-tests and analyze code coverage + coverage run -m unittest discover + +coverage-report: ## Generate CLI report for the code coverage + coverage report + +coverage-html: coverage-report ## Generate HTML report for the code coverage + coverage html + +coverage-browse-report: coverage-html ## Open the HTML report in the browser + sensible-browser htmlcov/index.html + ###### Deployment bundle: ## Bundle the tutor package in a single "dist/tutor" executable diff --git a/bin/main.py b/bin/main.py index 20bc050..0055f90 100755 --- a/bin/main.py +++ b/bin/main.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 from tutor.plugins import OfficialPlugin +from tutor.commands.cli import main # Manually install plugins (this is for creating the bundle) for plugin_name in [ @@ -20,6 +21,5 @@ for plugin_name in [ except ImportError: pass -from tutor.commands.cli import main - -main() +if __name__ == "__main__": + main() diff --git a/docs/conf.py b/docs/conf.py index b9c2f8a..f1334a3 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -1,6 +1,7 @@ import io import os import sys +from typing import Any, Dict, List import docutils import docutils.parsers.rst @@ -28,7 +29,7 @@ pygments_style = None # -- Sphinx-Click configuration # https://sphinx-click.readthedocs.io/ -extensions.append('sphinx_click') +extensions.append("sphinx_click") # This is to avoid the addition of the local username to the docs os.environ["HOME"] = "~" # Make sure that sphinx-click can find the tutor module @@ -63,7 +64,7 @@ html_show_copyright = False # Custom variables here = os.path.abspath(os.path.dirname(__file__)) -about = {} +about: Dict[str, str] = {} with io.open( os.path.join(here, "..", "tutor", "__about__.py"), "rt", encoding="utf-8" ) as f: @@ -77,17 +78,17 @@ rst_prolog = """ # Custom directives def youtube( - _name, - _args, - _options, - content, - _lineno, - _contentOffset, - _blockText, - _state, - _stateMachine, -): - """ Restructured text extension for inserting youtube embedded videos """ + _name: Any, + _args: Any, + _options: Any, + content: List[str], + _lineno: Any, + _contentOffset: Any, + _blockText: Any, + _state: Any, + _stateMachine: Any, +) -> Any: + """Restructured text extension for inserting youtube embedded videos""" if not content: return [] video_id = content[0] diff --git a/requirements/dev.in b/requirements/dev.in index dbd6ecf..b126909 100644 --- a/requirements/dev.in +++ b/requirements/dev.in @@ -4,6 +4,7 @@ pip-tools pylint pyinstaller twine +coverage # Types packages types-PyYAML diff --git a/requirements/dev.txt b/requirements/dev.txt index eee2061..6cd8fd1 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -36,6 +36,8 @@ click==8.0.3 # pip-tools colorama==0.4.4 # via twine +coverage==6.2 + # via -r requirements/dev.in cryptography==35.0.0 # via secretstorage docutils==0.17.1 diff --git a/setup.py b/setup.py index 8d04d5e..f031401 100644 --- a/setup.py +++ b/setup.py @@ -1,11 +1,12 @@ import io import os from setuptools import find_packages, setup +from typing import Dict, List HERE = os.path.abspath(os.path.dirname(__file__)) -def load_readme(): +def load_readme() -> str: with io.open(os.path.join(HERE, "README.rst"), "rt", encoding="utf8") as f: readme = f.read() # Replace img src for publication on pypi @@ -14,8 +15,8 @@ def load_readme(): ) -def load_about(): - about = {} +def load_about() -> Dict[str, str]: + about: Dict[str, str] = {} with io.open( os.path.join(HERE, "tutor", "__about__.py"), "rt", encoding="utf-8" ) as f: @@ -23,14 +24,13 @@ def load_about(): return about -def load_requirements(filename: str): +def load_requirements(filename: str) -> List[str]: with io.open( os.path.join(HERE, "requirements", filename), "rt", encoding="utf-8" ) as f: return [line.strip() for line in f if is_requirement(line)] - -def is_requirement(line): +def is_requirement(line: str) -> bool: return not (line.strip() == "" or line.startswith("#")) @@ -72,4 +72,5 @@ setup( "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", ], + test_suite="tests", ) diff --git a/tests/commands/__init__.py b/tests/commands/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/commands/test_cli.py b/tests/commands/test_cli.py new file mode 100644 index 0000000..7518359 --- /dev/null +++ b/tests/commands/test_cli.py @@ -0,0 +1,26 @@ +import unittest + +from click.testing import CliRunner + +from tutor.commands.cli import cli, print_help + + +class CliTests(unittest.TestCase): + def test_help(self) -> None: + runner = CliRunner() + result = runner.invoke(print_help) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + + def test_cli_help(self) -> None: + runner = CliRunner() + result = runner.invoke(cli, ["--help"]) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + + def test_cli_version(self) -> None: + runner = CliRunner() + result = runner.invoke(cli, ["--version"]) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + self.assertRegex(result.output, r"cli, version \d+.\d+.\d+\n") diff --git a/tests/commands/test_config.py b/tests/commands/test_config.py new file mode 100644 index 0000000..319f5c8 --- /dev/null +++ b/tests/commands/test_config.py @@ -0,0 +1,116 @@ +import os +import tempfile +import unittest + +from click.testing import CliRunner + +from tests.helpers import TestContext, temporary_root +from tutor import config as tutor_config +from tutor.commands.config import config_command + + +class ConfigTests(unittest.TestCase): + def test_config_help(self) -> None: + runner = CliRunner() + result = runner.invoke(config_command, ["--help"]) + self.assertEqual(0, result.exit_code) + self.assertFalse(result.exception) + + def test_config_save(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(config_command, ["save"], obj=context) + self.assertEqual(0, result.exit_code) + self.assertFalse(result.exception) + + def test_config_save_interactive(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(config_command, ["save", "-i"], obj=context) + self.assertEqual(0, result.exit_code) + self.assertFalse(result.exception) + + def test_config_save_skip_update(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(config_command, ["save", "-e"], obj=context) + self.assertEqual(0, result.exit_code) + self.assertFalse(result.exception) + + def test_config_save_set_value(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke( + config_command, ["save", "-s", "key=value"], obj=context + ) + self.assertEqual(0, result.exit_code) + self.assertFalse(result.exception) + result = runner.invoke(config_command, ["printvalue", "key"], obj=context) + self.assertIn("value", result.output) + + def test_config_save_unset_value(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(config_command, ["save", "-U", "key"], obj=context) + self.assertEqual(0, result.exit_code) + self.assertFalse(result.exception) + result = runner.invoke(config_command, ["printvalue", "key"], obj=context) + self.assertEqual(1, result.exit_code) + + def test_config_printroot(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(config_command, ["printroot"], obj=context) + self.assertEqual(0, result.exit_code) + self.assertFalse(result.exception) + self.assertIn(context.root, result.output) + + def test_config_printvalue(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + runner.invoke(config_command, ["save"], obj=context) + result = runner.invoke( + config_command, ["printvalue", "MYSQL_ROOT_PASSWORD"], obj=context + ) + self.assertEqual(0, result.exit_code) + self.assertFalse(result.exception) + self.assertTrue(result.output) + + def test_config_render(self) -> None: + with tempfile.TemporaryDirectory() as dest: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + runner.invoke(config_command, ["save"], obj=context) + result = runner.invoke( + config_command, ["render", context.root, dest], obj=context + ) + self.assertEqual(0, result.exit_code) + self.assertFalse(result.exception) + + def test_config_render_with_extra_configs(self) -> None: + with tempfile.TemporaryDirectory() as dest: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + runner.invoke(config_command, ["save"], obj=context) + result = runner.invoke( + config_command, + [ + "render", + "-x", + os.path.join(context.root, tutor_config.CONFIG_FILENAME), + context.root, + dest, + ], + obj=context, + ) + self.assertEqual(0, result.exit_code) + self.assertFalse(result.exception) diff --git a/tests/commands/test_context.py b/tests/commands/test_context.py new file mode 100644 index 0000000..dd76bde --- /dev/null +++ b/tests/commands/test_context.py @@ -0,0 +1,18 @@ +import os +import unittest + +from tests.helpers import TestContext, TestJobRunner, temporary_root +from tutor import config as tutor_config + + +class TestContextTests(unittest.TestCase): + def test_create_testcontext(self) -> None: + with temporary_root() as root: + context = TestContext(root) + config = tutor_config.load_full(root) + runner = context.job_runner(config) + self.assertTrue(os.path.exists(context.root)) + self.assertFalse( + os.path.exists(os.path.join(context.root, tutor_config.CONFIG_FILENAME)) + ) + self.assertTrue(isinstance(runner, TestJobRunner)) diff --git a/tests/commands/test_dev.py b/tests/commands/test_dev.py new file mode 100644 index 0000000..7a7205a --- /dev/null +++ b/tests/commands/test_dev.py @@ -0,0 +1,20 @@ +import unittest + +from click.testing import CliRunner + +from tutor.commands.compose import bindmount_command +from tutor.commands.dev import dev + + +class DevTests(unittest.TestCase): + def test_dev_help(self) -> None: + runner = CliRunner() + result = runner.invoke(dev, ["--help"]) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + + def test_dev_bindmount(self) -> None: + runner = CliRunner() + result = runner.invoke(bindmount_command, ["--help"]) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) diff --git a/tests/commands/test_images.py b/tests/commands/test_images.py new file mode 100644 index 0000000..1a3c058 --- /dev/null +++ b/tests/commands/test_images.py @@ -0,0 +1,176 @@ +import unittest +from unittest.mock import Mock, patch + +from click.testing import CliRunner + +from tests.helpers import TestContext, temporary_root +from tutor import images, plugins +from tutor.commands.config import config_command +from tutor.commands.images import ImageNotFoundError, images_command + + +class ImagesTests(unittest.TestCase): + def test_images_help(self) -> None: + runner = CliRunner() + result = runner.invoke(images_command, ["--help"]) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + + def test_images_pull_image(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(images_command, ["pull"], obj=context) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + + def test_images_pull_plugin_invalid_plugin_should_throw_error(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(images_command, ["pull", "plugin"], obj=context) + self.assertEqual(1, result.exit_code) + self.assertEqual(ImageNotFoundError, type(result.exception)) + + @patch.object(plugins.BasePlugin, "iter_installed", return_value=[]) + @patch.object( + plugins.Plugins, + "iter_hooks", + return_value=[ + ( + "dev-plugins", + {"plugin": "plugin:dev-1.0.0", "plugin2": "plugin2:dev-1.0.0"}, + ) + ], + ) + @patch.object(images, "pull", return_value=None) + def test_images_pull_plugin( + self, _image_pull: Mock, iter_hooks: Mock, iter_installed: Mock + ) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(images_command, ["pull", "plugin"], obj=context) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + iter_hooks.assert_called_once_with("remote-image") + _image_pull.assert_called_once_with("plugin:dev-1.0.0") + iter_installed.assert_called() + + def test_images_printtag_image(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(images_command, ["printtag", "openedx"], obj=context) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + self.assertRegex( + result.output, r"docker.io/overhangio/openedx:\d+.\d+.\d+\n" + ) + + @patch.object(plugins.BasePlugin, "iter_installed", return_value=[]) + @patch.object( + plugins.Plugins, + "iter_hooks", + return_value=[ + ( + "dev-plugins", + {"plugin": "plugin:dev-1.0.0", "plugin2": "plugin2:dev-1.0.0"}, + ) + ], + ) + def test_images_printtag_plugin( + self, iter_hooks: Mock, iter_installed: Mock + ) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(images_command, ["printtag", "plugin"], obj=context) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + iter_hooks.assert_called_once_with("build-image") + iter_installed.assert_called() + self.assertEqual(result.output, "plugin:dev-1.0.0\n") + + @patch.object(plugins.BasePlugin, "iter_installed", return_value=[]) + @patch.object( + plugins.Plugins, + "iter_hooks", + return_value=[ + ( + "dev-plugins", + {"plugin": "plugin:dev-1.0.0", "plugin2": "plugin2:dev-1.0.0"}, + ) + ], + ) + @patch.object(images, "build", return_value=None) + def test_images_build_plugin( + self, image_build: Mock, iter_hooks: Mock, iter_installed: Mock + ) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + runner.invoke(config_command, ["save"], obj=context) + result = runner.invoke(images_command, ["build", "plugin"], obj=context) + self.assertIsNone(result.exception) + self.assertEqual(0, result.exit_code) + image_build.assert_called() + iter_hooks.assert_called_once_with("build-image") + iter_installed.assert_called() + self.assertIn("plugin:dev-1.0.0", image_build.call_args[0]) + + @patch.object(plugins.BasePlugin, "iter_installed", return_value=[]) + @patch.object( + plugins.Plugins, + "iter_hooks", + return_value=[ + ( + "dev-plugins", + {"plugin": "plugin:dev-1.0.0", "plugin2": "plugin2:dev-1.0.0"}, + ) + ], + ) + @patch.object(images, "build", return_value=None) + def test_images_build_plugin_with_args( + self, image_build: Mock, iter_hooks: Mock, iter_installed: Mock + ) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + runner.invoke(config_command, ["save"], obj=context) + args = [ + "build", + "--no-cache", + "-a", + "myarg=value", + "--add-host", + "host", + "--target", + "target", + "-d", + "docker_args", + "plugin", + ] + result = runner.invoke( + images_command, + args, + obj=context, + ) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + iter_hooks.assert_called_once_with("build-image") + iter_installed.assert_called() + image_build.assert_called() + self.assertIn("plugin:dev-1.0.0", image_build.call_args[0]) + for arg in image_build.call_args[0][2:]: + if arg == "--build-arg": + continue + self.assertIn(arg, args) + + def test_images_push(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(images_command, ["push"], obj=context) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) diff --git a/tests/commands/test_k8s.py b/tests/commands/test_k8s.py new file mode 100644 index 0000000..4a9a730 --- /dev/null +++ b/tests/commands/test_k8s.py @@ -0,0 +1,13 @@ +import unittest + +from click.testing import CliRunner + +from tutor.commands.k8s import k8s + + +class K8sTests(unittest.TestCase): + def test_k8s_help(self) -> None: + runner = CliRunner() + result = runner.invoke(k8s, ["--help"]) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) diff --git a/tests/commands/test_local.py b/tests/commands/test_local.py new file mode 100644 index 0000000..88d930d --- /dev/null +++ b/tests/commands/test_local.py @@ -0,0 +1,25 @@ +import unittest + +from click.testing import CliRunner + +from tutor.commands.local import local, quickstart, upgrade + + +class LocalTests(unittest.TestCase): + def test_local_help(self) -> None: + runner = CliRunner() + result = runner.invoke(local, ["--help"]) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + + def test_local_quickstart_help(self) -> None: + runner = CliRunner() + result = runner.invoke(quickstart, ["--help"]) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + + def test_local_upgrade_help(self) -> None: + runner = CliRunner() + result = runner.invoke(upgrade, ["--help"]) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) diff --git a/tests/commands/test_plugins.py b/tests/commands/test_plugins.py new file mode 100644 index 0000000..f6a27b0 --- /dev/null +++ b/tests/commands/test_plugins.py @@ -0,0 +1,64 @@ +import unittest +from unittest.mock import Mock, patch + +from click.testing import CliRunner + +from tests.helpers import TestContext, temporary_root +from tutor import plugins +from tutor.commands.plugins import plugins_command + + +class PluginsTests(unittest.TestCase): + def test_plugins_help(self) -> None: + runner = CliRunner() + result = runner.invoke(plugins_command, ["--help"]) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + + def test_plugins_printroot(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(plugins_command, ["printroot"], obj=context) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + self.assertTrue(result.output) + + @patch.object(plugins.BasePlugin, "iter_installed", return_value=[]) + def test_plugins_list(self, _iter_installed: Mock) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(plugins_command, ["list"], obj=context) + self.assertEqual(0, result.exit_code) + self.assertIsNone(result.exception) + self.assertFalse(result.output) + _iter_installed.assert_called() + + def test_plugins_install_not_found_plugin(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke( + plugins_command, ["install", "notFound"], obj=context + ) + self.assertEqual(1, result.exit_code) + self.assertTrue(result.exception) + + def test_plugins_enable_not_installed_plugin(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke(plugins_command, ["enable", "notFound"], obj=context) + self.assertEqual(1, result.exit_code) + self.assertTrue(result.exception) + + def test_plugins_disable_not_installed_plugin(self) -> None: + with temporary_root() as root: + context = TestContext(root) + runner = CliRunner() + result = runner.invoke( + plugins_command, ["disable", "notFound"], obj=context + ) + self.assertEqual(0, result.exit_code) + self.assertFalse(result.exception) diff --git a/tests/helpers.py b/tests/helpers.py new file mode 100644 index 0000000..3762b8b --- /dev/null +++ b/tests/helpers.py @@ -0,0 +1,45 @@ +import os +import tempfile + +from tutor.commands.context import BaseJobContext +from tutor.jobs import BaseJobRunner +from tutor.types import Config + + +class TestJobRunner(BaseJobRunner): + def __init__(self, root: str, config: Config): + """ + Mock job runner for unit testing. + + This runner does nothing except print the service name and command, + separated by dashes. + """ + super().__init__(root, config) + + def run_job(self, service: str, command: str) -> int: + print( + os.linesep.join(["Service: {}".format(service), "-----", command, "----- "]) + ) + return 0 + + +def temporary_root() -> "tempfile.TemporaryDirectory[str]": + """ + Context manager to handle temporary test root. + + This function can be used as follows: + + with temporary_root() as root: + config = tutor_config.load_full(root) + ... + """ + return tempfile.TemporaryDirectory(prefix="tutor-test-root-") + + +class TestContext(BaseJobContext): + """ + Click context that will use only test job runners. + """ + + def job_runner(self, config: Config) -> TestJobRunner: + return TestJobRunner(self.root, config) diff --git a/tests/test_config.py b/tests/test_config.py index 30f934f..b3d5b18 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,12 +1,14 @@ +import json +import os import unittest from unittest.mock import Mock, patch -import tempfile import click +from tests.helpers import temporary_root from tutor import config as tutor_config from tutor import interactive -from tutor.types import get_typed, Config +from tutor.types import Config, get_typed class ConfigTests(unittest.TestCase): @@ -30,8 +32,8 @@ class ConfigTests(unittest.TestCase): self.assertNotEqual("abcd", config["MYSQL_ROOT_PASSWORD"]) @patch.object(tutor_config.fmt, "echo") - def test_save_load(self, _: Mock) -> None: - with tempfile.TemporaryDirectory() as root: + def test_update_twice_should_return_same_config(self, _: Mock) -> None: + with temporary_root() as root: config1 = tutor_config.load_minimal(root) tutor_config.save_config_file(root, config1) config2 = tutor_config.load_minimal(root) @@ -40,7 +42,7 @@ class ConfigTests(unittest.TestCase): @patch.object(tutor_config.fmt, "echo") def test_removed_entry_is_added_on_save(self, _: Mock) -> None: - with tempfile.TemporaryDirectory() as root: + with temporary_root() as root: with patch.object( tutor_config.utils, "random_string" ) as mock_random_string: @@ -62,7 +64,7 @@ class ConfigTests(unittest.TestCase): def mock_prompt(*_args: None, **kwargs: str) -> str: return kwargs["default"] - with tempfile.TemporaryDirectory() as rootdir: + with temporary_root() as rootdir: with patch.object(click, "prompt", new=mock_prompt): with patch.object(click, "confirm", new=mock_prompt): config = interactive.load_user_config(rootdir, interactive=True) @@ -74,6 +76,27 @@ class ConfigTests(unittest.TestCase): def test_is_service_activated(self) -> None: config: Config = {"RUN_SERVICE1": True, "RUN_SERVICE2": False} - self.assertTrue(tutor_config.is_service_activated(config, "service1")) self.assertFalse(tutor_config.is_service_activated(config, "service2")) + + @patch.object(tutor_config.fmt, "echo") + def test_json_config_is_overwritten_by_yaml(self, _: Mock) -> None: + with temporary_root() as root: + # Create config from scratch + config_yml_path = os.path.join(root, tutor_config.CONFIG_FILENAME) + config_json_path = os.path.join( + root, tutor_config.CONFIG_FILENAME.replace("yml", "json") + ) + config = tutor_config.load_full(root) + + # Save config to json + with open(config_json_path, "w", encoding="utf-8") as f: + json.dump(config, f, ensure_ascii=False, indent=4) + self.assertFalse(os.path.exists(config_yml_path)) + self.assertTrue(os.path.exists(config_json_path)) + + # Reload and compare + current = tutor_config.load_full(root) + self.assertTrue(os.path.exists(config_yml_path)) + self.assertFalse(os.path.exists(config_json_path)) + self.assertEqual(config, current) diff --git a/tests/test_env.py b/tests/test_env.py index b0e7395..5b24000 100644 --- a/tests/test_env.py +++ b/tests/test_env.py @@ -1,12 +1,10 @@ import os import tempfile import unittest -from unittest.mock import patch, Mock +from unittest.mock import Mock, patch from tutor import config as tutor_config -from tutor import env -from tutor import fmt -from tutor import exceptions +from tutor import env, exceptions, fmt from tutor.types import Config @@ -32,10 +30,10 @@ class EnvTests(unittest.TestCase): self.assertTrue(os.path.exists(path)) def test_pathjoin(self) -> None: - self.assertEqual( - "/tmp/env/target/dummy", env.pathjoin("/tmp", "target", "dummy") - ) - self.assertEqual("/tmp/env/dummy", env.pathjoin("/tmp", "dummy")) + with tempfile.TemporaryDirectory() as root: + self.assertEqual( + os.path.join(root, "env", "dummy"), env.pathjoin(root, "dummy") + ) def test_render_str(self) -> None: self.assertEqual( diff --git a/tests/test_images.py b/tests/test_images.py index 5a0759d..549406d 100644 --- a/tests/test_images.py +++ b/tests/test_images.py @@ -1,4 +1,5 @@ import unittest + from tutor import images from tutor.types import Config diff --git a/tests/test_jobs.py b/tests/test_jobs.py new file mode 100644 index 0000000..6ba8a6a --- /dev/null +++ b/tests/test_jobs.py @@ -0,0 +1,91 @@ +import re +import unittest +from io import StringIO +from unittest.mock import patch + +from tests.helpers import TestContext, temporary_root +from tutor import config as tutor_config +from tutor import jobs + + +class JobsTests(unittest.TestCase): + @patch("sys.stdout", new_callable=StringIO) + def test_initialise(self, mock_stdout: StringIO) -> None: + with temporary_root() as root: + context = TestContext(root) + config = tutor_config.load_full(root) + runner = context.job_runner(config) + jobs.initialise(runner) + + output = mock_stdout.getvalue().strip() + service = re.search(r"Service: (\w*)", output) + commands = re.search(r"(-----)([\S\s]+)(-----)", output) + assert service is not None + assert commands is not None + self.assertTrue(output.startswith("Initialising all services...")) + self.assertTrue(output.endswith("All services initialised.")) + self.assertEqual(service.group(1), "mysql") + self.assertTrue(commands.group(2)) + + def test_create_user_command_without_staff(self) -> None: + command = jobs.create_user_command("superuser", False, "username", "email") + self.assertNotIn("--staff", command) + + def test_create_user_command_with_staff(self) -> None: + command = jobs.create_user_command("superuser", True, "username", "email") + self.assertIn("--staff", command) + + def test_create_user_command_with_staff_with_password(self) -> None: + command = jobs.create_user_command( + "superuser", True, "username", "email", "command" + ) + self.assertIn("set_password", command) + + @patch("sys.stdout", new_callable=StringIO) + def test_import_demo_course(self, mock_stdout: StringIO) -> None: + with temporary_root() as root: + context = TestContext(root) + config = tutor_config.load_full(root) + runner = context.job_runner(config) + jobs.import_demo_course(runner) + + output = mock_stdout.getvalue() + service = re.search(r"Service: (\w*)", output) + commands = re.search(r"(-----)([\S\s]+)(-----)", output) + assert service is not None + assert commands is not None + self.assertEqual(service.group(1), "cms") + self.assertTrue( + commands.group(2) + .strip() + .startswith('echo "Loading settings $DJANGO_SETTINGS_MODULE"') + ) + + @patch("sys.stdout", new_callable=StringIO) + def test_set_theme(self, mock_stdout: StringIO) -> None: + with temporary_root() as root: + context = TestContext(root) + config = tutor_config.load_full(root) + runner = context.job_runner(config) + jobs.set_theme("sample_theme", ["domain1", "domain2"], runner) + + output = mock_stdout.getvalue() + service = re.search(r"Service: (\w*)", output) + commands = re.search(r"(-----)([\S\s]+)(-----)", output) + assert service is not None + assert commands is not None + self.assertEqual(service.group(1), "lms") + self.assertTrue( + commands.group(2) + .strip() + .startswith( + "export DJANGO_SETTINGS_MODULE=$SERVICE_VARIANT.envs.$SETTINGS" + ) + ) + + def test_get_all_openedx_domains(self) -> None: + with temporary_root() as root: + config = tutor_config.load_full(root) + domains = jobs.get_all_openedx_domains(config) + self.assertTrue(domains) + self.assertEqual(6, len(domains)) diff --git a/tests/test_plugins.py b/tests/test_plugins.py index 38f756e..2d30d57 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -2,10 +2,8 @@ import unittest from unittest.mock import Mock, patch from tutor import config as tutor_config -from tutor import exceptions -from tutor import fmt -from tutor import plugins -from tutor.types import get_typed, Config +from tutor import exceptions, fmt, plugins +from tutor.types import Config, get_typed class PluginsTests(unittest.TestCase): @@ -13,15 +11,16 @@ class PluginsTests(unittest.TestCase): plugins.Plugins.clear() @patch.object(plugins.DictPlugin, "iter_installed", return_value=[]) - def test_iter_installed(self, _dict_plugin_iter_installed: Mock) -> None: + def test_iter_installed(self, dict_plugin_iter_installed: Mock) -> None: with patch.object(plugins.pkg_resources, "iter_entry_points", return_value=[]): # type: ignore self.assertEqual([], list(plugins.iter_installed())) + dict_plugin_iter_installed.assert_called_once() def test_is_installed(self) -> None: self.assertFalse(plugins.is_installed("dummy")) @patch.object(plugins.DictPlugin, "iter_installed", return_value=[]) - def test_official_plugins(self, _dict_plugin_iter_installed: Mock) -> None: + def test_official_plugins(self, dict_plugin_iter_installed: Mock) -> None: with patch.object(plugins.importlib, "import_module", return_value=42): # type: ignore plugin1 = plugins.OfficialPlugin.load("plugin1") with patch.object(plugins.importlib, "import_module", return_value=43): # type: ignore @@ -35,6 +34,7 @@ class PluginsTests(unittest.TestCase): [plugin1, plugin2], list(plugins.iter_installed()), ) + dict_plugin_iter_installed.assert_called_once() def test_enable(self) -> None: config: Config = {plugins.CONFIG_KEY: []} diff --git a/tests/test_utils.py b/tests/test_utils.py index 2c23179..73e11bf 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,7 +1,12 @@ import base64 +import os +import sys +import tempfile import unittest +from io import StringIO +from unittest.mock import MagicMock, patch -from tutor import utils +from tutor import exceptions, utils class UtilsTests(unittest.TestCase): @@ -24,7 +29,7 @@ class UtilsTests(unittest.TestCase): def test_list_if(self) -> None: self.assertEqual('["cms"]', utils.list_if([("lms", False), ("cms", True)])) - def test_encrypt_decrypt(self) -> None: + def test_encrypt_success(self) -> None: password = "passw0rd" encrypted1 = utils.encrypt(password) encrypted2 = utils.encrypt(password) @@ -32,6 +37,24 @@ class UtilsTests(unittest.TestCase): self.assertTrue(utils.verify_encrypted(encrypted1, password)) self.assertTrue(utils.verify_encrypted(encrypted2, password)) + def test_encrypt_fail(self) -> None: + password = "passw0rd" + self.assertFalse(utils.verify_encrypted(password, password)) + + def test_ensure_file_directory_exists(self) -> None: + with tempfile.TemporaryDirectory() as root: + tempPath = os.path.join(root, "tempDir", "tempFile") + utils.ensure_file_directory_exists(tempPath) + self.assertTrue(os.path.exists(os.path.dirname(tempPath))) + + def test_ensure_file_directory_exists_dirExists(self) -> None: + with tempfile.TemporaryDirectory() as root: + tempPath = os.path.join(root, "tempDir") + os.makedirs(tempPath) + self.assertRaises( + exceptions.TutorError, utils.ensure_file_directory_exists, tempPath + ) + def test_long_to_base64(self) -> None: self.assertEqual( b"\x00", base64.urlsafe_b64decode(utils.long_to_base64(0) + "==") @@ -45,3 +68,93 @@ class UtilsTests(unittest.TestCase): self.assertIsNotNone(imported.n) self.assertIsNotNone(imported.p) self.assertIsNotNone(imported.q) + + def test_is_root(self) -> None: + result = utils.is_root() + self.assertFalse(result) + + @patch("sys.platform", "win32") + def test_is_root_win32(self) -> None: + result = utils.is_root() + self.assertFalse(result) + + def test_get_user_id(self) -> None: + result = utils.get_user_id() + if sys.platform == "win32": + self.assertEqual(0, result) + else: + self.assertNotEqual(0, result) + + @patch("sys.platform", "win32") + def test_get_user_id_win32(self) -> None: + result = utils.get_user_id() + self.assertEqual(0, result) + + @patch("sys.stdout", new_callable=StringIO) + @patch("subprocess.Popen", autospec=True) + def test_execute_exit_without_error( + self, mock_popen: MagicMock, mock_stdout: StringIO + ) -> None: + process = mock_popen.return_value + mock_popen.return_value.__enter__.return_value = process + process.wait.return_value = 0 + process.communicate.return_value = ("output", "error") + + result = utils.execute("echo", "") + self.assertEqual(0, result) + self.assertEqual("echo \n", mock_stdout.getvalue()) + self.assertEqual(1, process.wait.call_count) + process.kill.assert_not_called() + + @patch("sys.stdout", new_callable=StringIO) + @patch("subprocess.Popen", autospec=True) + def test_execute_exit_with_error( + self, mock_popen: MagicMock, mock_stdout: StringIO + ) -> None: + process = mock_popen.return_value + mock_popen.return_value.__enter__.return_value = process + process.wait.return_value = 1 + process.communicate.return_value = ("output", "error") + + self.assertRaises(exceptions.TutorError, utils.execute, "echo", "") + self.assertEqual("echo \n", mock_stdout.getvalue()) + self.assertEqual(1, process.wait.call_count) + process.kill.assert_not_called() + + @patch("sys.stdout", new_callable=StringIO) + @patch("subprocess.Popen", autospec=True) + def test_execute_throw_exception( + self, mock_popen: MagicMock, mock_stdout: StringIO + ) -> None: + process = mock_popen.return_value + mock_popen.return_value.__enter__.return_value = process + process.wait.side_effect = ZeroDivisionError("Exception occurred.") + + self.assertRaises(ZeroDivisionError, utils.execute, "echo", "") + self.assertEqual("echo \n", mock_stdout.getvalue()) + self.assertEqual(2, process.wait.call_count) + process.kill.assert_called_once() + + @patch("sys.stdout", new_callable=StringIO) + @patch("subprocess.Popen", autospec=True) + def test_execute_keyboard_interrupt( + self, mock_popen: MagicMock, mock_stdout: StringIO + ) -> None: + process = mock_popen.return_value + mock_popen.return_value.__enter__.return_value = process + process.wait.side_effect = KeyboardInterrupt() + + with self.assertRaises(KeyboardInterrupt): + utils.execute("echo", "") + output = mock_stdout.getvalue() + self.assertIn("echo", output) + self.assertEqual(2, process.wait.call_count) + process.kill.assert_called_once() + + @patch("sys.platform", "win32") + def test_check_macos_memory_win32_should_skip(self) -> None: + utils.check_macos_memory() + + @patch("sys.platform", "darwin") + def test_check_macos_memory_darwin_filenotfound(self) -> None: + self.assertRaises(exceptions.TutorError, utils.check_macos_memory) diff --git a/tutor/commands/cli.py b/tutor/commands/cli.py index 19a1235..07f2757 100755 --- a/tutor/commands/cli.py +++ b/tutor/commands/cli.py @@ -1,20 +1,17 @@ -#! /usr/bin/env python3 import sys import appdirs import click +from .. import exceptions, fmt, utils +from ..__about__ import __app__, __version__ from .config import config_command from .context import Context from .dev import dev from .images import images_command from .k8s import k8s from .local import local -from .plugins import plugins_command, add_plugin_commands -from ..__about__ import __version__, __app__ -from .. import exceptions -from .. import fmt -from .. import utils +from .plugins import add_plugin_commands, plugins_command def main() -> None: @@ -35,7 +32,10 @@ def main() -> None: sys.exit(1) -@click.group(context_settings={"help_option_names": ["-h", "--help", "help"]}) +@click.group( + context_settings={"help_option_names": ["-h", "--help", "help"]}, + help="Tutor is the Docker-based Open edX distribution designed for peace of mind.", +) @click.version_option(version=__version__) @click.option( "-r", diff --git a/tutor/commands/compose.py b/tutor/commands/compose.py index 609c64f..f7c220e 100644 --- a/tutor/commands/compose.py +++ b/tutor/commands/compose.py @@ -6,11 +6,9 @@ import click from .. import bindmounts from .. import config as tutor_config from .. import env as tutor_env +from .. import fmt, jobs, utils from ..exceptions import TutorError -from .. import fmt -from .. import jobs from ..types import Config -from .. import utils from .context import BaseJobContext diff --git a/tutor/commands/config.py b/tutor/commands/config.py index 177a0a3..a776dc3 100644 --- a/tutor/commands/config.py +++ b/tutor/commands/config.py @@ -3,9 +3,7 @@ from typing import List import click from .. import config as tutor_config -from .. import env -from .. import exceptions -from .. import fmt +from .. import env, exceptions, fmt from .. import interactive as interactive_config from .. import serialize from ..types import Config diff --git a/tutor/commands/context.py b/tutor/commands/context.py index b85fb3b..3e859e0 100644 --- a/tutor/commands/context.py +++ b/tutor/commands/context.py @@ -9,7 +9,7 @@ class Context: The project `root` is passed to all subcommands of `tutor`; that's because it is defined as an argument of the top-level command. For instance: - tutor --root=... local run ... + $ tutor --root=... local run ... """ def __init__(self, root: str) -> None: diff --git a/tutor/commands/images.py b/tutor/commands/images.py index af08e91..29a8392 100644 --- a/tutor/commands/images.py +++ b/tutor/commands/images.py @@ -4,9 +4,7 @@ import click from .. import config as tutor_config from .. import env as tutor_env -from .. import exceptions -from .. import images -from .. import plugins +from .. import exceptions, images, plugins from ..types import Config from .context import Context @@ -88,7 +86,7 @@ def build( @click.argument("image_names", metavar="image", nargs=-1) @click.pass_obj def pull(context: Context, image_names: List[str]) -> None: - config = tutor_config.load(context.root) + config = tutor_config.load_full(context.root) for image in image_names: pull_image(config, image) @@ -97,7 +95,7 @@ def pull(context: Context, image_names: List[str]) -> None: @click.argument("image_names", metavar="image", nargs=-1) @click.pass_obj def push(context: Context, image_names: List[str]) -> None: - config = tutor_config.load(context.root) + config = tutor_config.load_full(context.root) for image in image_names: push_image(config, image) @@ -106,7 +104,7 @@ def push(context: Context, image_names: List[str]) -> None: @click.argument("image_names", metavar="image", nargs=-1) @click.pass_obj def printtag(context: Context, image_names: List[str]) -> None: - config = tutor_config.load(context.root) + config = tutor_config.load_full(context.root) for image in image_names: to_print = [] for _img, tag in iter_images(config, image, BASE_IMAGE_NAMES): diff --git a/tutor/commands/k8s.py b/tutor/commands/k8s.py index e7795ab..2eae6cf 100644 --- a/tutor/commands/k8s.py +++ b/tutor/commands/k8s.py @@ -6,12 +6,8 @@ import click from .. import config as tutor_config from .. import env as tutor_env -from .. import exceptions -from .. import fmt -from .. import jobs -from .. import serialize +from .. import exceptions, fmt, jobs, serialize, utils from ..types import Config, get_typed -from .. import utils from .config import save as config_save_command from .context import Context diff --git a/tutor/commands/local.py b/tutor/commands/local.py index fa50fe5..f1570b8 100644 --- a/tutor/commands/local.py +++ b/tutor/commands/local.py @@ -4,10 +4,8 @@ import click from .. import config as tutor_config from .. import env as tutor_env -from .. import fmt +from .. import exceptions, fmt, utils from ..types import Config, get_typed -from .. import utils -from .. import exceptions from . import compose from .config import save as config_save_command diff --git a/tutor/commands/plugins.py b/tutor/commands/plugins.py index fab677e..332378c 100644 --- a/tutor/commands/plugins.py +++ b/tutor/commands/plugins.py @@ -1,15 +1,13 @@ import os import shutil -from typing import List import urllib.request +from typing import List import click from .. import config as tutor_config from .. import env as tutor_env -from .. import exceptions -from .. import fmt -from .. import plugins +from .. import exceptions, fmt, plugins from .context import Context diff --git a/tutor/config.py b/tutor/config.py index 6b8f376..2185de3 100644 --- a/tutor/config.py +++ b/tutor/config.py @@ -3,6 +3,8 @@ import os from . import env, exceptions, fmt, plugins, serialize, utils from .types import Config, cast_config +CONFIG_FILENAME = "config.yml" + def load(root: str) -> Config: """ @@ -37,6 +39,11 @@ def load_minimal(root: str) -> Config: def load_full(root: str) -> Config: """ Load a full configuration, with user, base and defaults. + + Return: + current (dict): params currently saved in config.yml + defaults (dict): default values of params which might be missing from the + current config """ config = get_user(root) update_with_base(config) @@ -234,15 +241,13 @@ def convert_json2yml(root: str) -> None: return if os.path.exists(config_path(root)): raise exceptions.TutorError( - "Both config.json and config.yml exist in {}: only one of these files must exist to continue".format( - root - ) + f"Both config.json and {CONFIG_FILENAME} exist in {root}: only one of these files must exist to continue" ) config = get_yaml_file(json_path) save_config_file(root, config) os.remove(json_path) fmt.echo_info( - "File config.json detected in {} and converted to config.yml".format(root) + f"File config.json detected in {root} and converted to {CONFIG_FILENAME}" ) @@ -251,8 +256,8 @@ def save_config_file(root: str, config: Config) -> None: utils.ensure_file_directory_exists(path) with open(path, "w") as of: serialize.dump(config, of) - fmt.echo_info("Configuration saved to {}".format(path)) + fmt.echo_info(f"Configuration saved to {path}") def config_path(root: str) -> str: - return os.path.join(root, "config.yml") + return os.path.join(root, CONFIG_FILENAME) diff --git a/tutor/env.py b/tutor/env.py index 04f50a5..1482034 100644 --- a/tutor/env.py +++ b/tutor/env.py @@ -349,7 +349,8 @@ def current_version(root: str) -> str: path = pathjoin(root, VERSION_FILENAME) if not os.path.exists(path): return "0.0.0" - return open(path).read().strip() + with open(path) as f: + return f.read().strip() def read_template_file(*path: str) -> str: diff --git a/tutor/plugins.py b/tutor/plugins.py index 3cdfbf0..8156324 100644 --- a/tutor/plugins.py +++ b/tutor/plugins.py @@ -8,8 +8,8 @@ import appdirs import click import pkg_resources -from .__about__ import __app__ from . import exceptions, fmt, serialize +from .__about__ import __app__ from .types import Config, get_typed CONFIG_KEY = "PLUGINS" diff --git a/tutor/utils.py b/tutor/utils.py index 182e7ed..41df184 100644 --- a/tutor/utils.py +++ b/tutor/utils.py @@ -10,7 +10,6 @@ import sys from typing import List, Tuple import click - from Crypto.Protocol.KDF import bcrypt, bcrypt_check from Crypto.PublicKey import RSA from Crypto.PublicKey.RSA import RsaKey @@ -145,10 +144,11 @@ def get_user_id() -> int: """ Portable way to get user ID. Note: I have no idea if it actually works on windows... """ - if sys.platform == "win32": - # Don't even try - return 0 - return os.getuid() + if sys.platform != "win32": + return os.getuid() + + # Don't even try for windows + return 0 def docker_run(*command: str) -> int: @@ -187,7 +187,7 @@ def is_a_tty() -> bool: Return True if stdin is able to allocate a tty. Tty allocation sometimes cannot be enabled, for instance in cron jobs """ - return os.isatty(sys.stdin.fileno()) + return sys.stdin.isatty() def execute(*command: str) -> int: