Skip to content

Commit

Permalink
Fix various testing and linting issues
Browse files Browse the repository at this point in the history
  • Loading branch information
Martin Møldrup committed Jan 26, 2025
1 parent ba5e000 commit 0c13dfd
Show file tree
Hide file tree
Showing 8 changed files with 94 additions and 35 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/continuous_integration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jobs:
run: poetry install --no-interaction

- name: Run ruff
run: poetry run ruff check snappylapy
run: poetry run ruff check snappylapy --output-format=github

- name: Run mypy
run: poetry run mypy snappylapy
Expand Down
1 change: 1 addition & 0 deletions ruff.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ indent-width = 4

# Assume Python 3.9
target-version = "py39"
output-format = "concise"

[lint]
preview = true
Expand Down
33 changes: 26 additions & 7 deletions snappylapy/_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ def init() -> None:
# Check if already in .gitignore
with gitignore_path.open("r") as file:
lines = file.readlines()
regex = re.compile(rf"^{re.escape(directory_names.test_results_dir_name)}(/|$)")
regex = re.compile(
rf"^{re.escape(directory_names.test_results_dir_name)}(/|$)")
if any(regex.match(line) for line in lines):
typer.echo("Already in .gitignore.")
return
Expand All @@ -33,47 +34,65 @@ def init() -> None:
with gitignore_path.open("w") as file:
file.write(line_to_add)
file.writelines(lines)
typer.echo(f"Added {directory_names.test_results_dir_name}/ to .gitignore.")
typer.echo(
f"Added {directory_names.test_results_dir_name}/ to .gitignore.")


@app.command()
def clear(force: bool = typer.Option(False, "--force", "-f", help="Force deletion without confirmation")) -> None:
def clear(force: bool = typer.Option(

Check failure on line 42 in snappylapy/_cli.py

View workflow job for this annotation

GitHub Actions / lint_and_type_check

Ruff (FBT001)

snappylapy/_cli.py:42:11: FBT001 Boolean-typed positional argument in function definition
False,

Check failure on line 43 in snappylapy/_cli.py

View workflow job for this annotation

GitHub Actions / lint_and_type_check

Ruff (FBT003)

snappylapy/_cli.py:43:5: FBT003 Boolean positional value in function call
"--force",
"-f",
help="Force deletion without confirmation",
)) -> None:
"""Clear all test results and snapshots, recursively, using pathlib."""
list_of_files_to_delete = get_files_to_delete()
if not list_of_files_to_delete:
typer.echo("No files to delete.")
return
if not force:
# Ask for confirmation
typer.secho("\nAre you sure you want to delete all test results and snapshots?", fg=typer.colors.BRIGHT_BLUE)
response = typer.prompt("Type 'yes' to confirm, anything else to abort.")
typer.secho(
"\nAre you sure you want to delete all test results and snapshots?",
fg=typer.colors.BRIGHT_BLUE)
response = typer.prompt(
"Type 'yes' to confirm, anything else to abort.")
if response.lower() != "yes":
typer.echo("Aborted.")
return
# Delete files
delete_files(list_of_files_to_delete)
typer.echo(f"Deleted {len(list_of_files_to_delete)} files.")


def get_files_to_delete() -> list[pathlib.Path]:
"""Get list of files to delete."""
list_of_files_to_delete: list[pathlib.Path] = []
for dir_name in [directory_names.test_results_dir_name, directory_names.snapshot_dir_name]:
for dir_name in [
directory_names.test_results_dir_name,
directory_names.snapshot_dir_name,
]:
for root_dir in pathlib.Path().rglob(dir_name):
for file in root_dir.iterdir():
if file.is_file():
list_of_files_to_delete.append(file)
typer.echo(f"Found file to delete: {file}")
return list_of_files_to_delete


