From 8862ee360a2b1123d8d0589e905c582deb050e27 Mon Sep 17 00:00:00 2001 From: studyingegret <110317665+studyingegret@users.noreply.github.com> Date: Fri, 31 Jan 2025 23:18:48 +0800 Subject: [PATCH 01/12] Fix test failures on Windows The test failures on Windows (which are not present on Linux) are caused by the test code using simple string compare (through dict) when simulating files. On Windows, paths are formed with "\" instead of "/", which causes inconsistency in the string compare. (Specifically, the "\" were inserted by os.path.join in FluentResourceLoader.) --- fluent.runtime/tests/__init__.py | 6 ++++++ fluent.runtime/tests/test_fallback.py | 31 ++++++++++++++++++++------- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/fluent.runtime/tests/__init__.py b/fluent.runtime/tests/__init__.py index e69de29b..6ca983d6 100644 --- a/fluent.runtime/tests/__init__.py +++ b/fluent.runtime/tests/__init__.py @@ -0,0 +1,6 @@ +import os + +# Unify path separator, default path separator on Windows is \ not / +# Needed because many tests uses dict + string compare to make a virtual file structure +def normalize_path(path): + return "/".join(os.path.split(path)) \ No newline at end of file diff --git a/fluent.runtime/tests/test_fallback.py b/fluent.runtime/tests/test_fallback.py index 0195281f..3ed78d60 100644 --- a/fluent.runtime/tests/test_fallback.py +++ b/fluent.runtime/tests/test_fallback.py @@ -3,10 +3,19 @@ import unittest from unittest import mock +from . import normalize_path + from fluent.runtime import FluentLocalization, FluentResourceLoader ISFILE = os.path.isfile +def show_bundle(bundle): + for name in ["one", "two", "three", "four", "five"]: + if bundle.has_message(name): + print(name + ":", bundle.format_pattern(bundle.get_message(name).value)) + else: + print(name + ": Not present") + class TestLocalization(unittest.TestCase): def test_init(self): @@ -25,22 +34,28 @@ def test_bundles(self, codecs_open, isfile): "en/one.ftl": "four = exists", "en/two.ftl": "five = exists", } - isfile.side_effect = lambda p: p in data or ISFILE(p) - codecs_open.side_effect = lambda p, _, __: io.StringIO(data[p]) + #isfile.side_effect = lambda p: p in data or ISFILE(p) + isfile.side_effect = lambda p: normalize_path(p) in data or ISFILE(p) + codecs_open.side_effect = lambda p, _, __: io.StringIO(data[normalize_path(p)]) l10n = FluentLocalization( ["de", "fr", "en"], ["one.ftl", "two.ftl"], FluentResourceLoader("{locale}") ) + # Curious + print("\ntest_bundles") bundles_gen = l10n._bundles() bundle_de = next(bundles_gen) + show_bundle(bundle_de) self.assertEqual(bundle_de.locales[0], "de") self.assertTrue(bundle_de.has_message("one")) self.assertTrue(bundle_de.has_message("two")) bundle_fr = next(bundles_gen) + show_bundle(bundle_fr) self.assertEqual(bundle_fr.locales[0], "fr") self.assertFalse(bundle_fr.has_message("one")) self.assertTrue(bundle_fr.has_message("three")) self.assertListEqual(list(l10n._bundles())[:2], [bundle_de, bundle_fr]) bundle_en = next(bundles_gen) + show_bundle(bundle_en) self.assertEqual(bundle_en.locales[0], "en") self.assertEqual(l10n.format_value("one"), "in German") self.assertEqual(l10n.format_value("two"), "in German") @@ -57,8 +72,8 @@ def test_all_exist(self, codecs_open, isfile): "en/one.ftl": "one = exists", "en/two.ftl": "two = exists", } - isfile.side_effect = lambda p: p in data - codecs_open.side_effect = lambda p, _, __: io.StringIO(data[p]) + isfile.side_effect = lambda p: normalize_path(p) in data + codecs_open.side_effect = lambda p, _, __: io.StringIO(data[normalize_path(p)]) loader = FluentResourceLoader("{locale}") resources_list = list(loader.resources("en", ["one.ftl", "two.ftl"])) self.assertEqual(len(resources_list), 1) @@ -69,8 +84,8 @@ def test_one_exists(self, codecs_open, isfile): data = { "en/two.ftl": "two = exists", } - isfile.side_effect = lambda p: p in data - codecs_open.side_effect = lambda p, _, __: io.StringIO(data[p]) + isfile.side_effect = lambda p: normalize_path(p) in data + codecs_open.side_effect = lambda p, _, __: io.StringIO(data[normalize_path(p)]) loader = FluentResourceLoader("{locale}") resources_list = list(loader.resources("en", ["one.ftl", "two.ftl"])) self.assertEqual(len(resources_list), 1) @@ -79,8 +94,8 @@ def test_one_exists(self, codecs_open, isfile): def test_none_exist(self, codecs_open, isfile): data = {} - isfile.side_effect = lambda p: p in data - codecs_open.side_effect = lambda p, _, __: io.StringIO(data[p]) + isfile.side_effect = lambda p: normalize_path(p) in data + codecs_open.side_effect = lambda p, _, __: io.StringIO(data[normalize_path(p)]) loader = FluentResourceLoader("{locale}") resources_list = list(loader.resources("en", ["one.ftl", "two.ftl"])) self.assertEqual(len(resources_list), 0) From bea90b9ecf70e1ca7b322e032cee4a3a30d2f401 Mon Sep 17 00:00:00 2001 From: studyingegret <110317665+studyingegret@users.noreply.github.com> Date: Sat, 1 Feb 2025 08:44:41 +0800 Subject: [PATCH 02/12] Tidy code to prepare for PR --- fluent.runtime/tests/test_fallback.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/fluent.runtime/tests/test_fallback.py b/fluent.runtime/tests/test_fallback.py index 3ed78d60..17de43c8 100644 --- a/fluent.runtime/tests/test_fallback.py +++ b/fluent.runtime/tests/test_fallback.py @@ -9,13 +9,6 @@ ISFILE = os.path.isfile -def show_bundle(bundle): - for name in ["one", "two", "three", "four", "five"]: - if bundle.has_message(name): - print(name + ":", bundle.format_pattern(bundle.get_message(name).value)) - else: - print(name + ": Not present") - class TestLocalization(unittest.TestCase): def test_init(self): @@ -41,21 +34,17 @@ def test_bundles(self, codecs_open, isfile): ["de", "fr", "en"], ["one.ftl", "two.ftl"], FluentResourceLoader("{locale}") ) # Curious - print("\ntest_bundles") bundles_gen = l10n._bundles() bundle_de = next(bundles_gen) - show_bundle(bundle_de) self.assertEqual(bundle_de.locales[0], "de") self.assertTrue(bundle_de.has_message("one")) self.assertTrue(bundle_de.has_message("two")) bundle_fr = next(bundles_gen) - show_bundle(bundle_fr) self.assertEqual(bundle_fr.locales[0], "fr") self.assertFalse(bundle_fr.has_message("one")) self.assertTrue(bundle_fr.has_message("three")) self.assertListEqual(list(l10n._bundles())[:2], [bundle_de, bundle_fr]) bundle_en = next(bundles_gen) - show_bundle(bundle_en) self.assertEqual(bundle_en.locales[0], "en") self.assertEqual(l10n.format_value("one"), "in German") self.assertEqual(l10n.format_value("two"), "in German") From 02fa44f4b1fc2683e0a54ff739565798f1160fad Mon Sep 17 00:00:00 2001 From: studyingegret <110317665+studyingegret@users.noreply.github.com> Date: Sun, 2 Feb 2025 09:53:07 +0800 Subject: [PATCH 03/12] ci: Workaround Python 3.7 no longer supported on GItHub Actions --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1313529b..8e93419f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,9 @@ jobs: - run: python -m flake8 - run: python -m mypy fluent.syntax/fluent fluent.runtime/fluent test: - runs-on: ubuntu-latest + # Workaround Python 3.7 no longer supported https://github.com/actions/setup-python/issues/962#issuecomment-2414454450 + #runs-on: ubuntu-latest + runs-on: ubuntu-22.04 strategy: matrix: python-version: [3.7, 3.8, 3.9, "3.10", 3.11, 3.12, pypy3.9, pypy3.10] From 6e8e716432707ac0fa755830b2b49390a3f1834d Mon Sep 17 00:00:00 2001 From: studyingegret <110317665+studyingegret@users.noreply.github.com> Date: Sun, 2 Feb 2025 13:50:40 +0800 Subject: [PATCH 04/12] Fix flake8 errors --- fluent.runtime/tests/__init__.py | 5 +++-- fluent.runtime/tests/test_fallback.py | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fluent.runtime/tests/__init__.py b/fluent.runtime/tests/__init__.py index 6ca983d6..9ad211b2 100644 --- a/fluent.runtime/tests/__init__.py +++ b/fluent.runtime/tests/__init__.py @@ -1,6 +1,7 @@ import os + # Unify path separator, default path separator on Windows is \ not / -# Needed because many tests uses dict + string compare to make a virtual file structure +# Needed in test_falllback.py because it uses dict + string compare to make a virtual file structure def normalize_path(path): - return "/".join(os.path.split(path)) \ No newline at end of file + return "/".join(os.path.split(path)) diff --git a/fluent.runtime/tests/test_fallback.py b/fluent.runtime/tests/test_fallback.py index 17de43c8..126e5aa8 100644 --- a/fluent.runtime/tests/test_fallback.py +++ b/fluent.runtime/tests/test_fallback.py @@ -27,7 +27,6 @@ def test_bundles(self, codecs_open, isfile): "en/one.ftl": "four = exists", "en/two.ftl": "five = exists", } - #isfile.side_effect = lambda p: p in data or ISFILE(p) isfile.side_effect = lambda p: normalize_path(p) in data or ISFILE(p) codecs_open.side_effect = lambda p, _, __: io.StringIO(data[normalize_path(p)]) l10n = FluentLocalization( From 096712f9f52d865af573800c03cbd2b17d3812df Mon Sep 17 00:00:00 2001 From: studyingegret <110317665+studyingegret@users.noreply.github.com> Date: Mon, 3 Feb 2025 22:53:29 +0800 Subject: [PATCH 05/12] suggestions from code review: Run on windows-2022 Co-authored-by: Eemeli Aro --- .github/workflows/ci.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8e93419f..841eae82 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,11 +23,10 @@ jobs: - run: python -m flake8 - run: python -m mypy fluent.syntax/fluent fluent.runtime/fluent test: - # Workaround Python 3.7 no longer supported https://github.com/actions/setup-python/issues/962#issuecomment-2414454450 - #runs-on: ubuntu-latest - runs-on: ubuntu-22.04 + runs-on: ${{ matrix.os }} strategy: matrix: + os: [ubuntu-22.04, windows-2022] python-version: [3.7, 3.8, 3.9, "3.10", 3.11, 3.12, pypy3.9, pypy3.10] steps: - uses: actions/checkout@v4 From 3cf815ffdfc2dc0295b95facaaaefce8aaae6da6 Mon Sep 17 00:00:00 2001 From: studyingegret <110317665+studyingegret@users.noreply.github.com> Date: Tue, 4 Feb 2025 16:41:12 +0800 Subject: [PATCH 06/12] Easy refactoring of file simulation --- fluent.runtime/tests/test_fallback.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/fluent.runtime/tests/test_fallback.py b/fluent.runtime/tests/test_fallback.py index 126e5aa8..8cc40d0b 100644 --- a/fluent.runtime/tests/test_fallback.py +++ b/fluent.runtime/tests/test_fallback.py @@ -10,6 +10,11 @@ ISFILE = os.path.isfile +def patch_io(codecs_open, isfile, data): + isfile.side_effect = lambda p: normalize_path(p) in data or ISFILE(p) + codecs_open.side_effect = lambda p, _, __: io.StringIO(data[normalize_path(p)]) + + class TestLocalization(unittest.TestCase): def test_init(self): l10n = FluentLocalization( @@ -27,8 +32,7 @@ def test_bundles(self, codecs_open, isfile): "en/one.ftl": "four = exists", "en/two.ftl": "five = exists", } - isfile.side_effect = lambda p: normalize_path(p) in data or ISFILE(p) - codecs_open.side_effect = lambda p, _, __: io.StringIO(data[normalize_path(p)]) + patch_io(codecs_open, isfile, data) l10n = FluentLocalization( ["de", "fr", "en"], ["one.ftl", "two.ftl"], FluentResourceLoader("{locale}") ) @@ -60,8 +64,7 @@ def test_all_exist(self, codecs_open, isfile): "en/one.ftl": "one = exists", "en/two.ftl": "two = exists", } - isfile.side_effect = lambda p: normalize_path(p) in data - codecs_open.side_effect = lambda p, _, __: io.StringIO(data[normalize_path(p)]) + patch_io(codecs_open, isfile, data) loader = FluentResourceLoader("{locale}") resources_list = list(loader.resources("en", ["one.ftl", "two.ftl"])) self.assertEqual(len(resources_list), 1) @@ -72,8 +75,7 @@ def test_one_exists(self, codecs_open, isfile): data = { "en/two.ftl": "two = exists", } - isfile.side_effect = lambda p: normalize_path(p) in data - codecs_open.side_effect = lambda p, _, __: io.StringIO(data[normalize_path(p)]) + patch_io(codecs_open, isfile, data) loader = FluentResourceLoader("{locale}") resources_list = list(loader.resources("en", ["one.ftl", "two.ftl"])) self.assertEqual(len(resources_list), 1) @@ -82,8 +84,7 @@ def test_one_exists(self, codecs_open, isfile): def test_none_exist(self, codecs_open, isfile): data = {} - isfile.side_effect = lambda p: normalize_path(p) in data - codecs_open.side_effect = lambda p, _, __: io.StringIO(data[normalize_path(p)]) + patch_io(codecs_open, isfile, data) loader = FluentResourceLoader("{locale}") resources_list = list(loader.resources("en", ["one.ftl", "two.ftl"])) self.assertEqual(len(resources_list), 0) From 457d6980c8e26882da92f49d9c46d15fb4b3ab9e Mon Sep 17 00:00:00 2001 From: studyingegret <110317665+studyingegret@users.noreply.github.com> Date: Wed, 5 Feb 2025 18:29:01 +0800 Subject: [PATCH 07/12] Refactor & add tests for file simulating code --- fluent.runtime/tests/__init__.py | 7 ---- fluent.runtime/tests/test_fallback.py | 59 +++++++++------------------ fluent.runtime/tests/test_utils.py | 39 ++++++++++++++++++ fluent.runtime/tests/utils.py | 56 +++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 46 deletions(-) create mode 100644 fluent.runtime/tests/test_utils.py diff --git a/fluent.runtime/tests/__init__.py b/fluent.runtime/tests/__init__.py index 9ad211b2..e69de29b 100644 --- a/fluent.runtime/tests/__init__.py +++ b/fluent.runtime/tests/__init__.py @@ -1,7 +0,0 @@ -import os - - -# Unify path separator, default path separator on Windows is \ not / -# Needed in test_falllback.py because it uses dict + string compare to make a virtual file structure -def normalize_path(path): - return "/".join(os.path.split(path)) diff --git a/fluent.runtime/tests/test_fallback.py b/fluent.runtime/tests/test_fallback.py index 8cc40d0b..a02d1951 100644 --- a/fluent.runtime/tests/test_fallback.py +++ b/fluent.runtime/tests/test_fallback.py @@ -1,19 +1,8 @@ -import io -import os import unittest -from unittest import mock - -from . import normalize_path +from .utils import patch_files from fluent.runtime import FluentLocalization, FluentResourceLoader -ISFILE = os.path.isfile - - -def patch_io(codecs_open, isfile, data): - isfile.side_effect = lambda p: normalize_path(p) in data or ISFILE(p) - codecs_open.side_effect = lambda p, _, __: io.StringIO(data[normalize_path(p)]) - class TestLocalization(unittest.TestCase): def test_init(self): @@ -22,17 +11,14 @@ def test_init(self): ) self.assertTrue(callable(l10n.format_value)) - @mock.patch("os.path.isfile") - @mock.patch("codecs.open") - def test_bundles(self, codecs_open, isfile): - data = { - "de/one.ftl": "one = in German", - "de/two.ftl": "two = in German", - "fr/two.ftl": "three = in French", - "en/one.ftl": "four = exists", - "en/two.ftl": "five = exists", - } - patch_io(codecs_open, isfile, data) + @patch_files({ + "de/one.ftl": "one = in German", + "de/two.ftl": "two = in German", + "fr/two.ftl": "three = in French", + "en/one.ftl": "four = exists", + "en/two.ftl": "five = exists", + }) + def test_bundles(self): l10n = FluentLocalization( ["de", "fr", "en"], ["one.ftl", "two.ftl"], FluentResourceLoader("{locale}") ) @@ -56,35 +42,30 @@ def test_bundles(self, codecs_open, isfile): self.assertEqual(l10n.format_value("five"), "exists") -@mock.patch("os.path.isfile") -@mock.patch("codecs.open") class TestResourceLoader(unittest.TestCase): - def test_all_exist(self, codecs_open, isfile): - data = { - "en/one.ftl": "one = exists", - "en/two.ftl": "two = exists", - } - patch_io(codecs_open, isfile, data) + @patch_files({ + "en/one.ftl": "one = exists", + "en/two.ftl": "two = exists", + }) + def test_all_exist(self): loader = FluentResourceLoader("{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) - def test_one_exists(self, codecs_open, isfile): - data = { - "en/two.ftl": "two = exists", - } - patch_io(codecs_open, isfile, data) + @patch_files({ + "en/two.ftl": "two = exists", + }) + def test_one_exists(self): loader = FluentResourceLoader("{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) - def test_none_exist(self, codecs_open, isfile): - data = {} - patch_io(codecs_open, isfile, data) + @patch_files({}) + def test_none_exist(self): loader = FluentResourceLoader("{locale}") resources_list = list(loader.resources("en", ["one.ftl", "two.ftl"])) self.assertEqual(len(resources_list), 0) diff --git a/fluent.runtime/tests/test_utils.py b/fluent.runtime/tests/test_utils.py new file mode 100644 index 00000000..e1c35922 --- /dev/null +++ b/fluent.runtime/tests/test_utils.py @@ -0,0 +1,39 @@ +import unittest +from .utils import patch_files +import os +import codecs + + +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): + 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) + 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 contents to match + """ + if expect_contents is None: + self.assertFalse(os.path.isfile(filename), + "Expected " + filename + " to not exist.") + else: + self.assertTrue(os.path.isfile(filename), + "Expected " + filename + " to exist.") + self.assertEqual(codecs.open(filename, "r", "utf-8").read(), + expect_contents) diff --git a/fluent.runtime/tests/utils.py b/fluent.runtime/tests/utils.py index 56301df6..9d5d3266 100644 --- a/fluent.runtime/tests/utils.py +++ b/fluent.runtime/tests/utils.py @@ -1,5 +1,61 @@ +"""Utilities for testing. + +Import this module before patching libraries. +""" + import textwrap +from pathlib import PurePath +from unittest import mock +from io import StringIO +import functools def dedent_ftl(text): return textwrap.dedent(f"{text.rstrip()}\n") + + +# Unify path separator, default path separator on Windows is \ not / +# Supports only relative paths +# Needed in test_falllback.py because it uses dict + string compare to make a virtual file structure +def _normalize_path(path): + path = PurePath(path) + if path.is_absolute(): + raise ValueError("Absolute paths are not supported in file simulation yet. (" + + str(path) + ")") + if "." not in path.parts and ".." not in path.parts: + return "/".join(PurePath(path).parts) + else: + res_parts = [] + length = len(path.parts) + i = 0 + while i < length: + if path.parts[i] == ".": + i += 1 + elif i < length - 1 and path.parts[i+1] == "..": + i += 2 + else: + res_parts.append(path.parts[i]) + i += 1 + return "/".join(res_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. + """ + if files is None: + files = {} + + def then(func): + @mock.patch("os.path.isfile", side_effect=lambda p: _normalize_path(p) in files) + @mock.patch("codecs.open", side_effect=lambda p, _, __: StringIO(files[_normalize_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 From 8d25d2f94065c0012e7a0d6a551aea32fff11f4f Mon Sep 17 00:00:00 2001 From: studyingegret <110317665+studyingegret@users.noreply.github.com> Date: Thu, 6 Feb 2025 14:29:50 +0800 Subject: [PATCH 08/12] Delete unnecessary notice --- fluent.runtime/tests/utils.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/fluent.runtime/tests/utils.py b/fluent.runtime/tests/utils.py index 9d5d3266..c4fff1f7 100644 --- a/fluent.runtime/tests/utils.py +++ b/fluent.runtime/tests/utils.py @@ -1,7 +1,4 @@ -"""Utilities for testing. - -Import this module before patching libraries. -""" +"""Utilities for testing.""" import textwrap from pathlib import PurePath From f7eb82877445288c7d6867035c6d634da5b88df2 Mon Sep 17 00:00:00 2001 From: studyingegret <110317665+studyingegret@users.noreply.github.com> Date: Thu, 6 Feb 2025 17:21:01 +0800 Subject: [PATCH 09/12] pr review: Apply suggestions --- fluent.runtime/tests/test_utils.py | 4 ++-- fluent.runtime/tests/utils.py | 23 +++++------------------ 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/fluent.runtime/tests/test_utils.py b/fluent.runtime/tests/test_utils.py index e1c35922..783c4f05 100644 --- a/fluent.runtime/tests/test_utils.py +++ b/fluent.runtime/tests/test_utils.py @@ -31,9 +31,9 @@ def assertFileIs(self, filename, expect_contents): """ if expect_contents is None: self.assertFalse(os.path.isfile(filename), - "Expected " + filename + " to not exist.") + f"Expected {filename} to not exist.") else: self.assertTrue(os.path.isfile(filename), - "Expected " + filename + " to exist.") + f"Expected {filename} to exist.") self.assertEqual(codecs.open(filename, "r", "utf-8").read(), expect_contents) diff --git a/fluent.runtime/tests/utils.py b/fluent.runtime/tests/utils.py index 9d5d3266..745234a7 100644 --- a/fluent.runtime/tests/utils.py +++ b/fluent.runtime/tests/utils.py @@ -18,25 +18,12 @@ def dedent_ftl(text): # Supports only relative paths # Needed in test_falllback.py because it uses dict + string compare to make a virtual file structure def _normalize_path(path): + """Note: Does not support absolute paths or paths that + contain '.' or '..' parts.""" path = PurePath(path) - if path.is_absolute(): - raise ValueError("Absolute paths are not supported in file simulation yet. (" - + str(path) + ")") - if "." not in path.parts and ".." not in path.parts: - return "/".join(PurePath(path).parts) - else: - res_parts = [] - length = len(path.parts) - i = 0 - while i < length: - if path.parts[i] == ".": - i += 1 - elif i < length - 1 and path.parts[i+1] == "..": - i += 2 - else: - res_parts.append(path.parts[i]) - i += 1 - return "/".join(res_parts) + if path.is_absolute() or "." in path.parts or ".." in path.parts: + raise ValueError(f"Unsupported path: {path}") + return "/".join(path.parts) def patch_files(files: dict): From ad8e7bb2a383df2f9a72416471ea08a16dc41fe1 Mon Sep 17 00:00:00 2001 From: studyingegret <110317665+studyingegret@users.noreply.github.com> Date: Thu, 6 Feb 2025 17:34:31 +0800 Subject: [PATCH 10/12] Use context manager --- fluent.runtime/tests/test_utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fluent.runtime/tests/test_utils.py b/fluent.runtime/tests/test_utils.py index 783c4f05..70778344 100644 --- a/fluent.runtime/tests/test_utils.py +++ b/fluent.runtime/tests/test_utils.py @@ -27,7 +27,7 @@ def patch_me(a, b): def assertFileIs(self, filename, expect_contents): """ expect_contents is None: Expect file does not exist - expect_contents is a str: Expect file contents to match + expect_contents is a str: Expect file to exist and contents to match """ if expect_contents is None: self.assertFalse(os.path.isfile(filename), @@ -35,5 +35,5 @@ def assertFileIs(self, filename, expect_contents): else: self.assertTrue(os.path.isfile(filename), f"Expected {filename} to exist.") - self.assertEqual(codecs.open(filename, "r", "utf-8").read(), - expect_contents) + with codecs.open(filename, "r", "utf-8") as f: + self.assertEqual(f.read(), expect_contents) From 0c9c67b53eb2f992425787824ca51495dafcf5b3 Mon Sep 17 00:00:00 2001 From: studyingegret <110317665+studyingegret@users.noreply.github.com> Date: Fri, 7 Feb 2025 08:13:40 +0800 Subject: [PATCH 11/12] Try to fix test errors on Unix --- fluent.runtime/tests/test_utils.py | 3 +++ fluent.runtime/tests/utils.py | 27 ++++++++++++++++----------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/fluent.runtime/tests/test_utils.py b/fluent.runtime/tests/test_utils.py index 70778344..32c0ab0e 100644 --- a/fluent.runtime/tests/test_utils.py +++ b/fluent.runtime/tests/test_utils.py @@ -22,6 +22,9 @@ def patch_me(a, b): self.assertFileIs("en\\two.txt", "Two") self.assertFileIs("en/three.txt", None) self.assertFileIs("en\\three.txt", None) + + with self.assertRaises(ValueError): + os.path.isfile("en/") patch_me(10, "b") def assertFileIs(self, filename, expect_contents): diff --git a/fluent.runtime/tests/utils.py b/fluent.runtime/tests/utils.py index 7f4463e6..403a985c 100644 --- a/fluent.runtime/tests/utils.py +++ b/fluent.runtime/tests/utils.py @@ -1,7 +1,7 @@ """Utilities for testing.""" import textwrap -from pathlib import PurePath +from pathlib import PureWindowsPath, PurePosixPath from unittest import mock from io import StringIO import functools @@ -11,16 +11,21 @@ def dedent_ftl(text): return textwrap.dedent(f"{text.rstrip()}\n") -# Unify path separator, default path separator on Windows is \ not / -# Supports only relative paths # Needed in test_falllback.py because it uses dict + string compare to make a virtual file structure -def _normalize_path(path): +def _normalize_file_path(path): """Note: Does not support absolute paths or paths that contain '.' or '..' parts.""" - path = PurePath(path) - if path.is_absolute() or "." in path.parts or ".." in path.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}") - return "/".join(path.parts) + parts = path.replace("\\", "/").split("/") + if "." in parts or ".." in parts: + # path ends with a trailing pathsep + raise ValueError(f"Unsupported path: {path}") + if parts and parts[-1] == "": + raise ValueError(f"Path appears to be a directory, not a file: {path}") + return "/".join(parts) def patch_files(files: dict): @@ -32,12 +37,12 @@ def patch_files(files: dict): The implementation may be changed to match the mechanism used. """ - if files is None: - files = {} + + # Here it is possible to validate file names, but skipped def then(func): - @mock.patch("os.path.isfile", side_effect=lambda p: _normalize_path(p) in files) - @mock.patch("codecs.open", side_effect=lambda p, _, __: StringIO(files[_normalize_path(p)])) + @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) From c98188517092bac19a1f206ccb4a355c4588de17 Mon Sep 17 00:00:00 2001 From: studyingegret <110317665+studyingegret@users.noreply.github.com> Date: Fri, 7 Feb 2025 08:13:40 +0800 Subject: [PATCH 12/12] (amend) Fix displaced comment --- fluent.runtime/tests/test_utils.py | 3 +++ fluent.runtime/tests/utils.py | 27 ++++++++++++++++----------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/fluent.runtime/tests/test_utils.py b/fluent.runtime/tests/test_utils.py index 70778344..32c0ab0e 100644 --- a/fluent.runtime/tests/test_utils.py +++ b/fluent.runtime/tests/test_utils.py @@ -22,6 +22,9 @@ def patch_me(a, b): self.assertFileIs("en\\two.txt", "Two") self.assertFileIs("en/three.txt", None) self.assertFileIs("en\\three.txt", None) + + with self.assertRaises(ValueError): + os.path.isfile("en/") patch_me(10, "b") def assertFileIs(self, filename, expect_contents): diff --git a/fluent.runtime/tests/utils.py b/fluent.runtime/tests/utils.py index 7f4463e6..fec04180 100644 --- a/fluent.runtime/tests/utils.py +++ b/fluent.runtime/tests/utils.py @@ -1,7 +1,7 @@ """Utilities for testing.""" import textwrap -from pathlib import PurePath +from pathlib import PureWindowsPath, PurePosixPath from unittest import mock from io import StringIO import functools @@ -11,16 +11,21 @@ def dedent_ftl(text): return textwrap.dedent(f"{text.rstrip()}\n") -# Unify path separator, default path separator on Windows is \ not / -# Supports only relative paths # Needed in test_falllback.py because it uses dict + string compare to make a virtual file structure -def _normalize_path(path): +def _normalize_file_path(path): """Note: Does not support absolute paths or paths that contain '.' or '..' parts.""" - path = PurePath(path) - if path.is_absolute() or "." in path.parts or ".." in path.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}") - return "/".join(path.parts) + 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): @@ -32,12 +37,12 @@ def patch_files(files: dict): The implementation may be changed to match the mechanism used. """ - if files is None: - files = {} + + # Here it is possible to validate file names, but skipped def then(func): - @mock.patch("os.path.isfile", side_effect=lambda p: _normalize_path(p) in files) - @mock.patch("codecs.open", side_effect=lambda p, _, __: StringIO(files[_normalize_path(p)])) + @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)