feat(test): add unit tests for config, output, and dubbing modules by KYBvWHxW · Pull Request #24 · NarratorAI-Studio/narrator-ai-cli

KYBvWHxW · 2026-04-29T16:51:21Z

Related Issue

Closes #23

Summary

test_config.py: 10 tests — config load/save, env vars, defaults, error paths
test_output.py: 7 tests — JSON output, error/success/info formatting
test_dubbing.py: 7 tests — voice list, filtering by lang/tag, JSON output
Coverage: 30% → 37% (28 tests total, all pass)

Test Plan

All 28 tests pass locally
Coverage above 20% threshold (37%)

🤖 Generated with Claude Code

- test_config.py: 10 tests — load/save, env vars, defaults, error cases - test_output.py: 7 tests — JSON output, error/success/info formatting - test_dubbing.py: 7 tests — voice list filtering, JSON output, fields Coverage: 30% → 37% (28 tests, all pass) Closes #22 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds a new unit-test suite to improve coverage for core CLI modules (config management, output formatting, and dubbing voice listing/filtering), addressing Issue #23’s request to expand beyond smoke tests.

Changes:

Adds tests for narrator_ai.output JSON and message output helpers.
Adds tests for narrator_ai.commands.dubbing CLI JSON output and basic filtering behavior.
Adds tests for narrator_ai.config load/save behavior and env var overrides.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
tests/test_output.py	New tests covering JSON stdout output and error/success/info stderr output.
tests/test_dubbing.py	New CLI tests for dubbing list/languages/tags JSON output and filter options.
tests/test_config.py	New tests for default config, load/save roundtrip, and env-var overrides.

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

Copilot · 2026-04-29T16:54:56Z

+    assert result.exit_code == 0
+    import json
+    data = json.loads(result.output)
+    assert all(v["type"] == "英语" for v in data)


The CLI filter logic uses substring matching (lang in d["type"]), but this test asserts strict equality. To match the implemented behavior and avoid brittle failures if type ever contains additional descriptors, assert lang in v["type"] (or otherwise align the assertion with the command’s semantics).

Suggested change

assert all(v["type"] == "英语" for v in data)

assert all("英语" in v["type"] for v in data)

Copilot · 2026-04-29T16:54:56Z

+    assert result.exit_code == 0
+    import json
+    data = json.loads(result.output)
+    assert all(v["tag"] == "通用男声" for v in data)


The command filters tags with substring matching (tag in d["tag"]), but this test asserts strict equality. Align the assertion with the actual filter semantics (e.g., tag in v["tag"]) to reduce brittleness if tag strings expand in the future.

Suggested change

assert all(v["tag"] == "通用男声" for v in data)

assert all("通用男声" in v["tag"] for v in data)

Copilot · 2026-04-29T16:54:56Z

+from io import StringIO
+from unittest.mock import patch


StringIO and patch are imported but never used. With Ruff linting enabled (F401), this will fail CI; remove the unused imports or use them in the tests.

Suggested change

from io import StringIO

from unittest.mock import patch

Copilot · 2026-04-29T16:54:57Z

+def test_get_server_from_env():
+    with patch.dict(os.environ, {"NARRATOR_SERVER": "https://env.example.com"}):
+        assert get_server() == "https://env.example.com"
+
+
+def test_get_server_strips_trailing_slash():
+    with patch.dict(os.environ, {"NARRATOR_SERVER": "https://example.com/"}):


These tests call get_server() without isolating the on-disk config. If a developer/CI machine has an existing ~/.narrator-ai/config.yaml (or invalid YAML), load_config() can affect or break this test even though the env var is set. Patch narrator_ai.config.CONFIG_FILE to a nonexistent path (or tmp_path) within this test to make it hermetic.

Suggested change

def test_get_server_from_env():

with patch.dict(os.environ, {"NARRATOR_SERVER": "https://env.example.com"}):

assert get_server() == "https://env.example.com"

def test_get_server_strips_trailing_slash():

with patch.dict(os.environ, {"NARRATOR_SERVER": "https://example.com/"}):

def test_get_server_from_env(tmp_path):

with patch("narrator_ai.config.CONFIG_FILE", tmp_path / "nonexistent.yaml"), \

patch.dict(os.environ, {"NARRATOR_SERVER": "https://env.example.com"}):

assert get_server() == "https://env.example.com"

def test_get_server_strips_trailing_slash(tmp_path):

with patch("narrator_ai.config.CONFIG_FILE", tmp_path / "nonexistent.yaml"), \

patch.dict(os.environ, {"NARRATOR_SERVER": "https://example.com/"}):

Copilot · 2026-04-29T16:54:57Z

+def test_get_server_strips_trailing_slash():
+    with patch.dict(os.environ, {"NARRATOR_SERVER": "https://example.com/"}):


Same isolation issue as above: get_server() loads from CONFIG_FILE before reading env vars. Patch narrator_ai.config.CONFIG_FILE to a temp/nonexistent file in this test so it doesn't depend on the user's real config.

Suggested change

def test_get_server_strips_trailing_slash():

with patch.dict(os.environ, {"NARRATOR_SERVER": "https://example.com/"}):

def test_get_server_strips_trailing_slash(tmp_path):

with patch("narrator_ai.config.CONFIG_FILE", tmp_path / "nonexistent.yaml"), \

patch.dict(os.environ, {"NARRATOR_SERVER": "https://example.com/"}):

Copilot · 2026-04-29T16:54:57Z

+    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("/")


This test name says it "raises", but it currently asserts the default server is returned and explicitly notes it won't raise. Either rename it to reflect the behavior being tested, or adjust the setup to exercise the actual error path (e.g., create a config file with server: "" and clear env so get_server() raises SystemExit).

Suggested change

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("/")

config_file = tmp_path / "config.yaml"

config_dir = tmp_path

with patch("narrator_ai.config.CONFIG_FILE", config_file), \

patch("narrator_ai.config.CONFIG_DIR", config_dir):

save_config({"server": ""})

with patch.dict(os.environ, {}, clear=True):

with pytest.raises(SystemExit):

get_server()

Copilot · 2026-04-29T16:54:58Z

+def test_get_app_key_from_env():
+    with patch.dict(os.environ, {"NARRATOR_APP_KEY": "env-key-123"}):


This test calls get_app_key() without isolating CONFIG_FILE. If a local config file exists, it can change behavior (or fail parsing) even though the env var is set. Patch narrator_ai.config.CONFIG_FILE to a tmp/nonexistent path here for hermetic tests.

Suggested change

def test_get_app_key_from_env():

with patch.dict(os.environ, {"NARRATOR_APP_KEY": "env-key-123"}):

def test_get_app_key_from_env(tmp_path):

with patch("narrator_ai.config.CONFIG_FILE", tmp_path / "nonexistent.yaml"), \

patch.dict(os.environ, {"NARRATOR_APP_KEY": "env-key-123"}):

Copilot · 2026-04-29T16:54:58Z

+def test_get_timeout_from_env():
+    with patch.dict(os.environ, {"NARRATOR_TIMEOUT": "60"}):


This test calls get_timeout() without isolating CONFIG_FILE. load_config() may read a real user config (or invalid YAML) before applying the env override, which makes the test non-hermetic. Patch narrator_ai.config.CONFIG_FILE to a tmp/nonexistent path in this test.

Suggested change

def test_get_timeout_from_env():

with patch.dict(os.environ, {"NARRATOR_TIMEOUT": "60"}):

def test_get_timeout_from_env(tmp_path):

with patch("narrator_ai.config.CONFIG_FILE", tmp_path / "nonexistent.yaml"), \

patch.dict(os.environ, {"NARRATOR_TIMEOUT": "60"}):

Copilot AI review requested due to automatic review settings April 29, 2026 16:51

KYBvWHxW merged commit 130bb92 into main Apr 29, 2026
8 of 9 checks passed

KYBvWHxW deleted the feat/add-unit-tests branch April 29, 2026 16:51

Copilot started reviewing on behalf of KYBvWHxW April 29, 2026 16:51 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

KYBvWHxW mentioned this pull request Apr 29, 2026

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

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(test): add unit tests for config, output, and dubbing modules#24

feat(test): add unit tests for config, output, and dubbing modules#24
KYBvWHxW merged 1 commit into
mainfrom
feat/add-unit-tests

KYBvWHxW commented Apr 29, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	assert all(v["type"] == "英语" for v in data)
	assert all("英语" in v["type"] for v in data)

	assert all(v["tag"] == "通用男声" for v in data)
	assert all("通用男声" in v["tag"] for v in data)

		def test_get_server_strips_trailing_slash():
		with patch.dict(os.environ, {"NARRATOR_SERVER": "https://example.com/"}):

-    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("/")
+    config_file = tmp_path / "config.yaml"
+    config_dir = tmp_path
+    with patch("narrator_ai.config.CONFIG_FILE", config_file), \
+         patch("narrator_ai.config.CONFIG_DIR", config_dir):
+        save_config({"server": ""})
+        with patch.dict(os.environ, {}, clear=True):
+            with pytest.raises(SystemExit):
+                get_server()

		def test_get_app_key_from_env():
		with patch.dict(os.environ, {"NARRATOR_APP_KEY": "env-key-123"}):

		def test_get_timeout_from_env():
		with patch.dict(os.environ, {"NARRATOR_TIMEOUT": "60"}):

Conversation

KYBvWHxW commented Apr 29, 2026

Related Issue

Summary

Test Plan

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants