patrick/chitai
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore: bring the test suite up to ruff's standards

Browse Source 
Mostly automatic: empty f-strings, unused imports, formatting. The manual half
was duplicated import blocks stranded mid-file and `== None` assertions, which
become `is None` here because they compare plain attributes -- unlike the
identical rule in filters/book.py, where they build SQL.

Two unused variables are the tests never checking the disk in either
delete_files direction. Left in place under a noqa so the gap stays visible.
This commit is contained in:
patrick
2026-08-17 20:56:24 -04:00
parent 0c25f63600
commit 5ec5a4d334
15 changed files with 232 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/tests/unit/test_filesystem_library.py
+26 -11
View File
@@ -2,7 +2,10 @@
from pathlib import Path
from chitai.services.filesystem_library import BookPathGenerator, sanitize_path_component
from chitai.services.filesystem_library import (
BookPathGenerator,
sanitize_path_component,
)
ROOT = Path("/library")
@@ -23,12 +26,15 @@ def test_a_book_with_no_authors() -> None:
def test_a_series_adds_a_level_and_pads_the_position() -> None:
assert path_for(
title="Persepolis Rising",
authors=["James S. A. Corey"],
series="The Expanse",
series_position="7",
) == ROOT / "James S. A. Corey" / "The Expanse" / "07 - Persepolis Rising"
assert (
path_for(
title="Persepolis Rising",
authors=["James S. A. Corey"],
series="The Expanse",
series_position="7",
)
== ROOT / "James S. A. Corey" / "The Expanse" / "07 - Persepolis Rising"
)
def test_a_slash_in_a_title_does_not_add_a_directory() -> None:
@@ -43,16 +49,25 @@ def test_a_slash_in_a_title_does_not_add_a_directory() -> None:
generated = path_for(title="Back in Black: AC/DC", authors=["Murray Engleheart"])
assert generated == ROOT / "Murray Engleheart" / "Back in Black: AC_DC"
assert generated.relative_to(ROOT).parts == ("Murray Engleheart", "Back in Black: AC_DC")
assert generated.relative_to(ROOT).parts == (
"Murray Engleheart",
"Back in Black: AC_DC",
)
def test_a_slash_in_an_author_or_series_is_handled_too() -> None:
assert path_for(title="Split", authors=["A/B Collective"]) == (
ROOT / "A_B Collective" / "Split"
)
assert path_for(
title="Volume One", authors=["Someone"], series="Either/Or", series_position="1"
) == ROOT / "Someone" / "Either_Or" / "01 - Volume One"
assert (
path_for(
title="Volume One",
authors=["Someone"],
series="Either/Or",
series_position="1",
)
== ROOT / "Someone" / "Either_Or" / "01 - Volume One"
)
def test_control_characters_are_removed() -> None:
backend/tests/unit/test_matching.py
+13 -5
View File
@@ -54,7 +54,8 @@ class TestNormalizeTitle:
assert normalize_title("The") == "the"
@pytest.mark.parametrize(
"title", ["Catch 22", "Fahrenheit 451", "Blade Runner 2049", "1984", "Apollo 13"]
"title",
["Catch 22", "Fahrenheit 451", "Blade Runner 2049", "1984", "Apollo 13"],
)
def test_a_number_is_not_an_edition(self, title: str) -> None:
"""Edition stripping keys on the `e`; a bare number is part of the title."""
@@ -163,8 +164,13 @@ class TestNormalizeIdentifier:
def test_uuids_are_refused(self) -> None:
"""Generated per build, so they only re-find what the hash check catches."""
assert normalize_identifier("uuid", "3f2b1c4e-1111-2222-3333-444455556666") is None
assert normalize_identifier("urn:uuid", "3f2b1c4e-1111-2222-3333-444455556666") is None
assert (
normalize_identifier("uuid", "3f2b1c4e-1111-2222-3333-444455556666") is None
)
assert (
normalize_identifier("urn:uuid", "3f2b1c4e-1111-2222-3333-444455556666")
is None
)
def test_other_schemes_keep_their_own_key(self) -> None:
assert normalize_identifier("asin", "B000FC0PDA") == "asin:b000fc0pda"
@@ -187,7 +193,9 @@ class TestIsbnConversion:
assert isbn10_to_isbn13("043942089X") == "9780439420891"
@pytest.mark.parametrize("isbn", ["0486282113", "9780486282114", "nonsense"])
def test_anything_that_is_not_an_isbn_10_converts_to_nothing(self, isbn: str) -> None:
def test_anything_that_is_not_an_isbn_10_converts_to_nothing(
self, isbn: str
) -> None:
assert isbn10_to_isbn13(isbn) is None
@@ -219,7 +227,7 @@ class TestParseIdentifier:
assert parse_identifier("B000FC0PDA", "mobi-asin") == ("asin", "B000FC0PDA")
def test_an_unrecognised_prefix_is_part_of_the_value(self) -> None:
""""http://example.com/book" is not an identifier called "http"."""
""" "http://example.com/book" is not an identifier called "http"."""
assert parse_identifier("http://www.gutenberg.org/5200") == (
"id",
"http://www.gutenberg.org/5200",
backend/tests/unit/test_metadata_extractor.py
+13 -5
View File
@@ -22,7 +22,6 @@ class TestEpubExtractor:
assert metadata["published_date"] == date(year=2001, month=7, day=1)
EPUB = Path("tests/data_files/Metamorphosis - Franz Kafka.epub")
PDF = Path("tests/data_files/Calculus Made Easy - Silvanus Thompson.pdf")
@@ -31,7 +30,9 @@ PDF = Path("tests/data_files/Calculus Made Easy - Silvanus Thompson.pdf")
class TestIdentifierMerging:
"""A book's formats each contribute identifiers; none of them replaces the rest."""
async def test_every_format_contributes(self, monkeypatch: pytest.MonkeyPatch) -> None:
async def test_every_format_contributes(
self, monkeypatch: pytest.MonkeyPatch
) -> None:
"""
Identifiers are a collection, not a single value.
@@ -47,7 +48,9 @@ class TestIdentifierMerging:
}
async def pdf(_file):
return {"identifiers": {"isbn-13": "9781718500402", "isbn-10": "1593270356"}}
return {
"identifiers": {"isbn-13": "9781718500402", "isbn-10": "1593270356"}
}
monkeypatch.setattr(EpubExtractor, "extract_metadata", epub)
monkeypatch.setattr(PdfExtractor, "extract_metadata", pdf)
@@ -133,7 +136,9 @@ class TestSplitEdition:
),
],
)
def test_editions_are_split_out(self, title: str, stripped: str, edition: int) -> None:
def test_editions_are_split_out(
self, title: str, stripped: str, edition: int
) -> None:
assert split_edition(title) == (stripped, edition)
@pytest.mark.parametrize(
@@ -171,7 +176,10 @@ class TestEditionFromFiles:
"""The PDF fixture calls itself a 2nd edition in its own metadata title."""
metadata = await Extractor.extract_metadata([PDF])
assert metadata["title"] == "The Project Gutenberg eBook #33283: Calculus Made Easy"
assert (
metadata["title"]
== "The Project Gutenberg eBook #33283: Calculus Made Easy"
)
assert metadata["edition"] == 2
backend/tests/unit/test_services/test_book_service.py
+72 -28
View File
@@ -25,7 +25,9 @@ PDF = DATA_FILES / "Calculus Made Easy - Silvanus Thompson.pdf"
def upload(path: Path, name: str | None = None) -> UploadFile:
"""An uploaded file carrying the bytes of one of the test fixtures."""
return UploadFile(
content_type="application/pdf" if path.suffix == ".pdf" else "application/epub+zip",
content_type="application/pdf"
if path.suffix == ".pdf"
else "application/epub+zip",
filename=name or path.name,
file_data=path.read_bytes(),
)
@@ -67,7 +69,7 @@ class TestBookServiceCRUD:
# Add path manually as it won't be generated (not using the create function, but manually inserting into db)
book.path = f"{test_library.root_path}/J.R.R Tolkien/The Fellowship of the Ring"
await aios.makedirs(book.path) # type: ignore[arg-type]
await aios.makedirs(book.path) # type: ignore[arg-type]
books_service.repository.session.add(book)
await books_service.repository.session.commit()
@@ -538,9 +540,7 @@ class TestBookPathCollisions:
assert original.path != forced.path
paths = {
Path(book.path) / book.files[0].path for book in (original, forced)
}
paths = {Path(book.path) / book.files[0].path for book in (original, forced)}
assert len(paths) == 2
assert all(path.is_file() for path in paths)
@@ -581,7 +581,10 @@ class TestBookPathCollisions:
# Renamed onto the first book's author and title.
await books_service.update_book(
second.books[0].id,
{"title": original.title, "authors": [author.name for author in original.authors]},
{
"title": original.title,
"authors": [author.name for author in original.authors],
},
test_library,
)
@@ -743,7 +746,8 @@ class TestDuplicateBooks:
)
matches = await books_service.find_duplicate_books(
{"title": "Building Microservices", "authors": ["Newman, Sam;"]}, test_library
{"title": "Building Microservices", "authors": ["Newman, Sam;"]},
test_library,
)
assert [match.book_id for match in matches] == [stored.id]
@@ -834,16 +838,22 @@ class TestDuplicateBooks:
"series": "Foundation",
}
assert await books_service.find_duplicate_books(
incoming | {"series_position": "2"}, test_library
) == []
assert (
await books_service.find_duplicate_books(
incoming | {"series_position": "2"}, test_library
)
== []
)
# The same volume, written a little differently, still matches.
assert len(
await books_service.find_duplicate_books(
incoming | {"series_position": "1.0"}, test_library
assert (
len(
await books_service.find_duplicate_books(
incoming | {"series_position": "1.0"}, test_library
)
)
) == 1
== 1
)
async def test_a_book_is_not_its_own_duplicate(
self, books_service: BookService, test_library: m.Library
@@ -909,7 +919,10 @@ class TestAuthorNames:
existing row and then collides with it on the unique index.
"""
first = await store_book(
books_service, test_library, title="Building Microservices", authors=["Sam Newman"]
books_service,
test_library,
title="Building Microservices",
authors=["Sam Newman"],
)
second = await store_book(
books_service,
@@ -953,7 +966,9 @@ class TestAuthorNames:
"Franz Kafka.epub" is not a person.
"""
result = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB, "Some Unknown Book - Ada Lovelace.epub")]),
BooksCreateFromFiles(
files=[upload(EPUB, "Some Unknown Book - Ada Lovelace.epub")]
),
test_library,
)
@@ -1014,7 +1029,10 @@ class TestMergeBooks:
) -> None:
"""The survivor keeps its own fields unless the caller says otherwise."""
keep = await store_book(
books_service, test_library, title="Building Microservices", authors=["Sam Newman"]
books_service,
test_library,
title="Building Microservices",
authors=["Sam Newman"],
)
fold = await store_book(
books_service,
@@ -1030,7 +1048,11 @@ class TestMergeBooks:
assert merged.publisher is None
other = await store_book(
books_service, test_library, title="Monolith", authors=["Sam Newman"], edition=3
books_service,
test_library,
title="Monolith",
authors=["Sam Newman"],
edition=3,
)
merged = await books_service.merge_books(
keep.id, [other.id], test_library, metadata={"edition": 3}
@@ -1096,10 +1118,14 @@ class TestMergeBooks:
await books_service.merge_books(keep.id, [fold.id], test_library)
rows = (
await books_service.repository.session.execute(
select(m.BookProgress).where(m.BookProgress.book_id == keep.id)
(
await books_service.repository.session.execute(
select(m.BookProgress).where(m.BookProgress.book_id == keep.id)
)
)
).scalars().all()
.scalars()
.all()
)
assert [row.percentage for row in rows] == [0.6]
@@ -1112,13 +1138,23 @@ class TestMergeBooks:
) -> None:
"""Both books on one shelf must not leave the survivor linked to it twice."""
keep = await store_book(
books_service, test_library, title="A", authors=["X"], tags=["Shared", "Only Keep"]
books_service,
test_library,
title="A",
authors=["X"],
tags=["Shared", "Only Keep"],
)
fold = await store_book(
books_service, test_library, title="B", authors=["X"], tags=["Shared", "Only Fold"]
books_service,
test_library,
title="B",
authors=["X"],
tags=["Shared", "Only Fold"],
)
shelf = m.BookList(title="Later", user_id=test_user.id, library_id=test_library.id)
shelf = m.BookList(
title="Later", user_id=test_user.id, library_id=test_library.id
)
session.add(shelf)
await session.commit()
session.add_all(
@@ -1131,13 +1167,21 @@ class TestMergeBooks:
merged = await books_service.merge_books(keep.id, [fold.id], test_library)
assert sorted(tag.name for tag in merged.tags) == ["Only Fold", "Only Keep", "Shared"]
assert sorted(tag.name for tag in merged.tags) == [
"Only Fold",
"Only Keep",
"Shared",
]
links = (
await books_service.repository.session.execute(
select(m.BookListLink).where(m.BookListLink.book_id == keep.id)
(
await books_service.repository.session.execute(
select(m.BookListLink).where(m.BookListLink.book_id == keep.id)
)
)
).scalars().all()
.scalars()
.all()
)
assert len(links) == 1
async def test_an_identifier_moves_only_under_a_name_the_survivor_lacks(
backend/tests/unit/test_services/test_bookshelf_service.py
+1 -8
View File
@@ -2,19 +2,12 @@
import pytest
from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession
from chitai.services import ShelfService
from chitai.database import models as m
import pytest
from sqlalchemy import select
from chitai.services.bookshelf import ShelfService
from chitai.services import BookService
from chitai.database.models.book_list import BookList, BookListLink
from chitai.database import models as m
@pytest.fixture
backend/tests/unit/test_services/test_calibre_import_service.py
+11 -3
View File
@@ -201,7 +201,9 @@ async def test_importing_twice_creates_nothing(
]
held_by = [
skipped.book_id for skipped in result.skipped if skipped.reason == "already stored"
skipped.book_id
for skipped in result.skipped
if skipped.reason == "already stored"
]
assert all(book_id is not None for book_id in held_by)
@@ -223,7 +225,9 @@ async def test_a_file_the_catalogue_lists_but_disk_does_not(
await source.close()
assert len(result.created) == 1
assert [(s.calibre_id, s.reason) for s in result.skipped] == [(2, "no files on disk")]
assert [(s.calibre_id, s.reason) for s in result.skipped] == [
(2, "no files on disk")
]
async def test_one_broken_book_does_not_stop_the_import(
@@ -312,7 +316,11 @@ async def test_shared_authors_and_tags_are_one_row_each(
1, "One", authors=["Franz Kafka"], tags=["Fiction"], formats={"EPUB": EPUB}
)
fixture.add_book(
2, "Two", authors=["Franz Kafka"], tags=["Fiction"], formats={"EPUB": OTHER_EPUB}
2,
"Two",
authors=["Franz Kafka"],
tags=["Fiction"],
formats={"EPUB": OTHER_EPUB},
)
root = fixture.commit()
backend/tests/unit/test_services/test_library_service.py
+7 -7
View File
@@ -34,8 +34,8 @@ class TestLibraryServiceCRUD:
assert library.name == "Test Library"
assert library.root_path == library_path
assert library.path_template == "{author}/{title}"
assert library.description == None
assert library.read_only == False
assert library.description is None
assert library.read_only is False
# Check if directory was created
assert Path(library.root_path).is_dir()
@@ -56,8 +56,8 @@ class TestLibraryServiceCRUD:
read_only=False,
)
with pytest.raises(PermissionError) as exc_info:
library = await library_service.create(library_data)
with pytest.raises(PermissionError):
await library_service.create(library_data)
# Check if directory was created
assert not Path(library_path).exists()
@@ -86,8 +86,8 @@ class TestLibraryServiceCRUD:
assert library.name == "Test Library"
assert library.root_path == library_path
assert library.path_template == "{author}/{title}"
assert library.description == None
assert library.read_only == True
assert library.description is None
assert library.read_only is True
async def test_create_library_read_only_nonexistent_path(
self, library_service: LibraryService, tmp_path: Path
@@ -138,7 +138,7 @@ class TestLibraryServiceCRUD:
assert library.root_path == "./books"
assert library.path_template == "{author}/{title}"
assert library.description is None
assert library.read_only == False
assert library.read_only is False
# async def test_delete_library_keep_files(
# self, session: AsyncSession, library_service: LibraryService
backend/tests/unit/test_services/test_user_service.py
+5 -5
View File
@@ -21,7 +21,7 @@ class TestUserServiceAuthentication:
# Create a user with a known password
password = "password123"
user = m.User(email=f"test@example.com", password=password)
user = m.User(email="test@example.com", password=password)
session.add(user)
await session.commit()
@@ -52,7 +52,7 @@ class TestUserServiceAuthentication:
# Create user
password = "password123"
user = m.User(email=f"test@example.com", password=password)
user = m.User(email="test@example.com", password=password)
session.add(user)
await session.commit()
@@ -85,7 +85,7 @@ class TestUserServiceCRUD:
) -> None:
"""Test getting user by email."""
user = m.User(email=f"test@example.com", password="password123")
user = m.User(email="test@example.com", password="password123")
session.add(user)
await session.commit()
@@ -102,12 +102,12 @@ class TestUserServiceCRUD:
"""Test creating a new user with a duplicate email."""
# Create first user
user = m.User(email=f"test@example.com", password="password123")
user = m.User(email="test@example.com", password="password123")
session.add(user)
await session.commit()
# Create second user
user = m.User(email=f"test@example.com", password="password12345")
user = m.User(email="test@example.com", password="password12345")
with pytest.raises(IntegrityError) as exc_info:
session.add(user)