def delete_files(list_of_files_to_delete: list[pathlib.Path]) -> None:
"""Delete files."""
# Delete files
for file in list_of_files_to_delete:
file.unlink()
# Delete directories
for dir_name in [directory_names.test_results_dir_name, directory_names.snapshot_dir_name]:
for dir_name in [
directory_names.test_results_dir_name,
directory_names.snapshot_dir_name,
]:
for root_dir in pathlib.Path().rglob(dir_name):
root_dir.rmdir()


if __name__ == "__main__":
app()
85 changes: 60 additions & 25 deletions snappylapy/_plugin.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
"""Pytest plugin for snapshot testing."""
from __future__ import annotations

import os
import pathlib
from typing import Any
import re
import pytest
import pathlib
import _pytest.mark
from collections.abc import Callable
from snappylapy import Expect, LoadSnapshot
from snappylapy.constants import DEFEAULT_SNAPSHOT_BASE_DIR
from snappylapy.fixtures import Settings
from snappylapy.session import SnapshotSession
from snappylapy.constants import DEFEAULT_SNAPSHOT_BASE_DIR
import re
from typing import Any


def _get_kwargs_from_depend_function(depends_function, marker_name: str, kwags_key: str) -> Any:
def _get_kwargs_from_depend_function(
depends_function: Callable,
marker_name: str,
kwags_key: str,
) -> str | None:
"""Get a test function with a pytest marker assigned and get a value from the marker."""
if not hasattr(depends_function, "pytestmark"):
return None
Expand All @@ -21,7 +28,11 @@ def _get_kwargs_from_depend_function(depends_function, marker_name: str, kwags_k
return mark.kwargs.get(kwags_key, None)
return None

def _get_args_from_depend_function(depends_function, marker_name: str) -> Any:

def _get_args_from_depend_function(
depends_function: Callable,
marker_name: str,
) -> tuple[Any] | None:
"""Get a test function with a pytest marker assigned and get a value from the marker."""
if not hasattr(depends_function, "pytestmark"):
return None
Expand All @@ -31,12 +42,13 @@ def _get_args_from_depend_function(depends_function, marker_name: str) -> Any:
return mark.args
return None


@pytest.fixture
def snappylapy_settings(request: pytest.FixtureRequest) -> Settings:
"""Initialize the Settings object for the test."""
update_snapshots = request.config.getoption("--snapshot-update")
marker = request.node.get_closest_marker("snappylapy")
match = re.search(r'\[(.*?)\]', request.node.name)
match = re.search(r"\[(.*?)\]", request.node.name)
param_name: str | None = match.group(1) if match else None
settings = Settings(
test_filename=request.module.__name__,
Expand All @@ -48,16 +60,17 @@ def snappylapy_settings(request: pytest.FixtureRequest) -> Settings:
output_dir = marker.kwargs.get("output_dir", None)
if output_dir:
settings.snapshots_base_dir = output_dir
path_output_dir: None | pathlib.Path = None
if hasattr(request, 'param'):
path_output_dir: pathlib.Path | None = None
if hasattr(request, "param"):
path_output_dir = request.param
settings.depending_snapshots_base_dir = path_output_dir
settings.snapshots_base_dir = path_output_dir
settings.custom_name = path_output_dir.name
# If not parametrized, get the depends from the marker
depends = marker.kwargs.get("depends", []) if marker else []
if depends:
input_dir_from_depends = _get_kwargs_from_depend_function(depends[0], "snappylapy", "output_dir")
input_dir_from_depends = _get_kwargs_from_depend_function(
depends[0], "snappylapy", "output_dir")
if input_dir_from_depends:
path_output_dir = pathlib.Path(input_dir_from_depends)
settings.depending_test_filename = depends[0].__module__
Expand All @@ -67,7 +80,8 @@ def snappylapy_settings(request: pytest.FixtureRequest) -> Settings:


@pytest.fixture
def expect(request: pytest.FixtureRequest, snappylapy_settings: Settings) -> Expect:
def expect(request: pytest.FixtureRequest,
snappylapy_settings: Settings) -> Expect:
"""Initialize the snapshot object with update_snapshots flag from pytest option."""
snappylapy_session: SnapshotSession = request.config.snappylapy_session # type: ignore[attr-defined]
return Expect(
Expand All @@ -77,7 +91,8 @@ def expect(request: pytest.FixtureRequest, snappylapy_settings: Settings) -> Exp


@pytest.fixture
def load_snapshot(request: pytest.FixtureRequest, snappylapy_settings: Settings) -> LoadSnapshot:
def load_snapshot(_: pytest.FixtureRequest,
snappylapy_settings: Settings) -> LoadSnapshot:
"""Initialize the LoadSnapshot object."""
return LoadSnapshot(snappylapy_settings)

Expand Down Expand Up @@ -130,63 +145,83 @@ def pytest_addoption(parser: pytest.Parser) -> None:

def pytest_sessionstart(session: pytest.Session) -> None:
"""Initialize the snapshot session."""
session.config.snappylapy_session = SnapshotSession() # type: ignore[attr-defined]
session.config.snappylapy_session = SnapshotSession(
) # type: ignore[attr-defined]


class ExceptionDuringTestSetupError(Exception):
"""Error raised when an exception is raised during the setup of the tests."""

class ReturnError():

class ReturnError:
"""When an exception is raised during the setup of the tests, raise an exception when trying to access the attribute."""

Check failure on line 157 in snappylapy/_plugin.py

View workflow job for this annotation

GitHub Actions / lint_and_type_check

Ruff (E501)

snappylapy/_plugin.py:157:121: E501 Line too long (124 > 120)

def __init__(self, exception: Exception, message: str) -> None:
self._message = message
self._exception = exception

def __getattribute__(self, name: str):
def __getattribute__(self, name: str) -> Any:

Check failure on line 163 in snappylapy/_plugin.py

View workflow job for this annotation

GitHub Actions / lint_and_type_check

Ruff (ANN401)

snappylapy/_plugin.py:163:46: ANN401 Dynamically typed expressions (typing.Any) are disallowed in `__getattribute__`
"""If an exception was raised during the setup of the tests, raise an exception when trying to access the attribute."""

Check failure on line 164 in snappylapy/_plugin.py

View workflow job for this annotation

GitHub Actions / lint_and_type_check

Ruff (E501)

snappylapy/_plugin.py:164:121: E501 Line too long (127 > 120)
exception = object.__getattribute__(self, "_exception")
if exception is not None and os.getenv("PYTEST_CURRENT_TEST"):
exception_message = f"When during setup of the tests an error was raised: {exception}"
if self._message:
exception_message = f"{self._message} {exception_message}"
raise ExceptionDuringTestSetupError() from exception
raise ExceptionDuringTestSetupError from exception
return object.__getattribute__(self, name)


@pytest.fixture
def test_directory(request: pytest.FixtureRequest, snappylapy_settings: Settings) -> pathlib.Path:
def test_directory(_: pytest.FixtureRequest,

Check failure on line 175 in snappylapy/_plugin.py

View workflow job for this annotation

GitHub Actions / lint_and_type_check

Ruff (PT019)

snappylapy/_plugin.py:175:20: PT019 Fixture `_` without value is injected as parameter, use `@pytest.mark.usefixtures` instead
snappylapy_settings: Settings) -> pathlib.Path:
"""Get the test directory for the test. Raise a better error message if the fixture is not parametrized."""
try:
return snappylapy_settings.snapshots_base_dir
except Exception as e:
error_msg = "The test_directory fixture is not parametrized, please add the snappylapy marker to the test, e.g. @pytest.mark.snappylapy(foreach_folder_in='test_data')"

Check failure on line 181 in snappylapy/_plugin.py

View workflow job for this annotation

GitHub Actions / lint_and_type_check

Ruff (E501)

snappylapy/_plugin.py:181:121: E501 Line too long (175 > 120)
raise Exception(error_msg) from e

Check failure on line 182 in snappylapy/_plugin.py

View workflow job for this annotation

GitHub Actions / lint_and_type_check

Ruff (TRY002)

snappylapy/_plugin.py:182:15: TRY002 Create your own exception


def pytest_generate_tests(metafunc: pytest.Metafunc) -> None:
"""Generate parametrized tests for the pipeline output and input."""
marker = metafunc.definition.get_closest_marker("snappylapy")
if not marker:
return
foreach_folder_in: str | pathlib.Path = marker.kwargs.get("foreach_folder_in", None)
foreach_folder_in: str | pathlib.Path = marker.kwargs.get(
"foreach_folder_in", None)
if foreach_folder_in:
test_cases = [p for p in pathlib.Path(foreach_folder_in).iterdir() if p.is_dir()]
test_cases = [
p for p in pathlib.Path(foreach_folder_in).iterdir() if p.is_dir()
]
ids = [p.name for p in test_cases]
metafunc.parametrize("snappylapy_settings", test_cases, indirect=True, ids=ids)
metafunc.parametrize("snappylapy_settings",
test_cases,
indirect=True,
ids=ids)
depends = marker.kwargs.get("depends", []) if marker else []
if depends:
function_depends = marker.kwargs['depends'][0]
function_depends = marker.kwargs["depends"][0]
if not hasattr(function_depends, "pytestmark"):
return
function_depends_marker: _pytest.mark.structures.Mark = function_depends.pytestmark[0]
function_depends_marker: _pytest.mark.structures.Mark = function_depends.pytestmark[
0]
# It might be parametrized
# Example: Mark(name='parametrize', args=('test_directory', ['test_data/case1', 'test_data/case2']), kwargs={})

Check failure on line 209 in snappylapy/_plugin.py

View workflow job for this annotation

GitHub Actions / lint_and_type_check

Ruff (ERA001)

snappylapy/_plugin.py:209:9: ERA001 Found commented-out code
# Parametize the snappylapy_settings fixture
# if function_depends_marker.name == "parametrize":
# ids = function_depends_marker.kwargs.get("ids", None)

Check failure on line 212 in snappylapy/_plugin.py

View workflow job for this annotation

GitHub Actions / lint_and_type_check

Ruff (ERA001)

snappylapy/_plugin.py:212:9: ERA001 Found commented-out code
# metafunc.parametrize("snappylapy_settings", function_depends_marker.args[1], indirect=True, ids=ids)
if function_depends_marker.name == "snappylapy":
foreach_folder_in = _get_kwargs_from_depend_function(depends[0], "snappylapy", "foreach_folder_in")
foreach_folder_in = _get_kwargs_from_depend_function(
depends[0], "snappylapy", "foreach_folder_in")
if not foreach_folder_in:
return
test_cases = [p for p in pathlib.Path(foreach_folder_in).iterdir() if p.is_dir()]
test_cases = [
p for p in pathlib.Path(foreach_folder_in).iterdir()
if p.is_dir()
]
ids = [p.name for p in test_cases]
metafunc.parametrize("snappylapy_settings", test_cases, indirect=True, ids=ids)
metafunc.parametrize("snappylapy_settings",
test_cases,
indirect=True,
ids=ids)
3 changes: 3 additions & 0 deletions snappylapy/constants.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
"""Read-only constants."""
import pathlib

DEFEAULT_SNAPSHOT_BASE_DIR = pathlib.Path()


class DirectoryNames:
"""Class to enforce immutable directory names, since there is side effect if they are changed."""

Expand Down
3 changes: 1 addition & 2 deletions snappylapy/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"""
from __future__ import annotations

import pathlib
from .expectation_classes import (
BytesExpect,
DictExpect,
Expand All @@ -23,9 +22,9 @@
JsonPickleSerializer,
StringSerializer,
)
from snappylapy.constants import directory_names
from snappylapy.session import SnapshotSession
from typing import Any
from snappylapy.constants import directory_names


class Expect:
Expand Down
1 change: 1 addition & 0 deletions test_data/case1/hello.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hallo world
1 change: 1 addition & 0 deletions test_data/case2/galaxy.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello Galaxy!

0 comments on commit 0c13dfd

Please sign in to comment.