test(docs[sphinx_fonts]): add tests for sphinx_fonts extension#1023
Merged
Conversation
why: ruff D101/D103 rules flag missing docstrings on SetupDict and setup(), causing CI lint failures in repos that lint docs/_ext/. what: - Add docstring to SetupDict TypedDict class - Add docstring to setup() function
why: codecov drops because docs/_ext/sphinx_fonts.py is measured for coverage but has zero tests across all repos. what: - Add test_sphinx_fonts.py with 21 tests covering all functions - Test pure functions, I/O with monkeypatch, Sphinx events with SimpleNamespace - Cover all branches: cached/success/URLError/OSError, html/non-html, empty/with fonts
why: CI ruff check fails on EM101 (string literal in exception), TRY003 (long exception message), and D403 (uncapitalized docstring). what: - Extract exception messages to variables for EM101/TRY003 - Capitalize docstrings starting with "setup" for D403
why: CI ruff format check fails on multi-line function call formatting. what: - Apply ruff format to test_sphinx_fonts.py
why: CI mypy fails with unused-ignore because sphinx_fonts is imported as an untyped module via sys.path, making arg-type suppression unnecessary. what: - Remove all type: ignore[arg-type] comments from test_sphinx_fonts.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1023 +/- ##
=======================================
Coverage 81.01% 81.01%
=======================================
Files 28 28
Lines 2628 2628
Branches 492 492
=======================================
Hits 2129 2129
Misses 368 368
Partials 131 131 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Member
Author
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
why: font_faces.append() ran unconditionally, emitting @font-face CSS rules pointing to missing files when download failed (browser 404s). what: - Move font_faces.append() inside the if _download_font() block - Update test to expect 0 entries on download failure
why: urlretrieve can leave partial .woff2 on OSError mid-transfer, poisoning the cache and delivering corrupt fonts on subsequent builds. what: - Add dest.unlink() in except block to remove partial files - Add test for partial file cleanup behavior
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docs/_ext/sphinx_fonts.pysphinx_fontsextension for ruff D101/D103 complianceSimpleNamespacestubs andmonkeypatchfor I/O isolationTest coverage
_cache_dir()_cdn_url()_download_font()_on_builder_inited()_on_html_page_context()setup()Commits
docs(fonts[lint]): add docstrings to sphinx_fonts extensiontest(docs[sphinx_fonts]): add tests for sphinx_fonts extensiontest(docs[sphinx_fonts]): fix ruff lint errorstest(docs[sphinx_fonts]): apply ruff formattest(docs[sphinx_fonts]): removetype: ignorecomments for mypy compat