Skip to content

fix: resolve ruff lint errors in new test files#25

Merged
KYBvWHxW merged 1 commit intomainfrom
fix/test-lint-errors
Apr 29, 2026
Merged

fix: resolve ruff lint errors in new test files#25
KYBvWHxW merged 1 commit intomainfrom
fix/test-lint-errors

Conversation

@KYBvWHxW
Copy link
Copy Markdown
Contributor

Summary

Test Plan

  • ruff check . passes
  • ruff format --check . passes
  • 28 tests pass

🤖 Generated with Claude Code

- Remove unused imports (F401)
- Fix import sorting (I001)
- Combine with statements (SIM117)
- Move inline imports to top level

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 29, 2026 17:03
@KYBvWHxW KYBvWHxW merged commit 8688f81 into main Apr 29, 2026
9 checks passed
@KYBvWHxW KYBvWHxW deleted the fix/test-lint-errors branch April 29, 2026 17:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Cleans up Ruff lint issues in newly added test files from #24 by removing unused imports, standardizing import placement/sorting, and simplifying with statements.

Changes:

  • Remove unused imports and sort imports in tests/test_output.py.
  • Hoist json imports to module scope in tests/test_dubbing.py.
  • Combine nested/continued with statements in tests/test_config.py (including using parenthesized context managers for readability).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/test_output.py Removes unused imports and normalizes import ordering for output helper tests.
tests/test_dubbing.py Moves json imports to the top-level to satisfy Ruff and reduce repetition.
tests/test_config.py Refactors with blocks to satisfy Ruff SIM rules and improve consistency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_config.py
Comment on lines 53 to 57
def test_get_server_raises_when_not_configured(tmp_path):
with patch("narrator_ai.config.CONFIG_FILE", tmp_path / "nonexistent.yaml"), \
patch.dict(os.environ, {}, clear=True):
with patch("narrator_ai.config.CONFIG_FILE", tmp_path / "nonexistent.yaml"), patch.dict(os.environ, {}, clear=True):
# DEFAULT_CONFIG has server set, so this won't raise
server = get_server()
assert server == DEFAULT_CONFIG["server"].rstrip("/")
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_get_server_raises_when_not_configured does not exercise the raising path: get_server() cannot raise here because DEFAULT_CONFIG["server"] is non-empty and load_config() falls back to it when the config file is missing. This makes the test name/intent misleading. Consider either renaming the test to reflect that it asserts the default server is returned, or change the setup to actually cover the SystemExit branch (e.g., patch DEFAULT_CONFIG / load_config() so server is empty and then assert pytest.raises(SystemExit)).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants