code: refactor version checking code

tests/test_env.py

@@ -3,9 +3,11 @@ import tempfile
 import unittest
 from unittest.mock import Mock, patch
 
+from tutor.__about__ import __version__
 from tutor import config as tutor_config
 from tutor import env, exceptions, fmt
 from tutor.types import Config
+from tests.helpers import temporary_root
 
 
 class EnvTests(unittest.TestCase):
@@ -30,9 +32,9 @@ class EnvTests(unittest.TestCase):
         self.assertTrue(os.path.exists(path))
 
     def test_pathjoin(self) -> None:
-        with tempfile.TemporaryDirectory() as root:
+        with temporary_root() as root:
             self.assertEqual(
-                os.path.join(root, "env", "dummy"), env.pathjoin(root, "dummy")
+                os.path.join(env.base_dir(root), "dummy"), env.pathjoin(root, "dummy")
             )
 
     def test_render_str(self) -> None:
@@ -76,21 +78,26 @@ class EnvTests(unittest.TestCase):
         )
 
     def test_save_full(self) -> None:
-        with tempfile.TemporaryDirectory() as root:
+        with temporary_root() as root:
             config = tutor_config.load_full(root)
             with patch.object(fmt, "STDOUT"):
                 env.save(root, config)
             self.assertTrue(
-                os.path.exists(os.path.join(root, "env", "local", "docker-compose.yml"))
+                os.path.exists(
+                    os.path.join(env.base_dir(root), "local", "docker-compose.yml")
+                )
             )
 
     def test_save_full_with_https(self) -> None:
-        with tempfile.TemporaryDirectory() as root:
+        with temporary_root() as root:
             config = tutor_config.load_full(root)
             config["ENABLE_HTTPS"] = True
             with patch.object(fmt, "STDOUT"):
                 env.save(root, config)
-                with open(os.path.join(root, "env", "apps", "caddy", "Caddyfile")) as f:
+                with open(
+                    os.path.join(env.base_dir(root), "apps", "caddy", "Caddyfile"),
+                    encoding="utf-8",
+                ) as f:
                     self.assertIn("www.myopenedx.com{$default_site_port}", f.read())
 
     def test_patch(self) -> None:
@@ -120,11 +127,15 @@ class EnvTests(unittest.TestCase):
             # Create two templates
             os.makedirs(os.path.join(plugin_templates, "plugin1", "apps"))
             with open(
-                os.path.join(plugin_templates, "plugin1", "unrendered.txt"), "w"
+                os.path.join(plugin_templates, "plugin1", "unrendered.txt"),
+                "w",
+                encoding="utf-8",
             ) as f:
                 f.write("This file should not be rendered")
             with open(
-                os.path.join(plugin_templates, "plugin1", "apps", "rendered.txt"), "w"
+                os.path.join(plugin_templates, "plugin1", "apps", "rendered.txt"),
+                "w",
+                encoding="utf-8",
             ) as f:
                 f.write("Hello my ID is {{ ID }}")
 
@@ -137,7 +148,7 @@ class EnvTests(unittest.TestCase):
                 "iter_enabled",
                 return_value=[plugin1],
             ):
-                with tempfile.TemporaryDirectory() as root:
+                with temporary_root() as root:
                     # Render plugin templates
                     env.save_plugin_templates(plugin1, root, config)
 
@@ -150,7 +161,7 @@ class EnvTests(unittest.TestCase):
                     )
                     self.assertFalse(os.path.exists(dst_unrendered))
                     self.assertTrue(os.path.exists(dst_rendered))
-                    with open(dst_rendered) as f:
+                    with open(dst_rendered, encoding="utf-8") as f:
                         self.assertEqual("Hello my ID is abcd", f.read())
 
     def test_renderer_is_reset_on_config_change(self) -> None:
@@ -161,7 +172,9 @@ class EnvTests(unittest.TestCase):
             # Create one template
             os.makedirs(os.path.join(plugin_templates, plugin1.name))
             with open(
-                os.path.join(plugin_templates, plugin1.name, "myplugin.txt"), "w"
+                os.path.join(plugin_templates, plugin1.name, "myplugin.txt"),
+                "w",
+                encoding="utf-8",
             ) as f:
                 f.write("some content")
 
@@ -199,3 +212,38 @@ class EnvTests(unittest.TestCase):
                 )
             ),
         )
+
+    def test_current_version_in_empty_env(self) -> None:
+        with temporary_root() as root:
+            self.assertIsNone(env.current_version(root))
+            self.assertIsNone(env.current_release(root))
+            self.assertFalse(env.needs_major_upgrade(root))
+            self.assertTrue(env.is_up_to_date(root))
+
+    def test_current_version_in_lilac_env(self) -> None:
+        with temporary_root() as root:
+            os.makedirs(env.base_dir(root))
+            with open(
+                os.path.join(env.base_dir(root), env.VERSION_FILENAME),
+                "w",
+                encoding="utf-8",
+            ) as f:
+                f.write("12.0.46")
+            self.assertEqual("12.0.46", env.current_version(root))
+            self.assertEqual("lilac", env.current_release(root))
+            self.assertTrue(env.needs_major_upgrade(root))
+            self.assertFalse(env.is_up_to_date(root))
+
+    def test_current_version_in_latest_env(self) -> None:
+        with temporary_root() as root:
+            os.makedirs(env.base_dir(root))
+            with open(
+                os.path.join(env.base_dir(root), env.VERSION_FILENAME),
+                "w",
+                encoding="utf-8",
+            ) as f:
+                f.write(__version__)
+            self.assertEqual(__version__, env.current_version(root))
+            self.assertEqual("maple", env.current_release(root))
+            self.assertFalse(env.needs_major_upgrade(root))
+            self.assertTrue(env.is_up_to_date(root))

tutor/env.py

@@ -1,4 +1,3 @@
-import codecs
 import os
 from copy import deepcopy
 from typing import Any, Iterable, List, Optional, Type, Union
@@ -139,9 +138,7 @@ class Renderer:
             try:
                 patches.append(self.render_str(patch))
             except exceptions.TutorError:
-                fmt.echo_error(
+                fmt.echo_error(f"Error rendering patch '{name}' from plugin {plugin}")
-                    "Error rendering patch '{}' from plugin {}".format(name, plugin)
-                )
                 raise
         rendered = separator.join(patches)
         if rendered:
@@ -193,9 +190,7 @@ class Renderer:
         try:
             return template.render(**self.config)
         except jinja2.exceptions.UndefinedError as e:
-            raise exceptions.TutorError(
+            raise exceptions.TutorError(f"Missing configuration value: {e.args[0]}")
-                "Missing configuration value: {}".format(e.args[0])
-            )
 
 
 def save(root: str, config: Config) -> None:
@@ -219,7 +214,7 @@ def save(root: str, config: Config) -> None:
             save_plugin_templates(plugin, root, config)
 
     upgrade_obsolete(root)
-    fmt.echo_info("Environment generated in {}".format(base_dir(root)))
+    fmt.echo_info(f"Environment generated in {base_dir(root)}")
 
 
 def upgrade_obsolete(_root: str) -> None:
@@ -280,7 +275,7 @@ def render_unknown(config: Config, value: Any) -> Any:
     """
     if isinstance(value, str):
         return render_str(config, value)
-    elif isinstance(value, dict):
+    if isinstance(value, dict):
         return {k: render_unknown(config, v) for k, v in value.items()}
     return value
 
@@ -299,15 +294,12 @@ def render_str(config: Config, text: str) -> str:
 
 def check_is_up_to_date(root: str) -> None:
     if not is_up_to_date(root):
-        message = (
-            "The current environment stored at {} is not up-to-date: it is at "
-            "v{} while the 'tutor' binary is at v{}. You should upgrade "
-            "the environment by running:\n"
-            "\n"
-            "    tutor config save"
-        )
         fmt.echo_alert(
-            message.format(base_dir(root), current_version(root), __version__)
+            f"The current environment stored at {base_dir(root)} is not up-to-date: it is at "
+            f"v{current_version(root)} while the 'tutor' binary is at v{__version__}. You should upgrade "
+            f"the environment by running:\n"
+            f"\n"
+            f"    tutor config save"
         )
 
 
@@ -315,22 +307,29 @@ def is_up_to_date(root: str) -> bool:
     """
     Check if the currently rendered version is equal to the current tutor version.
     """
-    return current_version(root) == __version__
+    current = current_version(root)
+    return current is None or current == __version__
 
 
 def needs_major_upgrade(root: str) -> bool:
     """
-    Return the current version as a tuple of int. E.g: (1, 0, 2).
+    Return true if the current version is less than the tutor version.
     """
-    current = int(current_version(root).split(".")[0])
+    current = current_version(root)
-    required = int(__version__.split(".")[0])
+    if current is None:
-    return 0 < current < required
+        return False
+    current_as_int = int(current.split(".")[0])
+    required = int(__version__.split(".", maxsplit=1)[0])
+    return 0 < current_as_int < required
 
 
-def current_release(root: str) -> str:
+def current_release(root: str) -> Optional[str]:
     """
-    Return the name of the current Open edX release.
+    Return the name of the current Open edX release. If the current environment has no version, return None.
     """
+    current = current_version(root)
+    if current is None:
+        return None
     return {
         "0": "ironwood",
         "3": "ironwood",
@@ -338,19 +337,19 @@ def current_release(root: str) -> str:
         "11": "koa",
         "12": "lilac",
         "13": "maple",
-    }[current_version(root).split(".")[0]]
+    }[current.split(".")[0]]
 
 
-def current_version(root: str) -> str:
+def current_version(root: str) -> Optional[str]:
     """
     Return the current environment version. If the current environment has no version,
-    return "0.0.0".
+    return None.
     """
     path = pathjoin(root, VERSION_FILENAME)
     if not os.path.exists(path):
-        return "0.0.0"
+        return None
-    with open(path) as f:
+    with open(path, encoding="utf-8") as fi:
-        return f.read().strip()
+        return fi.read().strip()
 
 
 def read_template_file(*path: str) -> str:
@@ -358,7 +357,7 @@ def read_template_file(*path: str) -> str:
     Read raw content of template located at `path`.
     """
     src = template_path(*path)
-    with codecs.open(src, encoding="utf-8") as fi:
+    with open(src, encoding="utf-8") as fi:
         return fi.read()