From a7e5ff00705e327930d5b328a60351b332eccb92 Mon Sep 17 00:00:00 2001 From: piegames Date: Mon, 26 May 2025 10:37:09 +0200 Subject: [PATCH] tests/functional2: Make symlink handling less confusing The current `RelativeTo` design is both more complex and more confusing than necessary. Its four variants are now reduced to only two. They are now also represented as different classes, to better communicate the difference in semantics and also intent. Change-Id: Ia60fc7a2dfa0f62bdef90dde347fd8603fd3fbf9 --- tests/functional2/README.md | 7 +- tests/functional2/lang/lang_util.py | 10 +-- tests/functional2/lang/test_lang_infra.py | 66 ++++++++----------- .../testlib/fixtures/file_helper.py | 62 +++++++---------- .../testlib/fixtures/test_file_helper.py | 44 +++++-------- 5 files changed, 71 insertions(+), 118 deletions(-) diff --git a/tests/functional2/README.md b/tests/functional2/README.md index 8ccf00aa3..9eb04a912 100644 --- a/tests/functional2/README.md +++ b/tests/functional2/README.md @@ -167,7 +167,8 @@ The input type is [FileDeclaration](./testlib/fixtures/file_helper.py), a dict f - `CopyFile("input/path")`: Copy the specified local file - `CopyTree("input/path")`: Like `CopyFile`, but recursively - `CopyTemplate("input/path", { "replace": "with"})`: Like `CopyFile`, but with an additional set of substitutions where each instance of "@key@" in the input file will be replaced by its associated value. -- `Symlink("target/path")` +- `Symlink("target/path")`: Create a symlink with the specified target. Therefore, relative paths are relative to the symlink's location. +- `AssetSymlink("source/path)`: Create a symlink pointing to a local asset file within the test suite. Paths must be relative and will be resolved relative to the current Python file. The created symlink will be absolute. ```python import pytest @@ -199,12 +200,12 @@ In order for the golden values to actually update within the code base (compared ```python import pytest -from functional2.testlib.fixtures.file_helper import Symlink, RelativeTo +from functional2.testlib.fixtures.file_helper import AssetSymlink @pytest.mark.parametrize( "files", # Setup the symlink so that the golden value will update - [ { "out": Symlink("assets/test_example/out.exp", RelativeTo.TEST), } ], + [ { "out": AssetSymlink("assets/test_example/out.exp"), } ], indirect=True, ) def test_example(files, snapshot): diff --git a/tests/functional2/lang/lang_util.py b/tests/functional2/lang/lang_util.py index 31fb3cf48..f68a859ea 100644 --- a/tests/functional2/lang/lang_util.py +++ b/tests/functional2/lang/lang_util.py @@ -8,7 +8,7 @@ from typing import Any import toml from toml import TomlDecodeError -from functional2.testlib.fixtures.file_helper import FileDeclaration, CopyFile, Symlink, RelativeTo +from functional2.testlib.fixtures.file_helper import FileDeclaration, CopyFile, AssetSymlink from functional2.testlib.utils import test_base_folder @@ -87,12 +87,8 @@ class LangTest: files = { "in.nix": CopyFile(f"{self.folder}/in{self.suffix}.nix"), "lib.nix": CopyFile("lib.nix"), - "out.exp": Symlink( - f"{self.folder}/{self.test_name}{self.suffix}.out.exp", relative_to=RelativeTo.TEST - ), - "err.exp": Symlink( - f"{self.folder}/{self.test_name}{self.suffix}.err.exp", relative_to=RelativeTo.TEST - ), + "out.exp": AssetSymlink(f"{self.folder}/{self.test_name}{self.suffix}.out.exp"), + "err.exp": AssetSymlink(f"{self.folder}/{self.test_name}{self.suffix}.err.exp"), } for file in self.extra_files: # Make sure to add the extra-files requested by the test.toml diff --git a/tests/functional2/lang/test_lang_infra.py b/tests/functional2/lang/test_lang_infra.py index 02c1ad00e..1a2d275a1 100644 --- a/tests/functional2/lang/test_lang_infra.py +++ b/tests/functional2/lang/test_lang_infra.py @@ -4,7 +4,7 @@ from textwrap import dedent import pytest from functional2.testlib.commands import Command -from functional2.testlib.fixtures.file_helper import File, RelativeTo, Symlink +from functional2.testlib.fixtures.file_helper import File, AssetSymlink from functional2.testlib.fixtures.snapshot import Snapshot from functional2.testlib.utils import get_functional2_lang_files @@ -111,13 +111,11 @@ def test_skips_py_files(files: Path, pytest_command: Command): "n_suffix": { "in-1.nix": File("{}"), "in-2.nix": File("{}"), - "eval-okay-1.out.exp": Symlink( - "assets/test_lang_infra/runner_eo.out.exp", - relative_to=RelativeTo.TEST, + "eval-okay-1.out.exp": AssetSymlink( + "assets/test_lang_infra/runner_eo.out.exp" ), - "eval-okay-2.out.exp": Symlink( - "assets/test_lang_infra/runner_eo.out.exp", - relative_to=RelativeTo.TEST, + "eval-okay-2.out.exp": AssetSymlink( + "assets/test_lang_infra/runner_eo.out.exp" ), } } @@ -148,13 +146,11 @@ def test_collects_with_n_suffix(pytest_command: Command): "short_string_suffix": { "in-speaker.nix": File("{}"), "in-microphone.nix": File("{}"), - "eval-okay-speaker.out.exp": Symlink( - "assets/test_lang_infra/runner_eo.out.exp", - relative_to=RelativeTo.TEST, + "eval-okay-speaker.out.exp": AssetSymlink( + "assets/test_lang_infra/runner_eo.out.exp" ), - "eval-okay-microphone.out.exp": Symlink( - "assets/test_lang_infra/runner_eo.out.exp", - relative_to=RelativeTo.TEST, + "eval-okay-microphone.out.exp": AssetSymlink( + "assets/test_lang_infra/runner_eo.out.exp" ), } } @@ -185,13 +181,11 @@ def test_collects_short_string_suffix(pytest_command: Command): "dash_suffix": { "in-string-with-dash.nix": File("{}"), "in-some-more--dashes.nix": File("{}"), - "eval-okay-string-with-dash.out.exp": Symlink( - "assets/test_lang_infra/runner_eo.out.exp", - relative_to=RelativeTo.TEST, + "eval-okay-string-with-dash.out.exp": AssetSymlink( + "assets/test_lang_infra/runner_eo.out.exp" ), - "eval-okay-some-more--dashes.out.exp": Symlink( - "assets/test_lang_infra/runner_eo.out.exp", - relative_to=RelativeTo.TEST, + "eval-okay-some-more--dashes.out.exp": AssetSymlink( + "assets/test_lang_infra/runner_eo.out.exp" ), } } @@ -223,13 +217,11 @@ def test_collects_string_suffix_with_dash(pytest_command: Command): "bad_naming": { "in-&x.nix": File("{}"), "in-.nix": File("{}"), - "eval-okay-&x.out.exp": Symlink( - "assets/test_lang_infra/runner_eo.out.exp", - relative_to=RelativeTo.TEST, + "eval-okay-&x.out.exp": AssetSymlink( + "assets/test_lang_infra/runner_eo.out.exp" ), - "eval-okay-.out.exp": Symlink( - "assets/test_lang_infra/runner_eo.out.exp", - relative_to=RelativeTo.TEST, + "eval-okay-.out.exp": AssetSymlink( + "assets/test_lang_infra/runner_eo.out.exp" ), } } @@ -260,24 +252,20 @@ def test_collection_fails_with_bad_naming(pytest_command: Command): "lang": { "infra_okay_runners": { "in.nix": File("{}"), - "eval-okay.out.exp": Symlink( - "assets/test_lang_infra/runner_eo.out.exp", - relative_to=RelativeTo.TEST, + "eval-okay.out.exp": AssetSymlink( + "assets/test_lang_infra/runner_eo.out.exp" ), - "parse-okay.out.exp": Symlink( - "assets/test_lang_infra/runner_po.out.exp", - relative_to=RelativeTo.TEST, + "parse-okay.out.exp": AssetSymlink( + "assets/test_lang_infra/runner_po.out.exp" ), }, "infra_fail_runners": { "in.nix": File("{"), - "eval-fail.err.exp": Symlink( - "assets/test_lang_infra/runner_ef.err.exp", - relative_to=RelativeTo.TEST, + "eval-fail.err.exp": AssetSymlink( + "assets/test_lang_infra/runner_ef.err.exp" ), - "parse-fail.err.exp": Symlink( - "assets/test_lang_infra/runner_pf.err.exp", - relative_to=RelativeTo.TEST, + "parse-fail.err.exp": AssetSymlink( + "assets/test_lang_infra/runner_pf.err.exp" ), }, } @@ -495,9 +483,7 @@ def test_invalid_toml(pytest_command: Command): "update_test": {"in.nix": File("{}"), "eval-okay.out.exp": File("old")} } }, - "out.exp": Symlink( - "assets/test_lang_infra/runner_eo.out.exp", relative_to=RelativeTo.TEST - ), + "out.exp": AssetSymlink("assets/test_lang_infra/runner_eo.out.exp"), } ), (["-k", "update_test", "--accept-tests"], False), diff --git a/tests/functional2/testlib/fixtures/file_helper.py b/tests/functional2/testlib/fixtures/file_helper.py index 16af1cd1e..cbab611a5 100644 --- a/tests/functional2/testlib/fixtures/file_helper.py +++ b/tests/functional2/testlib/fixtures/file_helper.py @@ -1,6 +1,5 @@ import shutil from abc import ABC, abstractmethod -from enum import StrEnum from pathlib import Path from typing import Any @@ -103,54 +102,37 @@ class CopyTemplate(_ByContentFileish): return self.content -class RelativeTo(StrEnum): - TEST = "test" - """ - Base for the given path is the directory of the test - """ - TARGET = "target" - """ - Base for the given path is the directory of the target / where all files are created - """ - ROOT = "root" - """ - Base for the given path is the root folder (/) - """ - SELF = "self" - """ - No base path is given, used to create relative symlinks (e.g. `"../test"`) - """ - - class Symlink(Fileish): - def __init__(self, source: str, relative_to: RelativeTo = RelativeTo.TARGET): + def __init__(self, target: str): """ - Declares a file as a symlink to a different location + Declares a file as a symlink to a different location. + + NOTE: due to limitations of symlinks on Windows, tests using this might be flaky and fail!! + :param target: Path to the target where the symlink should be pointing + """ + self.target = target + + def copy_to(self, path: Path, _origin: Path): + path.symlink_to(self.target) + + +class AssetSymlink(Fileish): + def __init__(self, source: str): + """ + Declares a file as a symlink to a local asset file. NOTE: due to limitations of symlinks on Windows, tests using this might be flakey and fail!! - :param source: Path to the source where the symlink should be pointing - :raise ValueError: When the given source is an absolute Path, but relative to wasn't set to ROOT + :param source: Path to the source where the symlink should be pointing. + Paths are relative to the current test's module. + :raise ValueError: When the given source path is absolute """ self.source = source - self.relative_to = relative_to def copy_to(self, path: Path, origin: Path): - if self.relative_to is not RelativeTo.ROOT and Path(self.source).is_absolute(): - msg = "absolute paths are only supported when using RelativeTo.ROOT" + if Path(self.source).is_absolute(): + msg = "absolute paths are not allowed" raise ValueError(msg) - match self.relative_to: - case RelativeTo.TEST: - base = origin - case RelativeTo.TARGET: - base = path.parent - case RelativeTo.ROOT: - base = Path("/") - case RelativeTo.SELF: - base = None - case _: - msg = f"unknown relativity {self.relative_to}" - raise ValueError(msg) - target_path = base / self.source if base is not None else Path(self.source) + target_path = origin / self.source path.symlink_to(target_path) diff --git a/tests/functional2/testlib/fixtures/test_file_helper.py b/tests/functional2/testlib/fixtures/test_file_helper.py index 4ecbe85ad..565f2b16b 100644 --- a/tests/functional2/testlib/fixtures/test_file_helper.py +++ b/tests/functional2/testlib/fixtures/test_file_helper.py @@ -8,7 +8,7 @@ from functional2.testlib.fixtures.file_helper import ( CopyTree, File, Symlink, - RelativeTo, + AssetSymlink, ) @@ -190,11 +190,9 @@ def test_mode_setting(files: Path): @pytest.mark.parametrize( - "files", - [{"a": Symlink("assets/test_file_helper/copy_file_test.txt", RelativeTo.TEST)}], - indirect=True, + "files", [{"a": AssetSymlink("assets/test_file_helper/copy_file_test.txt")}], indirect=True ) -def test_file_symlink(files: Path): +def test_asset_symlink(files: Path): file = files / "a" assert file.exists(follow_symlinks=False) @@ -206,23 +204,7 @@ def test_file_symlink(files: Path): @pytest.mark.parametrize( - "files", - [{"a": Symlink("test_folder/b.txt", RelativeTo.TARGET), "test_folder": {"b.txt": File("zzz")}}], - indirect=True, -) -def test_file_symlink_target(files: Path): - file = files / "a" - assert file.exists(follow_symlinks=False) - - assert file.is_symlink() - - assert file.readlink().exists() - - assert file.read_text() == "zzz" - - -@pytest.mark.parametrize( - "files", [{"a": Symlink("assets/test_file_helper/test_folder", RelativeTo.TEST)}], indirect=True + "files", [{"a": AssetSymlink("assets/test_file_helper/test_folder")}], indirect=True ) def test_dir_symlink(files: Path): folder = files / "a" @@ -238,23 +220,29 @@ def test_dir_symlink(files: Path): @pytest.mark.parametrize( - "files", - [{"a": Symlink("assets/test_file_helper/this_does_not_exist", RelativeTo.TEST)}], - indirect=True, + "files", [{"a": AssetSymlink("assets/test_file_helper/this_does_not_exist")}], indirect=True ) -def test_invalid_symlink(files: Path): +def test_invalid_asset_symlink(files: Path): link = files / "a" assert link.exists(follow_symlinks=False) assert link.is_symlink() assert not link.readlink().exists() +@pytest.mark.xfail(raises=ValueError) @pytest.mark.parametrize( "files", - [{"tg": File("Hello World"), "folder": {"link": Symlink("../tg", RelativeTo.SELF)}}], + [{"a": AssetSymlink("/absolute/assets/test_file_helper/this_does_not_exist")}], indirect=True, ) -def test_direct_relativity_symlink(files: Path): +def test_absolute_asset_symlink(files: Path): + pass + + +@pytest.mark.parametrize( + "files", [{"tg": File("Hello World"), "folder": {"link": Symlink("../tg")}}], indirect=True +) +def test_file_symlink(files: Path): assert (files / "tg").exists() link = files / "folder" / "link" assert link.exists(follow_symlinks=False)