refactor: add code coverage, cover CLI commands with tests

.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

.gitignore

@@ -7,3 +7,7 @@ __pycache__
 /build/
 /dist/
 /release_description.md
+
+# Unit test/ coverage reports
+.coverage
+/htmlcov/

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

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
+if __name__ == "__main__":
-
+    main()
-main()

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,
+    _name: Any,
-    _args,
+    _args: Any,
-    _options,
+    _options: Any,
-    content,
+    content: List[str],
-    _lineno,
+    _lineno: Any,
-    _contentOffset,
+    _contentOffset: Any,
-    _blockText,
+    _blockText: Any,
-    _state,
+    _state: Any,
-    _stateMachine,
+    _stateMachine: Any,
-):
+) -> Any:
-    """ Restructured text extension for inserting youtube embedded videos """
+    """Restructured text extension for inserting youtube embedded videos"""
     if not content:
         return []
     video_id = content[0]

requirements/dev.in

@@ -4,6 +4,7 @@ pip-tools
 pylint
 pyinstaller
 twine
+coverage
 
 # Types packages
 types-PyYAML

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

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():
+def load_about() -> Dict[str, str]:
-    about = {}
+    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: str) -> bool:
-def is_requirement(line):
     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",
 )

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")

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)

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))

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)

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)

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)

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)

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)

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)

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:
+    def test_update_twice_should_return_same_config(self, _: Mock) -> None:
-        with tempfile.TemporaryDirectory() as root:
+        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)

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 env, exceptions, fmt
-from tutor import fmt
-from tutor import exceptions
 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(
+        with tempfile.TemporaryDirectory() as root:
-            "/tmp/env/target/dummy", env.pathjoin("/tmp", "target", "dummy")
+            self.assertEqual(
-        )
+                os.path.join(root, "env", "dummy"), env.pathjoin(root, "dummy")
-        self.assertEqual("/tmp/env/dummy", env.pathjoin("/tmp", "dummy"))
+            )
 
     def test_render_str(self) -> None:
         self.assertEqual(

tests/test_images.py

@@ -1,4 +1,5 @@
 import unittest
+
 from tutor import images
 from tutor.types import Config
 

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))

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 exceptions, fmt, plugins
-from tutor import fmt
+from tutor.types import Config, get_typed
-from tutor import plugins
-from tutor.types import get_typed, Config
 
 
 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: []}

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)

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 .plugins import add_plugin_commands, plugins_command
-from ..__about__ import __version__, __app__
-from .. import exceptions
-from .. import fmt
-from .. import utils
 
 
 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",

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
 
 

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 env, exceptions, fmt
-from .. import exceptions
-from .. import fmt
 from .. import interactive as interactive_config
 from .. import serialize
 from ..types import Config

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:

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 exceptions, images, plugins
-from .. import images
-from .. import 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):

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 exceptions, fmt, jobs, serialize, utils
-from .. import fmt
-from .. import jobs
-from .. import serialize
 from ..types import Config, get_typed
-from .. import utils
 from .config import save as config_save_command
 from .context import Context
 

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
 

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 exceptions, fmt, plugins
-from .. import fmt
-from .. import plugins
 from .context import Context
 
 

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(
+            f"Both config.json and {CONFIG_FILENAME} exist in {root}: only one of these files must exist to continue"
-                root
-            )
         )
     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)

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:

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"

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":
+    if sys.platform != "win32":
-        # Don't even try
+        return os.getuid()
-        return 0
+
-    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: