From 1e0c3055085ddadab5858cdddaa3159d3f9a3155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Mon, 30 May 2022 18:12:00 +0200 Subject: [PATCH] fix: get rid of the `tutor config render` command This command is useless now that we can implement themes as plugins. This allows us to considerably simplify the Renderer class constructor. --- CHANGELOG.md | 2 ++ tests/commands/test_config.py | 29 ----------------------------- tests/test_env.py | 12 ++++++------ tutor/commands/config.py | 24 ------------------------ tutor/env.py | 26 ++++++++++---------------- 5 files changed, 18 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4b4dbd..8524fba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ Every user-facing change should have an entry in this changelog. Please respect ## Unreleased +- 💥[Fix] Get rid of the `tutor config render` command, which is useless now that themes can be implemented as plugins. (by @regisb) + ## v13.2.3 (2022-05-30) - [Fix] Truncate site display name to 50 characters with a warning, fixing data too long error for long site names. (by @navinkarkera) diff --git a/tests/commands/test_config.py b/tests/commands/test_config.py index f044e11..731a183 100644 --- a/tests/commands/test_config.py +++ b/tests/commands/test_config.py @@ -1,9 +1,6 @@ -import os -import tempfile import unittest from tests.helpers import temporary_root -from tutor import config as tutor_config from .base import TestCommandMixin @@ -61,29 +58,3 @@ class ConfigTests(unittest.TestCase, TestCommandMixin): self.assertFalse(result.exception) self.assertEqual(0, result.exit_code) self.assertTrue(result.output) - - def test_config_render(self) -> None: - with tempfile.TemporaryDirectory() as dest: - with temporary_root() as root: - self.invoke_in_root(root, ["config", "save"]) - result = self.invoke_in_root(root, ["config", "render", root, dest]) - 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: - self.invoke_in_root(root, ["config", "save"]) - result = self.invoke_in_root( - root, - [ - "config", - "render", - "-x", - os.path.join(root, tutor_config.CONFIG_FILENAME), - root, - dest, - ], - ) - self.assertEqual(0, result.exit_code) - self.assertFalse(result.exception) diff --git a/tests/test_env.py b/tests/test_env.py index b1a5e79..44a4c98 100644 --- a/tests/test_env.py +++ b/tests/test_env.py @@ -13,13 +13,13 @@ from tutor.types import Config class EnvTests(PluginsTestCase): def test_walk_templates(self) -> None: - renderer = env.Renderer({}, [env.TEMPLATES_ROOT]) + renderer = env.Renderer() templates = list(renderer.walk_templates("local")) self.assertIn("local/docker-compose.yml", templates) def test_walk_templates_partials_are_ignored(self) -> None: template_name = "apps/openedx/settings/partials/common_all.py" - renderer = env.Renderer({}, [env.TEMPLATES_ROOT], ignore_folders=["partials"]) + renderer = env.Renderer() templates = list(renderer.walk_templates("apps")) self.assertIn(template_name, renderer.environment.loader.list_templates()) self.assertNotIn(template_name, templates) @@ -28,7 +28,7 @@ class EnvTests(PluginsTestCase): self.assertTrue(env.is_binary_file("/home/somefile.ico")) def test_find_os_path(self) -> None: - renderer = env.Renderer({}, [env.TEMPLATES_ROOT]) + renderer = env.Renderer() path = renderer.find_os_path("local/docker-compose.yml") self.assertTrue(os.path.exists(path)) @@ -180,14 +180,14 @@ class EnvTests(PluginsTestCase): # Load env once config: Config = {"PLUGINS": []} - env1 = env.Renderer.instance(config).environment + env1 = env.Renderer(config).environment # Enable plugins plugins.load("plugin1") # Load env a second time config["PLUGINS"] = ["myplugin"] - env2 = env.Renderer.instance(config).environment + env2 = env.Renderer(config).environment self.assertNotIn("plugin1/myplugin.txt", env1.loader.list_templates()) self.assertIn("plugin1/myplugin.txt", env2.loader.list_templates()) @@ -199,7 +199,7 @@ class EnvTests(PluginsTestCase): "notsomething_test_app": 2, "something3_test_app": 3, } - renderer = env.Renderer.instance(config) + renderer = env.Renderer(config) self.assertEqual([2, 3], list(renderer.iter_values_named(suffix="test_app"))) self.assertEqual([1, 3], list(renderer.iter_values_named(prefix="something"))) self.assertEqual( diff --git a/tutor/commands/config.py b/tutor/commands/config.py index b329c95..83d347a 100644 --- a/tutor/commands/config.py +++ b/tutor/commands/config.py @@ -64,29 +64,6 @@ def save( env.save(context.root, config) -@click.command(help="Render a template folder with eventual extra configuration files") -@click.option( - "-x", - "--extra-config", - "extra_configs", - multiple=True, - type=click.Path(exists=True, resolve_path=True, dir_okay=False), - help="Load extra configuration file (can be used multiple times)", -) -@click.argument("src", type=click.Path(exists=True, resolve_path=True)) -@click.argument("dst") -@click.pass_obj -def render(context: Context, extra_configs: List[str], src: str, dst: str) -> None: - config = tutor_config.load(context.root) - for extra_config in extra_configs: - config.update( - env.render_unknown(config, tutor_config.get_yaml_file(extra_config)) - ) - renderer = env.Renderer(config, [src]) - renderer.render_all_to(dst) - fmt.echo_info(f"Templates rendered to {dst}") - - @click.command(help="Print the project root") @click.pass_obj def printroot(context: Context) -> None: @@ -106,6 +83,5 @@ def printvalue(context: Context, key: str) -> None: config_command.add_command(save) -config_command.add_command(render) config_command.add_command(printroot) config_command.add_command(printvalue) diff --git a/tutor/env.py b/tutor/env.py index 634372c..f0edc6f 100644 --- a/tutor/env.py +++ b/tutor/env.py @@ -67,26 +67,20 @@ class JinjaEnvironment(jinja2.Environment): class Renderer: - @classmethod - def instance(cls: t.Type["Renderer"], config: Config) -> "Renderer": - # Load template roots: these are required to be able to use - # {% include .. %} directives - template_roots = hooks.Filters.ENV_TEMPLATE_ROOTS.apply([TEMPLATES_ROOT]) - return cls(config, template_roots, ignore_folders=["partials"]) - def __init__( self, - config: Config, - template_roots: t.List[str], + config: t.Optional[Config] = None, ignore_folders: t.Optional[t.List[str]] = None, ): + config = config or {} self.config = deepcopy(config) - self.template_roots = template_roots - self.ignore_folders = ignore_folders or [] - self.ignore_folders.append(".git") + self.template_roots = hooks.Filters.ENV_TEMPLATE_ROOTS.apply([TEMPLATES_ROOT]) + self.ignore_folders = ["partials", ".git"] + if ignore_folders is not None: + self.ignore_folders = ignore_folders # Create environment with extra filters and globals - self.environment = JinjaEnvironment(template_roots) + self.environment = JinjaEnvironment(self.template_roots) # Filters plugin_filters: t.Iterator[ @@ -264,7 +258,7 @@ def save_all_from(prefix: str, dst: str, config: Config) -> None: Render the templates that start with `prefix` and store them with the same hierarchy at `dst`. Here, `prefix` can be the result of os.path.join(...). """ - renderer = Renderer.instance(config) + renderer = Renderer(config) renderer.render_all_to(dst, prefix.replace(os.sep, "/")) @@ -285,7 +279,7 @@ def render_file(config: Config, *path: str) -> t.Union[str, bytes]: """ Return the rendered contents of a template. """ - renderer = Renderer.instance(config) + renderer = Renderer(config) template_name = "/".join(path) return renderer.render_template(template_name) @@ -312,7 +306,7 @@ def render_str(config: Config, text: str) -> str: Return: substituted (str) """ - return Renderer.instance(config).render_str(text) + return Renderer(config).render_str(text) def check_is_up_to_date(root: str) -> None: