Replace codecs.open() with built-in open() (#210)

* Set newline arg in open() to disable universal newlines
* Refactor test util patch_files() to use TemporaryDirectory

---------

Co-authored-by: Eemeli Aro <eemeli@mozilla.com>

Author: PredaaA

fluent.runtime/fluent/runtime/fallback.py

@@ -1,4 +1,3 @@
-import codecs
 import os
 from collections.abc import Generator
 from typing import TYPE_CHECKING, Any, Callable, Union, cast
@@ -158,7 +157,8 @@ def resources(
                 path = self.localize_path(os.path.join(root, resource_id), locale)
                 if not os.path.isfile(path):
                     continue
-                content = codecs.open(path, "r", "utf-8").read()
+                with open(path, "r", encoding="utf-8", newline="\n") as file:
+                    content = file.read()
                 resources.append(FluentParser().parse(content))
             if resources:
                 yield resources

fluent.runtime/tests/test_fallback.py

@@ -1,4 +1,5 @@
 import unittest
+from os.path import join
 from .utils import patch_files
 
 from fluent.runtime import FluentLocalization, FluentResourceLoader
@@ -11,30 +12,26 @@ def test_init(self):
         )
         self.assertTrue(callable(l10n.format_value))
 
-    @patch_files({
-        "de/one.ftl": """one = in German
-            .foo = one in German
-        """,
-        "de/two.ftl": """two = in German
-            .foo = two in German
-        """,
-        "fr/two.ftl": """three = in French
-            .foo = three in French
-        """,
-        "en/one.ftl": """four = exists
-            .foo = four in English
-        """,
-        "en/two.ftl": """
-five = exists
-    .foo = five in English
-bar =
-    .foo = bar in English
-baz = baz in English
-        """,
-    })
-    def test_bundles(self):
+    @patch_files(
+        {
+            "de": {
+                "one.ftl": "one = in German\n  .foo = one in German\n",
+                "two.ftl": "two = in German\n  .foo = two in German\n",
+            },
+            "fr": {"two.ftl": "three = in French\n  .foo = three in French\n"},
+            "en": {
+                "one.ftl": "four = exists\n  .foo = four in English\n",
+                "two.ftl": "five = exists\n  .foo = five in English\n"
+                + "bar =\n  .foo = bar in English\n"
+                + "baz = baz in English\n",
+            },
+        }
+    )
+    def test_bundles(self, root):
         l10n = FluentLocalization(
-            ["de", "fr", "en"], ["one.ftl", "two.ftl"], FluentResourceLoader("{locale}")
+            ["de", "fr", "en"],
+            ["one.ftl", "two.ftl"],
+            FluentResourceLoader(join(root, "{locale}")),
         )
         bundles_gen = l10n._bundles()
         bundle_de = next(bundles_gen)
@@ -91,29 +88,31 @@ def test_bundles(self):
 
 
 class TestResourceLoader(unittest.TestCase):
-    @patch_files({
-        "en/one.ftl": "one = exists",
-        "en/two.ftl": "two = exists",
-    })
-    def test_all_exist(self):
-        loader = FluentResourceLoader("{locale}")
+    @patch_files(
+        {
+            "en": {
+                "one.ftl": "one = exists",
+                "two.ftl": "two = exists",
+            }
+        }
+    )
+    def test_all_exist(self, root):
+        loader = FluentResourceLoader(join(root, "{locale}"))
         resources_list = list(loader.resources("en", ["one.ftl", "two.ftl"]))
         self.assertEqual(len(resources_list), 1)
         resources = resources_list[0]
         self.assertEqual(len(resources), 2)
 
-    @patch_files({
-        "en/two.ftl": "two = exists",
-    })
-    def test_one_exists(self):
-        loader = FluentResourceLoader("{locale}")
+    @patch_files({"en": {"two.ftl": "two = exists"}})
+    def test_one_exists(self, root):
+        loader = FluentResourceLoader(join(root, "{locale}"))
         resources_list = list(loader.resources("en", ["one.ftl", "two.ftl"]))
         self.assertEqual(len(resources_list), 1)
         resources = resources_list[0]
         self.assertEqual(len(resources), 1)
 
     @patch_files({})
-    def test_none_exist(self):
-        loader = FluentResourceLoader("{locale}")
+    def test_none_exist(self, root):
+        loader = FluentResourceLoader(join(root, "{locale}"))
         resources_list = list(loader.resources("en", ["one.ftl", "two.ftl"]))
         self.assertEqual(len(resources_list), 0)

fluent.runtime/tests/test_utils.py

@@ -1,42 +1,30 @@
 import unittest
 from .utils import patch_files
-import os
-import codecs
+from os.path import isdir, isfile, join
 
 
 class TestFileSimulate(unittest.TestCase):
     def test_basic(self):
-        @patch_files({
-            "the.txt": "The",
-            "en/one.txt": "One",
-            "en/two.txt": "Two"
-        })
-        def patch_me(a, b):
+        @patch_files(
+            {
+                "the.txt": "The",
+                "en": {
+                    "one.txt": "One",
+                    "two.txt": "Two",
+                },
+            }
+        )
+        def patch_me(a, b, root):
             self.assertEqual(a, 10)
             self.assertEqual(b, "b")
-            self.assertFileIs(os.path.basename(__file__), None)
-            self.assertFileIs("the.txt", "The")
-            self.assertFileIs("en/one.txt", "One")
-            self.assertFileIs("en\\one.txt", "One")
-            self.assertFileIs("en/two.txt", "Two")
-            self.assertFileIs("en\\two.txt", "Two")
-            self.assertFileIs("en/three.txt", None)
-            self.assertFileIs("en\\three.txt", None)
+            with open(join(root, "the.txt")) as f:
+                self.assertEqual(f.read(), "The")
+            with open(join(root, "en", "one.txt")) as f:
+                self.assertEqual(f.read(), "One")
+            with open(join(root, "en", "two.txt")) as f:
+                self.assertEqual(f.read(), "Two")
+            self.assertTrue(isdir(join(root, "en")))
+            self.assertFalse(isfile(join(root, "none.txt")))
+            self.assertFalse(isfile(join(root, "en", "three.txt")))
 
-            with self.assertRaises(ValueError):
-                os.path.isfile("en/")
         patch_me(10, "b")
-
-    def assertFileIs(self, filename, expect_contents):
-        """
-        expect_contents is None: Expect file does not exist
-        expect_contents is a str: Expect file to exist and contents to match
-        """
-        if expect_contents is None:
-            self.assertFalse(os.path.isfile(filename),
-                             f"Expected {filename} to not exist.")
-        else:
-            self.assertTrue(os.path.isfile(filename),
-                            f"Expected {filename} to exist.")
-            with codecs.open(filename, "r", "utf-8") as f:
-                self.assertEqual(f.read(), expect_contents)

fluent.runtime/tests/utils.py

@@ -1,50 +1,35 @@
 """Utilities for testing."""
 
 import textwrap
-from pathlib import PureWindowsPath, PurePosixPath
-from unittest import mock
-from io import StringIO
-import functools
+from functools import wraps
+from os import mkdir
+from os.path import join
+from tempfile import TemporaryDirectory
 
 
 def dedent_ftl(text):
     return textwrap.dedent(f"{text.rstrip()}\n")
 
 
-# Needed in test_falllback.py because it uses dict + string compare to make a virtual file structure
-def _normalize_file_path(path):
-    """Note: Does not support absolute paths or paths that
-    contain '.' or '..' parts."""
-    # Cannot use os.path or PurePath, because they only recognize
-    # one kind of path separator
-    if PureWindowsPath(path).is_absolute() or PurePosixPath(path).is_absolute():
-        raise ValueError(f"Unsupported path: {path}")
-    parts = path.replace("\\", "/").split("/")
-    if "." in parts or ".." in parts:
-        raise ValueError(f"Unsupported path: {path}")
-    if parts and parts[-1] == "":
-        # path ends with a trailing pathsep
-        raise ValueError(f"Path appears to be a directory, not a file: {path}")
-    return "/".join(parts)
-
-
-def patch_files(files: dict):
-    """Decorate a function to simulate files ``files`` during the function.
-
-    The keys of ``files`` are file names and must use '/' for path separator.
-    The values are file contents. Directories or relative paths are not supported.
-    Example: ``{"en/one.txt": "One", "en/two.txt": "Two"}``
-
-    The implementation may be changed to match the mechanism used.
-    """
-
-    # Here it is possible to validate file names, but skipped
-
-    def then(func):
-        @mock.patch("os.path.isfile", side_effect=lambda p: _normalize_file_path(p) in files)
-        @mock.patch("codecs.open", side_effect=lambda p, _, __: StringIO(files[_normalize_file_path(p)]))
-        @functools.wraps(func)  # Make ret look like func to later decorators
-        def ret(*args, **kwargs):
-            func(*args[:-2], **kwargs)
-        return ret
-    return then
+def patch_files(tree: dict):
+    def build_file_tree(root: str, tree: dict) -> None:
+        for name, value in tree.items():
+            path = join(root, name)
+            if isinstance(value, str):
+                with open(path, "x", encoding="utf-8", newline="\n") as file:
+                    if value:
+                        file.write(value)
+            else:
+                mkdir(path)
+                build_file_tree(path, value)
+
+    def decorator(fn):
+        @wraps(fn)
+        def wrapper(*args, **kwargs):
+            with TemporaryDirectory() as root:
+                build_file_tree(root, tree)
+                return fn(*args, root, **kwargs)
+
+        return wrapper
+
+    return decorator

fluent.syntax/tests/syntax/test_reference.py

@@ -1,4 +1,3 @@
-import codecs
 import json
 import os
 import unittest
@@ -7,7 +6,7 @@
 
 
 def read_file(path):
-    with codecs.open(path, "r", encoding="utf-8") as file:
+    with open(path, "r", encoding="utf-8", newline="\n") as file:
         text = file.read()
     return text
 

fluent.syntax/tests/syntax/test_structure.py

@@ -1,4 +1,3 @@
-import codecs
 import json
 import os
 import unittest
@@ -7,7 +6,7 @@
 
 
 def read_file(path):
-    with codecs.open(path, "r", encoding="utf-8") as file:
+    with open(path, "r", encoding="utf-8", newline="\n") as file:
         text = file.read()
     return text
 

tools/fluentfmt.py

@@ -1,6 +1,5 @@
 #!/usr/bin/python
 
-import codecs
 import sys
 
 from fluent.syntax import parse, serialize
@@ -9,7 +8,7 @@
 
 
 def read_file(path):
-    with codecs.open(path, "r", encoding="utf-8") as file:
+    with open(path, "r", encoding="utf-8", newline="\n") as file:
         text = file.read()
     return text
 

tools/parse.py

@@ -1,6 +1,5 @@
 #!/usr/bin/python
 
-import codecs
 import json
 import sys
 
@@ -10,7 +9,7 @@
 
 
 def read_file(path):
-    with codecs.open(path, "r", encoding="utf-8") as file:
+    with open(path, "r", encoding="utf-8", newline="\n") as file:
         text = file.read()
     return text
 

tools/serialize.py

@@ -1,6 +1,5 @@
 #!/usr/bin/python
 
-import codecs
 import json
 import sys
 
@@ -10,7 +9,7 @@
 
 
 def read_json(path):
-    with codecs.open(path, "r", encoding="utf-8") as file:
+    with open(path, "r", encoding="utf-8", newline="\n") as file:
         return json.load(file)