09e9f3545f
Test / Code Quality (push) Has been cancelled
Test / Test (macos-latest, Python 3.10) (push) Has been cancelled
Test / Test (macos-latest, Python 3.11) (push) Has been cancelled
Test / Test (macos-latest, Python 3.12) (push) Has been cancelled
Test / Test (macos-latest, Python 3.13) (push) Has been cancelled
Test / Test (macos-latest, Python 3.14) (push) Has been cancelled
Test / Test (ubuntu-latest, Python 3.10) (push) Has been cancelled
Test / Test (ubuntu-latest, Python 3.11) (push) Has been cancelled
Test / Test (ubuntu-latest, Python 3.12) (push) Has been cancelled
Test / Test (ubuntu-latest, Python 3.13) (push) Has been cancelled
Test / Test (ubuntu-latest, Python 3.14) (push) Has been cancelled
Test / Test (windows-latest, Python 3.10) (push) Has been cancelled
Test / Test (windows-latest, Python 3.11) (push) Has been cancelled
Test / Test (windows-latest, Python 3.12) (push) Has been cancelled
Test / Test (windows-latest, Python 3.13) (push) Has been cancelled
Test / Test (windows-latest, Python 3.14) (push) Has been cancelled
CodeQL / Analyze (push) Has been cancelled
dependency-audit / pip-audit (push) Has been cancelled
2252 lines
83 KiB
Python
2252 lines
83 KiB
Python
"""Tests for download CLI commands."""
|
|
|
|
import importlib
|
|
import json
|
|
from datetime import datetime
|
|
from pathlib import Path
|
|
from unittest.mock import AsyncMock, patch
|
|
|
|
import pytest
|
|
from click.testing import CliRunner
|
|
|
|
import notebooklm.auth as auth_module
|
|
from notebooklm.cli import helpers as helpers_module
|
|
from notebooklm.notebooklm_cli import cli
|
|
from notebooklm.types import Artifact
|
|
|
|
from .conftest import create_mock_client, inject_client
|
|
|
|
# Get the actual download module (not the click group that shadows it)
|
|
download_module = importlib.import_module("notebooklm.cli.download_cmd")
|
|
|
|
|
|
def make_artifact(
|
|
id: str, title: str, _artifact_type: int, status: int = 3, created_at: datetime = None
|
|
) -> Artifact:
|
|
"""Create an Artifact for testing."""
|
|
return Artifact(
|
|
id=id,
|
|
title=title,
|
|
_artifact_type=_artifact_type,
|
|
status=status,
|
|
created_at=created_at or datetime.fromtimestamp(1234567890),
|
|
)
|
|
|
|
|
|
@pytest.fixture
|
|
def runner():
|
|
return CliRunner()
|
|
|
|
|
|
@pytest.fixture
|
|
def mock_auth():
|
|
from notebooklm.auth import AuthTokens
|
|
|
|
auth = AuthTokens(
|
|
cookies={
|
|
"SID": "test",
|
|
"HSID": "test",
|
|
"SSID": "test",
|
|
"APISID": "test",
|
|
"SAPISID": "test",
|
|
},
|
|
csrf_token="csrf",
|
|
session_id="session",
|
|
)
|
|
|
|
with (
|
|
patch.object(helpers_module, "load_auth_from_storage") as mock_load,
|
|
patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch,
|
|
):
|
|
mock_load.return_value = auth.flat_cookies
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
yield mock_load
|
|
|
|
|
|
@pytest.fixture
|
|
def mock_fetch_tokens():
|
|
"""Mock ``fetch_tokens_with_domains`` for download CLI commands."""
|
|
with (
|
|
patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch,
|
|
):
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
yield mock_fetch
|
|
|
|
|
|
# =============================================================================
|
|
# PER-TYPE DOWNLOAD SMOKE TESTS (PARAMETRIZED)
|
|
# =============================================================================
|
|
#
|
|
# The bare per-type single-download happy-path tests
|
|
# (``test_download_<type>``) differ only by artifact type code, download
|
|
# method name, and output extension. They collapse into one parametrize over
|
|
# ``(cmd, method, type_code, output_name, is_dir)`` crossed with text/JSON
|
|
# output mode — folding the per-type cluster (#1315) and the audio text-vs-JSON
|
|
# happy-path pair (#1317) into a single matrix. Each text case asserts exit 0
|
|
# (plus the file/dir is written); each JSON case asserts the shared
|
|
# ``download_single`` envelope. Type-specific flag, error, and distinct-shape
|
|
# tests remain standalone below.
|
|
|
|
# File-based single-download types: (cmd, method, type_code, output_name).
|
|
# These write a single file to ``output_name`` and emit pure stdout in both
|
|
# text and JSON modes, so they run the full text x JSON matrix.
|
|
_FILE_DOWNLOAD_CASES = [
|
|
("audio", "download_audio", 1, "audio.mp3"),
|
|
("video", "download_video", 3, "video.mp4"),
|
|
("infographic", "download_infographic", 7, "infographic.png"),
|
|
]
|
|
|
|
|
|
class TestDownloadStandardTypes:
|
|
"""Per-type single-download happy-path coverage across text and JSON modes."""
|
|
|
|
@pytest.mark.parametrize("output_mode", ["text", "json"])
|
|
@pytest.mark.parametrize(
|
|
"cmd,method,type_code,output_name",
|
|
_FILE_DOWNLOAD_CASES,
|
|
ids=[case[0] for case in _FILE_DOWNLOAD_CASES],
|
|
)
|
|
def test_download_file_type(
|
|
self, runner, mock_auth, tmp_path, output_mode, cmd, method, type_code, output_name
|
|
):
|
|
expected_id = f"{cmd}_1"
|
|
mock_client = create_mock_client()
|
|
output_target = tmp_path / output_name
|
|
|
|
async def mock_download(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
Path(output_path).write_bytes(b"fake content")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact(expected_id, "My Artifact", type_code)]
|
|
)
|
|
setattr(mock_client.artifacts, method, mock_download)
|
|
|
|
args = ["download", cmd, str(output_target), "-n", "nb_123"]
|
|
if output_mode == "json":
|
|
args.append("--json")
|
|
|
|
with patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch:
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
result = runner.invoke(cli, args, obj=inject_client(mock_client))
|
|
|
|
assert result.exit_code == 0, result.output
|
|
# The file is written in both modes — that is the command's purpose.
|
|
assert output_target.exists()
|
|
if output_mode == "json":
|
|
data = json.loads(result.output)
|
|
assert data["operation"] == "download_single"
|
|
assert data["status"] == "downloaded"
|
|
assert data["artifact"]["id"] == expected_id
|
|
# Happy path must not emit an error envelope.
|
|
assert "error" not in data
|
|
assert "code" not in data
|
|
|
|
def test_download_slide_deck(self, runner, mock_auth, tmp_path):
|
|
"""Slide-deck downloads a directory of slides (text mode only).
|
|
|
|
Kept off the file-type JSON matrix because slide-deck writes a
|
|
directory and the command prepends a format-extension warning to
|
|
stdout, so it never emitted a clean JSON document in the original
|
|
suite either.
|
|
"""
|
|
mock_client = create_mock_client()
|
|
output_dir = tmp_path / "slides"
|
|
|
|
async def mock_download_slide_deck(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
Path(output_path).mkdir(parents=True, exist_ok=True)
|
|
(Path(output_path) / "slide_1.png").write_bytes(b"fake slide")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("slide_1", "My Slides", 8)]
|
|
)
|
|
mock_client.artifacts.download_slide_deck = mock_download_slide_deck
|
|
|
|
with patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch:
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "slide-deck", str(output_dir), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert (output_dir / "slide_1.png").exists()
|
|
|
|
|
|
# =============================================================================
|
|
# DOWNLOAD AUDIO TESTS
|
|
# =============================================================================
|
|
|
|
|
|
class TestDownloadAudio:
|
|
def test_download_audio_dry_run(self, runner, mock_auth, tmp_path):
|
|
mock_client = create_mock_client()
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("audio_123", "My Audio", 1)]
|
|
)
|
|
|
|
with (
|
|
patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch,
|
|
):
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--dry-run", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
assert "DRY RUN" in result.output
|
|
|
|
def test_download_audio_no_artifacts(self, runner, mock_auth):
|
|
mock_client = create_mock_client()
|
|
mock_client.artifacts.list = AsyncMock(return_value=[])
|
|
|
|
with (
|
|
patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch,
|
|
):
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
result = runner.invoke(
|
|
cli, ["download", "audio", "-n", "nb_123"], obj=inject_client(mock_client)
|
|
)
|
|
|
|
assert "No completed audio artifacts found" in result.output or result.exit_code != 0
|
|
|
|
|
|
# =============================================================================
|
|
# DOWNLOAD FLAGS TESTS
|
|
# =============================================================================
|
|
|
|
|
|
class TestDownloadFlags:
|
|
def test_download_audio_latest(self, runner, mock_auth, tmp_path):
|
|
"""Test --latest flag selects most recent artifact"""
|
|
mock_client = create_mock_client()
|
|
|
|
output_file = tmp_path / "audio.mp3"
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
Path(output_path).write_bytes(b"fake audio")
|
|
return output_path
|
|
|
|
# Set up artifacts namespace (pre-created by create_mock_client)
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact(
|
|
"audio_old", "Old Audio", 1, created_at=datetime.fromtimestamp(1000000000)
|
|
),
|
|
make_artifact(
|
|
"audio_new", "New Audio", 1, created_at=datetime.fromtimestamp(2000000000)
|
|
),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
with (
|
|
patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch,
|
|
):
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", str(output_file), "--latest", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
|
|
def test_download_audio_earliest(self, runner, mock_auth, tmp_path):
|
|
"""Test --earliest flag selects oldest artifact"""
|
|
mock_client = create_mock_client()
|
|
|
|
output_file = tmp_path / "audio.mp3"
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
Path(output_path).write_bytes(b"fake audio")
|
|
return output_path
|
|
|
|
# Set up artifacts namespace (pre-created by create_mock_client)
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact(
|
|
"audio_old", "Old Audio", 1, created_at=datetime.fromtimestamp(1000000000)
|
|
),
|
|
make_artifact(
|
|
"audio_new", "New Audio", 1, created_at=datetime.fromtimestamp(2000000000)
|
|
),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
with (
|
|
patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch,
|
|
):
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", str(output_file), "--earliest", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
|
|
def test_download_force_overwrites(self, runner, mock_auth, tmp_path):
|
|
"""Test --force flag overwrites existing file"""
|
|
mock_client = create_mock_client()
|
|
|
|
output_file = tmp_path / "audio.mp3"
|
|
output_file.write_bytes(b"existing content")
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
Path(output_path).write_bytes(b"new content")
|
|
return output_path
|
|
|
|
# Set up artifacts namespace (pre-created by create_mock_client)
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("audio_123", "Audio", 1)]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
with (
|
|
patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch,
|
|
):
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", str(output_file), "--force", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
assert output_file.read_bytes() == b"new content"
|
|
|
|
def test_download_no_clobber_skips(self, runner, mock_auth, tmp_path):
|
|
"""Test --no-clobber flag skips existing file"""
|
|
mock_client = create_mock_client()
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("audio_123", "Audio", 1)]
|
|
)
|
|
|
|
output_file = tmp_path / "audio.mp3"
|
|
output_file.write_bytes(b"existing content")
|
|
|
|
with (
|
|
patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch,
|
|
):
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
runner.invoke(
|
|
cli,
|
|
["download", "audio", str(output_file), "--no-clobber", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
# File should remain unchanged
|
|
assert output_file.read_bytes() == b"existing content"
|
|
|
|
|
|
# =============================================================================
|
|
# JSON OUTPUT UNICODE TESTS
|
|
# =============================================================================
|
|
|
|
|
|
class TestDownloadJsonOutputUnicode:
|
|
def test_download_json_output_preserves_unicode(self, runner, mock_auth):
|
|
"""`download <type> --json` should emit CJK / emoji as real UTF-8, not \\uXXXX.
|
|
|
|
The per-leaf generic helper now lives behind
|
|
``services.download.execute_download``. Patching there short-circuits
|
|
the entire async download pipeline the same way the old generic-helper
|
|
patch did before.
|
|
"""
|
|
fake_result = {
|
|
"artifact_id": "audio_123",
|
|
"title": "中文音频 🎧",
|
|
"output_path": "音频.mp3",
|
|
}
|
|
|
|
async def fake_execute_download(*args, **kwargs):
|
|
return fake_result
|
|
|
|
with (
|
|
patch.object(
|
|
download_module,
|
|
"execute_download",
|
|
fake_execute_download,
|
|
),
|
|
patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch,
|
|
):
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
result = runner.invoke(cli, ["download", "audio", "--json", "-n", "nb_123"])
|
|
|
|
assert result.exit_code == 0
|
|
data = json.loads(result.output)
|
|
assert data["title"] == "中文音频 🎧"
|
|
assert data["output_path"] == "音频.mp3"
|
|
# Raw output must contain real CJK/emoji, not escaped sequences.
|
|
assert "中文音频" in result.output
|
|
assert "🎧" in result.output
|
|
assert "\\u" not in result.output
|
|
|
|
|
|
class TestDownloadWarnings:
|
|
def test_format_mismatch_warning_is_rendered_by_command_layer(self, runner, mock_auth):
|
|
"""Format-extension warnings carried by the plan are echoed by the Click layer."""
|
|
|
|
async def fake_execute_download(*args, **kwargs):
|
|
return {
|
|
"operation": "download_single",
|
|
"artifact": {"title": "Slides", "selection_reason": "latest"},
|
|
"output_path": "deck.pdf",
|
|
"status": "downloaded",
|
|
}
|
|
|
|
with (
|
|
patch.object(download_module, "execute_download", fake_execute_download),
|
|
patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch,
|
|
):
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
result = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"slide-deck",
|
|
"deck.pdf",
|
|
"--format",
|
|
"pptx",
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
combined = result.output + (result.stderr if result.stderr_bytes else "")
|
|
assert "does not end with '.pptx'" in combined
|
|
|
|
|
|
# =============================================================================
|
|
# COMMAND EXISTENCE TESTS
|
|
# =============================================================================
|
|
|
|
|
|
class TestDownloadCommandsExist:
|
|
def test_download_group_exists(self, runner):
|
|
result = runner.invoke(cli, ["download", "--help"])
|
|
assert result.exit_code == 0
|
|
assert "audio" in result.output
|
|
assert "video" in result.output
|
|
|
|
def test_download_audio_command_exists(self, runner):
|
|
result = runner.invoke(cli, ["download", "audio", "--help"])
|
|
assert result.exit_code == 0
|
|
assert "OUTPUT_PATH" in result.output
|
|
assert "--notebook" in result.output or "-n" in result.output
|
|
|
|
def test_download_cinematic_video_alias_exists(self, runner):
|
|
"""Verify 'download cinematic-video' alias is registered and shows help."""
|
|
result = runner.invoke(cli, ["download", "cinematic-video", "--help"])
|
|
assert result.exit_code == 0
|
|
assert "cinematic" in result.output.lower()
|
|
|
|
def test_download_cinematic_video_alias_callable(self, runner, mock_auth, tmp_path):
|
|
"""Verify 'download cinematic-video' alias invokes download video logic."""
|
|
mock_client = create_mock_client()
|
|
|
|
output_file = tmp_path / "cinematic.mp4"
|
|
|
|
async def mock_download_video(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
Path(output_path).write_bytes(b"fake cinematic content")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("cin_1", "My Cinematic Video", 3)]
|
|
)
|
|
mock_client.artifacts.download_video = mock_download_video
|
|
|
|
with (
|
|
patch.object(
|
|
auth_module, "fetch_tokens_with_domains", new_callable=AsyncMock
|
|
) as mock_fetch,
|
|
):
|
|
mock_fetch.return_value = ("csrf", "session")
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "cinematic-video", str(output_file), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert output_file.exists()
|
|
|
|
|
|
# =============================================================================
|
|
# FLAG CONFLICT VALIDATION TESTS
|
|
# =============================================================================
|
|
|
|
|
|
class TestDownloadFlagConflicts:
|
|
"""Test that conflicting flag combinations raise appropriate errors."""
|
|
|
|
def test_force_and_no_clobber_conflict(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""Test --force and --no-clobber cannot be used together."""
|
|
mock_client = create_mock_client()
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("audio_123", "Audio", 1)]
|
|
)
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--force", "--no-clobber", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
assert "Cannot specify both --force and --no-clobber" in result.output
|
|
|
|
def test_latest_and_earliest_conflict(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""Test --latest and --earliest cannot be used together."""
|
|
mock_client = create_mock_client()
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("audio_123", "Audio", 1)]
|
|
)
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--latest", "--earliest", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
assert "Cannot specify both --latest and --earliest" in result.output
|
|
|
|
def test_all_and_artifact_conflict(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""Test --all and --artifact cannot be used together."""
|
|
mock_client = create_mock_client()
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("audio_123", "Audio", 1)]
|
|
)
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--all", "--artifact", "art_123", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
assert "Cannot specify both --all and --artifact" in result.output
|
|
|
|
|
|
# =============================================================================
|
|
# AUTO-RENAME TESTS
|
|
# =============================================================================
|
|
|
|
|
|
class TestDownloadAutoRename:
|
|
"""Test auto-rename functionality when file exists and --force not specified."""
|
|
|
|
def test_auto_renames_on_conflict(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""When file exists without --force or --no-clobber, should auto-rename."""
|
|
mock_client = create_mock_client()
|
|
|
|
output_file = tmp_path / "audio.mp3"
|
|
output_file.write_bytes(b"existing content")
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
Path(output_path).write_bytes(b"new content")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("audio_123", "Audio", 1)]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", str(output_file), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
# Original file unchanged
|
|
assert output_file.read_bytes() == b"existing content"
|
|
# New file created with (2) suffix
|
|
renamed_file = tmp_path / "audio (2).mp3"
|
|
assert renamed_file.exists()
|
|
assert renamed_file.read_bytes() == b"new content"
|
|
|
|
|
|
# =============================================================================
|
|
# DOWNLOAD ALL TESTS
|
|
# =============================================================================
|
|
|
|
|
|
class TestDownloadAll:
|
|
"""Test --all flag for batch downloading."""
|
|
|
|
def test_download_all_basic(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""Test basic --all download to a directory."""
|
|
mock_client = create_mock_client()
|
|
|
|
output_dir = tmp_path / "downloads"
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
Path(output_path).write_bytes(b"audio content")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_1", "First Audio", 1),
|
|
make_artifact("audio_2", "Second Audio", 1),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--all", str(output_dir), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
assert output_dir.exists()
|
|
# Check that files were downloaded
|
|
downloaded_files = list(output_dir.glob("*.mp3"))
|
|
assert len(downloaded_files) == 2
|
|
|
|
def test_download_all_dry_run(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""Test --all --dry-run shows preview without downloading."""
|
|
mock_client = create_mock_client()
|
|
|
|
output_dir = tmp_path / "downloads"
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_1", "First Audio", 1),
|
|
make_artifact("audio_2", "Second Audio", 1),
|
|
]
|
|
)
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--all", "--dry-run", str(output_dir), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
assert "DRY RUN" in result.output
|
|
assert "2" in result.output # Count of artifacts
|
|
# Directory should NOT be created
|
|
assert not output_dir.exists()
|
|
|
|
def test_download_all_with_failures(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""Test --all continues on individual artifact failures.
|
|
|
|
Per P1.T4 contract: ANY per-item failure must propagate to a non-zero
|
|
exit code (the loop still attempts every artifact, but the command does
|
|
not silently report success when some artifacts failed).
|
|
"""
|
|
mock_client = create_mock_client()
|
|
|
|
output_dir = tmp_path / "downloads"
|
|
call_count = 0
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
nonlocal call_count
|
|
call_count += 1
|
|
if call_count == 1:
|
|
raise Exception("Network error")
|
|
Path(output_path).write_bytes(b"audio content")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_1", "First Audio", 1),
|
|
make_artifact("audio_2", "Second Audio", 1),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--all", str(output_dir), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
# Loop still attempts every artifact (so the second succeeds), but exit
|
|
# is non-zero because at least one item failed.
|
|
assert result.exit_code != 0
|
|
# One file should be downloaded
|
|
downloaded_files = list(output_dir.glob("*.mp3"))
|
|
assert len(downloaded_files) == 1
|
|
# The text-mode renderer must show the structured "Failed" section
|
|
# listing the actual error, not just the boolean "Error: True" guard.
|
|
# ``"1" in result.output`` was an unreliable assertion because the
|
|
# progress indicator ``Downloading 1/2:`` always emits a "1".
|
|
output_lower = result.output.lower()
|
|
assert "failed" in output_lower
|
|
assert "network error" in output_lower
|
|
assert "error: true" not in output_lower
|
|
|
|
|
|
class TestDownloadAllExitCodeContract:
|
|
"""P1.T4 §1 — `--all` failure exit-code + JSON envelope.
|
|
|
|
Contract: ANY per-item failure inside the `--all` loop must produce a
|
|
non-zero exit code AND a top-level envelope of shape
|
|
``{"error": true, "failed_count": N, "succeeded_count": M, "artifacts": [...]}``
|
|
so automation callers can distinguish partial from total failure without
|
|
walking every per-item entry.
|
|
"""
|
|
|
|
def test_all_fail_exits_nonzero_with_envelope(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""When every artifact fails, exit non-zero with full failure envelope."""
|
|
mock_client = create_mock_client()
|
|
output_dir = tmp_path / "downloads"
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
raise Exception("Network error")
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_1", "First Audio", 1),
|
|
make_artifact("audio_2", "Second Audio", 1),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"audio",
|
|
"--all",
|
|
"--json",
|
|
str(output_dir),
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
payload = json.loads(result.output)
|
|
assert payload.get("error") is True
|
|
assert payload.get("failed_count") == 2
|
|
assert payload.get("succeeded_count") == 0
|
|
# Per-item entries preserved under the new "artifacts" key.
|
|
assert len(payload.get("artifacts", [])) == 2
|
|
statuses = [a.get("status") for a in payload["artifacts"]]
|
|
assert statuses.count("failed") == 2
|
|
|
|
def test_partial_failure_exits_nonzero_with_envelope(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""When some succeed and some fail, exit non-zero and report both counts."""
|
|
mock_client = create_mock_client()
|
|
output_dir = tmp_path / "downloads"
|
|
call_count = 0
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
nonlocal call_count
|
|
call_count += 1
|
|
if call_count == 1:
|
|
raise Exception("Network error")
|
|
Path(output_path).write_bytes(b"audio content")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_1", "First Audio", 1),
|
|
make_artifact("audio_2", "Second Audio", 1),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"audio",
|
|
"--all",
|
|
"--json",
|
|
str(output_dir),
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
payload = json.loads(result.output)
|
|
assert payload.get("error") is True
|
|
assert payload.get("failed_count") == 1
|
|
assert payload.get("succeeded_count") == 1
|
|
assert len(payload.get("artifacts", [])) == 2
|
|
|
|
def test_partial_failure_text_mode_shows_breakdown(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""The text-mode renderer must show the structured Downloaded/Failed
|
|
breakdown on partial failure — NOT short-circuit to ``Error: True``.
|
|
|
|
Regression guard for the reviewer-flagged renderer bug: setting
|
|
``envelope["error"] = True`` to drive exit-code policy used to hit the
|
|
renderer's generic ``if "error" in result`` early-return guard and
|
|
swallow the per-item breakdown. The renderer now keys off the legacy
|
|
string-error shape only, so the typed-counts envelope falls through to
|
|
the ``download_all`` summary block.
|
|
"""
|
|
mock_client = create_mock_client()
|
|
output_dir = tmp_path / "downloads"
|
|
call_count = 0
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
nonlocal call_count
|
|
call_count += 1
|
|
if call_count == 1:
|
|
raise Exception("Network error")
|
|
Path(output_path).write_bytes(b"audio content")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_1", "First Audio", 1),
|
|
make_artifact("audio_2", "Second Audio", 1),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
# No --json: text-mode renderer must show the full breakdown.
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--all", str(output_dir), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
output_lower = result.output.lower()
|
|
# The Rich breakdown sections must appear ("Downloaded:" and "Failed:")
|
|
# along with the actual error message — proving the renderer did not
|
|
# short-circuit on the boolean error flag.
|
|
assert "downloaded" in output_lower
|
|
assert "failed" in output_lower
|
|
assert "network error" in output_lower
|
|
assert "first audio" in output_lower
|
|
assert "second audio" in output_lower
|
|
# And explicitly NOT the boolean-leak text.
|
|
assert "error: true" not in output_lower
|
|
|
|
def test_all_success_keeps_zero_exit_and_no_error_key(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""When every artifact succeeds, exit zero and omit the error key."""
|
|
mock_client = create_mock_client()
|
|
output_dir = tmp_path / "downloads"
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
Path(output_path).write_bytes(b"audio content")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_1", "First Audio", 1),
|
|
make_artifact("audio_2", "Second Audio", 1),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"audio",
|
|
"--all",
|
|
"--json",
|
|
str(output_dir),
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
payload = json.loads(result.output)
|
|
assert "error" not in payload
|
|
assert payload.get("failed_count") == 0
|
|
assert payload.get("succeeded_count") == 2
|
|
|
|
|
|
class TestDownloadAllNameFilter:
|
|
"""P1.T4 §2 — `--all --name <name>` filter applied to the artifact list."""
|
|
|
|
def test_name_filter_restricts_downloads(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""`--all --name "Beta"` must download only matching artifacts (substring,
|
|
case-insensitive), not every artifact in the notebook."""
|
|
mock_client = create_mock_client()
|
|
output_dir = tmp_path / "downloads"
|
|
downloaded_ids: list[str | None] = []
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
downloaded_ids.append(artifact_id)
|
|
Path(output_path).write_bytes(b"audio content")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_alpha", "Alpha Briefing", 1),
|
|
make_artifact("audio_beta1", "Beta Chapter One", 1),
|
|
make_artifact("audio_beta2", "Beta Chapter Two", 1),
|
|
make_artifact("audio_gamma", "Gamma Recap", 1),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"audio",
|
|
"--all",
|
|
"--name",
|
|
"beta",
|
|
str(output_dir),
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
assert sorted(downloaded_ids) == ["audio_beta1", "audio_beta2"]
|
|
downloaded_files = list(output_dir.glob("*.mp3"))
|
|
assert len(downloaded_files) == 2
|
|
|
|
def test_name_filter_dry_run_previews_only_matches(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""`--all --name <name> --dry-run` previews only matching artifacts."""
|
|
mock_client = create_mock_client()
|
|
output_dir = tmp_path / "downloads"
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_alpha", "Alpha Briefing", 1),
|
|
make_artifact("audio_beta1", "Beta Chapter One", 1),
|
|
make_artifact("audio_gamma", "Gamma Recap", 1),
|
|
]
|
|
)
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"audio",
|
|
"--all",
|
|
"--name",
|
|
"beta",
|
|
"--dry-run",
|
|
"--json",
|
|
str(output_dir),
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
payload = json.loads(result.output)
|
|
assert payload.get("dry_run") is True
|
|
assert payload.get("count") == 1
|
|
assert len(payload.get("artifacts", [])) == 1
|
|
assert payload["artifacts"][0]["id"] == "audio_beta1"
|
|
|
|
def test_name_filter_no_matches_returns_error(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""`--all --name <name>` with no matches: error envelope, non-zero exit."""
|
|
mock_client = create_mock_client()
|
|
output_dir = tmp_path / "downloads"
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_alpha", "Alpha Briefing", 1),
|
|
make_artifact("audio_gamma", "Gamma Recap", 1),
|
|
]
|
|
)
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"audio",
|
|
"--all",
|
|
"--name",
|
|
"nonexistent",
|
|
str(output_dir),
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
# Output should mention the filter that produced no matches
|
|
assert "nonexistent" in result.output.lower() or "no" in result.output.lower()
|
|
|
|
|
|
class TestDownloadAllDryRunFilenameParity:
|
|
"""P1.T4 §3 — dry-run and execution must produce the same final filenames.
|
|
|
|
Previously, the dry-run preview passed an empty `existing_names` set to
|
|
``artifact_title_to_filename`` for every artifact in the loop, so duplicate
|
|
titles all collapsed to the same base filename in preview output. The
|
|
execution path accumulates an ``existing_names`` set across iterations and
|
|
auto-renames duplicates to ``Title (2).ext``, ``Title (3).ext``, etc.
|
|
Dry-run must mirror the same disambiguation.
|
|
"""
|
|
|
|
def test_dry_run_disambiguates_duplicate_titles(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""Three artifacts with the same title produce three distinct filenames
|
|
in dry-run output, matching what the execution path would emit."""
|
|
mock_client = create_mock_client()
|
|
output_dir = tmp_path / "downloads"
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_1", "Duplicate Title", 1),
|
|
make_artifact("audio_2", "Duplicate Title", 1),
|
|
make_artifact("audio_3", "Duplicate Title", 1),
|
|
]
|
|
)
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"audio",
|
|
"--all",
|
|
"--dry-run",
|
|
"--json",
|
|
str(output_dir),
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
payload = json.loads(result.output)
|
|
assert payload.get("dry_run") is True
|
|
filenames = [a["filename"] for a in payload["artifacts"]]
|
|
# All three filenames must be distinct (the second and third get
|
|
# auto-renamed via (2)/(3) suffixes — same as the execution path).
|
|
assert len(set(filenames)) == 3
|
|
assert "Duplicate Title.mp3" in filenames
|
|
assert "Duplicate Title (2).mp3" in filenames
|
|
assert "Duplicate Title (3).mp3" in filenames
|
|
|
|
def test_dry_run_matches_execution_filenames(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""The filenames listed in dry-run must be the actual filenames the
|
|
execution path writes to disk."""
|
|
mock_client = create_mock_client()
|
|
output_dir_dry = tmp_path / "dry"
|
|
|
|
artifacts_payload = [
|
|
make_artifact("audio_1", "Duplicate Title", 1),
|
|
make_artifact("audio_2", "Duplicate Title", 1),
|
|
]
|
|
|
|
mock_client.artifacts.list = AsyncMock(return_value=artifacts_payload)
|
|
|
|
# First: dry-run pass.
|
|
result_dry = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"audio",
|
|
"--all",
|
|
"--dry-run",
|
|
"--json",
|
|
str(output_dir_dry),
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
assert result_dry.exit_code == 0
|
|
payload_dry = json.loads(result_dry.output)
|
|
dry_filenames = sorted(a["filename"] for a in payload_dry["artifacts"])
|
|
|
|
# Second: real execution pass (separate patch context — runner resets
|
|
# the client mock between invocations).
|
|
mock_client = create_mock_client()
|
|
output_dir_exec = tmp_path / "exec"
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
Path(output_path).write_bytes(b"audio content")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(return_value=artifacts_payload)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
result_exec = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"audio",
|
|
"--all",
|
|
"--json",
|
|
str(output_dir_exec),
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result_exec.exit_code == 0
|
|
payload_exec = json.loads(result_exec.output)
|
|
exec_filenames = sorted(a["filename"] for a in payload_exec["artifacts"])
|
|
assert dry_filenames == exec_filenames
|
|
|
|
def test_download_all_with_no_clobber(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""Test --all --no-clobber skips existing files."""
|
|
mock_client = create_mock_client()
|
|
|
|
output_dir = tmp_path / "downloads"
|
|
output_dir.mkdir(parents=True)
|
|
# Create existing file
|
|
(output_dir / "First Audio.mp3").write_bytes(b"existing")
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
Path(output_path).write_bytes(b"new content")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_1", "First Audio", 1),
|
|
make_artifact("audio_2", "Second Audio", 1),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--all", "--no-clobber", str(output_dir), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
# First file should remain unchanged
|
|
assert (output_dir / "First Audio.mp3").read_bytes() == b"existing"
|
|
# Second file should be downloaded
|
|
assert (output_dir / "Second Audio.mp3").exists()
|
|
|
|
|
|
# =============================================================================
|
|
# DOWNLOAD ERROR HANDLING TESTS
|
|
# =============================================================================
|
|
|
|
|
|
class TestDownloadArtifactFlag:
|
|
"""Test -a / --artifact flag for selecting by artifact ID."""
|
|
|
|
def test_download_by_full_artifact_id(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""Full artifact ID (20+ chars) bypasses prefix search and selects correctly."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "audio.mp3"
|
|
downloaded_ids = []
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
downloaded_ids.append(artifact_id)
|
|
Path(output_path).write_bytes(b"audio")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_aaa111bbb222ccc333", "First", 1),
|
|
make_artifact("audio_bbb222ccc333ddd444", "Second", 1),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"audio",
|
|
str(output_file),
|
|
"-a",
|
|
"audio_bbb222ccc333ddd444",
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
assert downloaded_ids == ["audio_bbb222ccc333ddd444"]
|
|
|
|
def test_download_full_id_not_in_list_errors(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""Full-length ID (20+ chars) that doesn't exist in the artifact list should error."""
|
|
mock_client = create_mock_client()
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("audio_aaa111bbb222ccc333", "Only", 1)]
|
|
)
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "-a", "nonexistentidtwentyplus1", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
assert "not found" in result.output.lower()
|
|
|
|
def test_download_by_partial_artifact_id(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""Partial artifact ID prefix resolves and selects the correct artifact."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "audio.mp3"
|
|
downloaded_ids = []
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
downloaded_ids.append(artifact_id)
|
|
Path(output_path).write_bytes(b"audio")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_aaa111", "First", 1),
|
|
make_artifact("audio_bbb222", "Second", 1),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", str(output_file), "-a", "audio_bbb", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
assert downloaded_ids == ["audio_bbb222"]
|
|
|
|
def test_download_ambiguous_partial_id_errors(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""Partial ID matching multiple artifacts produces an error."""
|
|
mock_client = create_mock_client()
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_artifact("audio_aaa111", "First", 1),
|
|
make_artifact("audio_aaa222", "Second", 1),
|
|
]
|
|
)
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "-a", "audio_aaa", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
assert "Ambiguous" in result.output
|
|
|
|
def test_download_partial_id_not_found_errors(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""Partial ID matching nothing produces an error."""
|
|
mock_client = create_mock_client()
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("audio_aaa111", "First", 1)]
|
|
)
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "-a", "zzz", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
assert "not found" in result.output.lower()
|
|
|
|
|
|
class TestDownloadErrorHandling:
|
|
"""Test error handling during downloads."""
|
|
|
|
def test_download_single_failure(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""When download fails, should return error gracefully."""
|
|
mock_client = create_mock_client()
|
|
|
|
output_file = tmp_path / "audio.mp3"
|
|
|
|
async def mock_download_audio(notebook_id, output_path, artifact_id=None, **kwargs):
|
|
raise Exception("Connection refused")
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("audio_123", "Audio", 1)]
|
|
)
|
|
mock_client.artifacts.download_audio = mock_download_audio
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", str(output_file), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
assert "Connection refused" in result.output or "error" in result.output.lower()
|
|
|
|
def test_download_name_not_found(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""When --name matches no artifacts, should show helpful error."""
|
|
mock_client = create_mock_client()
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_artifact("audio_123", "My Audio", 1)]
|
|
)
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--name", "nonexistent", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
# Should mention no match found or available artifacts
|
|
assert "No artifact" in result.output or "nonexistent" in result.output.lower()
|
|
|
|
|
|
# =============================================================================
|
|
# DOWNLOAD QUIZ + FLASHCARDS STANDARD-FLAG TESTS
|
|
# =============================================================================
|
|
|
|
|
|
def make_quiz_artifact(
|
|
id: str, title: str, status: int = 3, created_at: datetime | None = None
|
|
) -> Artifact:
|
|
"""Quiz artifact factory: type=4 + variant=2 → ArtifactType.QUIZ."""
|
|
return Artifact(
|
|
id=id,
|
|
title=title,
|
|
_artifact_type=4,
|
|
status=status,
|
|
created_at=created_at or datetime.fromtimestamp(1234567890),
|
|
_variant=2,
|
|
)
|
|
|
|
|
|
def make_flashcard_artifact(
|
|
id: str, title: str, status: int = 3, created_at: datetime | None = None
|
|
) -> Artifact:
|
|
"""Flashcard artifact factory: type=4 + variant=1 → ArtifactType.FLASHCARDS."""
|
|
return Artifact(
|
|
id=id,
|
|
title=title,
|
|
_artifact_type=4,
|
|
status=status,
|
|
created_at=created_at or datetime.fromtimestamp(1234567890),
|
|
_variant=1,
|
|
)
|
|
|
|
|
|
class TestDownloadQuizStandardFlags:
|
|
"""Smoke tests for the standard flag set on `download quiz`."""
|
|
|
|
def test_quiz_help_lists_full_flag_set(self, runner):
|
|
"""Each flag from the standard download flag set must appear in --help."""
|
|
result = runner.invoke(cli, ["download", "quiz", "--help"])
|
|
assert result.exit_code == 0
|
|
for flag in (
|
|
"--latest",
|
|
"--earliest",
|
|
"--all",
|
|
"--name",
|
|
"--artifact",
|
|
"--json",
|
|
"--dry-run",
|
|
"--force",
|
|
"--no-clobber",
|
|
"--format",
|
|
):
|
|
assert flag in result.output, f"missing flag in `download quiz --help`: {flag}"
|
|
|
|
def test_quiz_basic_download_writes_file(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""Single-artifact download writes the file via the dispatched API call."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "quiz.json"
|
|
|
|
async def fake_download_quiz(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
Path(output_path).write_text('{"questions": []}')
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_quiz_artifact("quiz_1", "Chapter 1 Quiz")]
|
|
)
|
|
mock_client.artifacts.download_quiz = fake_download_quiz
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "quiz", str(output_file), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert output_file.exists()
|
|
|
|
def test_quiz_latest_flag_selects_newest(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""--latest picks the newest completed quiz artifact."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "quiz.json"
|
|
chosen_ids: list[str | None] = []
|
|
|
|
async def fake_download_quiz(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
chosen_ids.append(artifact_id)
|
|
Path(output_path).write_text("{}")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_quiz_artifact(
|
|
"quiz_old", "Old", created_at=datetime.fromtimestamp(1000000000)
|
|
),
|
|
make_quiz_artifact(
|
|
"quiz_new", "New", created_at=datetime.fromtimestamp(2000000000)
|
|
),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_quiz = fake_download_quiz
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "quiz", str(output_file), "--latest", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert chosen_ids == ["quiz_new"]
|
|
|
|
def test_quiz_earliest_flag_selects_oldest(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""--earliest picks the oldest completed quiz artifact."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "quiz.json"
|
|
chosen_ids: list[str | None] = []
|
|
|
|
async def fake_download_quiz(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
chosen_ids.append(artifact_id)
|
|
Path(output_path).write_text("{}")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_quiz_artifact(
|
|
"quiz_old", "Old", created_at=datetime.fromtimestamp(1000000000)
|
|
),
|
|
make_quiz_artifact(
|
|
"quiz_new", "New", created_at=datetime.fromtimestamp(2000000000)
|
|
),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_quiz = fake_download_quiz
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "quiz", str(output_file), "--earliest", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert chosen_ids == ["quiz_old"]
|
|
|
|
def test_quiz_name_filter(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""--name picks the artifact whose title fuzzy-matches."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "quiz.json"
|
|
chosen_ids: list[str | None] = []
|
|
|
|
async def fake_download_quiz(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
chosen_ids.append(artifact_id)
|
|
Path(output_path).write_text("{}")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_quiz_artifact("quiz_a", "Chapter 1 Basics"),
|
|
make_quiz_artifact("quiz_b", "Final Exam Review"),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_quiz = fake_download_quiz
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "quiz", str(output_file), "--name", "Final", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert chosen_ids == ["quiz_b"]
|
|
|
|
def test_quiz_dry_run_does_not_download(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""--dry-run shows preview without invoking the API."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "quiz.json"
|
|
api_calls: list[str] = []
|
|
|
|
async def fake_download_quiz(*args, **kwargs):
|
|
api_calls.append("called")
|
|
return ""
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_quiz_artifact("quiz_1", "Quiz One")]
|
|
)
|
|
mock_client.artifacts.download_quiz = fake_download_quiz
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "quiz", str(output_file), "--dry-run", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert "DRY RUN" in result.output
|
|
assert api_calls == [] # No actual download
|
|
assert not output_file.exists()
|
|
|
|
def test_quiz_all_flag_downloads_each(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""--all batch-downloads every completed quiz to the target directory."""
|
|
mock_client = create_mock_client()
|
|
output_dir = tmp_path / "quizzes"
|
|
downloaded: list[str | None] = []
|
|
|
|
async def fake_download_quiz(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
downloaded.append(artifact_id)
|
|
Path(output_path).write_text('{"q": []}')
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_quiz_artifact("quiz_1", "First Quiz"),
|
|
make_quiz_artifact("quiz_2", "Second Quiz"),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_quiz = fake_download_quiz
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "quiz", "--all", str(output_dir), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert sorted(downloaded) == ["quiz_1", "quiz_2"]
|
|
assert len(list(output_dir.glob("*.json"))) == 2
|
|
|
|
def test_quiz_force_overwrites_existing_file(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""--force overwrites a file that already exists at output_path."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "quiz.json"
|
|
output_file.write_text("OLD")
|
|
|
|
async def fake_download_quiz(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
Path(output_path).write_text("NEW")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_quiz_artifact("quiz_1", "Quiz One")]
|
|
)
|
|
mock_client.artifacts.download_quiz = fake_download_quiz
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "quiz", str(output_file), "--force", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert output_file.read_text() == "NEW"
|
|
|
|
def test_quiz_no_clobber_skips_existing_file(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""--no-clobber leaves an existing file untouched."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "quiz.json"
|
|
output_file.write_text("EXISTING")
|
|
|
|
async def fake_download_quiz(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
Path(output_path).write_text("OVERWROTE")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_quiz_artifact("quiz_1", "Quiz One")]
|
|
)
|
|
mock_client.artifacts.download_quiz = fake_download_quiz
|
|
|
|
runner.invoke(
|
|
cli,
|
|
["download", "quiz", str(output_file), "--no-clobber", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
# File untouched
|
|
assert output_file.read_text() == "EXISTING"
|
|
|
|
def test_quiz_force_and_no_clobber_conflict(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""--force + --no-clobber must fail with a clear message."""
|
|
mock_client = create_mock_client()
|
|
mock_client.artifacts.list = AsyncMock(return_value=[make_quiz_artifact("quiz_1", "Quiz")])
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "quiz", "--force", "--no-clobber", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code != 0
|
|
assert "Cannot specify both --force and --no-clobber" in result.output
|
|
|
|
def test_quiz_json_output_emits_json(self, runner, mock_auth, mock_fetch_tokens, tmp_path):
|
|
"""--json emits a parseable JSON document on success."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "quiz.json"
|
|
|
|
async def fake_download_quiz(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
Path(output_path).write_text("{}")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_quiz_artifact("quiz_1", "Quiz One")]
|
|
)
|
|
mock_client.artifacts.download_quiz = fake_download_quiz
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "quiz", str(output_file), "--json", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
data = json.loads(result.output)
|
|
assert data["operation"] == "download_single"
|
|
assert data["status"] == "downloaded"
|
|
assert data["artifact"]["id"] == "quiz_1"
|
|
|
|
def test_quiz_format_markdown_passes_through_to_api(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""--format markdown propagates output_format='markdown' to the API."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "quiz.md"
|
|
captured: dict[str, str] = {}
|
|
|
|
async def fake_download_quiz(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
captured["output_format"] = output_format
|
|
Path(output_path).write_text("# Quiz")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_quiz_artifact("quiz_1", "Quiz One")]
|
|
)
|
|
mock_client.artifacts.download_quiz = fake_download_quiz
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"quiz",
|
|
str(output_file),
|
|
"--format",
|
|
"markdown",
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert captured["output_format"] == "markdown"
|
|
|
|
|
|
class TestDownloadFlashcardsStandardFlags:
|
|
"""Smoke tests for the standard flag set on `download flashcards`."""
|
|
|
|
def test_flashcards_help_lists_full_flag_set(self, runner):
|
|
"""Each flag from the standard download flag set must appear in --help."""
|
|
result = runner.invoke(cli, ["download", "flashcards", "--help"])
|
|
assert result.exit_code == 0
|
|
for flag in (
|
|
"--latest",
|
|
"--earliest",
|
|
"--all",
|
|
"--name",
|
|
"--artifact",
|
|
"--json",
|
|
"--dry-run",
|
|
"--force",
|
|
"--no-clobber",
|
|
"--format",
|
|
):
|
|
assert flag in result.output, f"missing flag in `download flashcards --help`: {flag}"
|
|
|
|
def test_flashcards_basic_download_writes_file(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""Single-artifact download writes the file via the dispatched API call."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "cards.json"
|
|
|
|
async def fake_download_flashcards(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
Path(output_path).write_text('{"cards": []}')
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_flashcard_artifact("fc_1", "Vocabulary Deck")]
|
|
)
|
|
mock_client.artifacts.download_flashcards = fake_download_flashcards
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "flashcards", str(output_file), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert output_file.exists()
|
|
|
|
def test_flashcards_all_flag_downloads_each(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""--all batch-downloads every completed deck to the target directory."""
|
|
mock_client = create_mock_client()
|
|
output_dir = tmp_path / "flashcards"
|
|
downloaded: list[str | None] = []
|
|
|
|
async def fake_download_flashcards(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
downloaded.append(artifact_id)
|
|
Path(output_path).write_text('{"cards": []}')
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_flashcard_artifact("fc_1", "Deck A"),
|
|
make_flashcard_artifact("fc_2", "Deck B"),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_flashcards = fake_download_flashcards
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "flashcards", "--all", str(output_dir), "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert sorted(downloaded) == ["fc_1", "fc_2"]
|
|
assert len(list(output_dir.glob("*.json"))) == 2
|
|
|
|
def test_flashcards_dry_run_does_not_download(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""--dry-run shows preview without invoking the API."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "cards.json"
|
|
api_calls: list[str] = []
|
|
|
|
async def fake_download_flashcards(*args, **kwargs):
|
|
api_calls.append("called")
|
|
return ""
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_flashcard_artifact("fc_1", "Deck One")]
|
|
)
|
|
mock_client.artifacts.download_flashcards = fake_download_flashcards
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "flashcards", str(output_file), "--dry-run", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert "DRY RUN" in result.output
|
|
assert api_calls == []
|
|
assert not output_file.exists()
|
|
|
|
def test_flashcards_force_overwrites_existing_file(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""--force overwrites a file that already exists at output_path."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "cards.json"
|
|
output_file.write_text("OLD")
|
|
|
|
async def fake_download_flashcards(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
Path(output_path).write_text("NEW")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_flashcard_artifact("fc_1", "Deck One")]
|
|
)
|
|
mock_client.artifacts.download_flashcards = fake_download_flashcards
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "flashcards", str(output_file), "--force", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert output_file.read_text() == "NEW"
|
|
|
|
def test_flashcards_no_clobber_skips_existing_file(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""--no-clobber leaves an existing file untouched."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "cards.json"
|
|
output_file.write_text("EXISTING")
|
|
|
|
async def fake_download_flashcards(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
Path(output_path).write_text("OVERWROTE")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_flashcard_artifact("fc_1", "Deck One")]
|
|
)
|
|
mock_client.artifacts.download_flashcards = fake_download_flashcards
|
|
|
|
runner.invoke(
|
|
cli,
|
|
["download", "flashcards", str(output_file), "--no-clobber", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert output_file.read_text() == "EXISTING"
|
|
|
|
def test_flashcards_json_output_emits_json(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""--json emits a parseable JSON document on success."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "cards.json"
|
|
|
|
async def fake_download_flashcards(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
Path(output_path).write_text("{}")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_flashcard_artifact("fc_1", "Deck One")]
|
|
)
|
|
mock_client.artifacts.download_flashcards = fake_download_flashcards
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "flashcards", str(output_file), "--json", "-n", "nb_123"],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
data = json.loads(result.output)
|
|
assert data["operation"] == "download_single"
|
|
assert data["status"] == "downloaded"
|
|
assert data["artifact"]["id"] == "fc_1"
|
|
|
|
def test_flashcards_format_html_passes_through_to_api(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""--format html propagates output_format='html' to the API."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "cards.html"
|
|
captured: dict[str, str] = {}
|
|
|
|
async def fake_download_flashcards(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
captured["output_format"] = output_format
|
|
Path(output_path).write_text("<html></html>")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[make_flashcard_artifact("fc_1", "Deck One")]
|
|
)
|
|
mock_client.artifacts.download_flashcards = fake_download_flashcards
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"flashcards",
|
|
str(output_file),
|
|
"--format",
|
|
"html",
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert captured["output_format"] == "html"
|
|
|
|
def test_flashcards_artifact_id_selects_specific(
|
|
self, runner, mock_auth, mock_fetch_tokens, tmp_path
|
|
):
|
|
"""-a/--artifact selects a specific deck by ID (partial-match resolution)."""
|
|
mock_client = create_mock_client()
|
|
output_file = tmp_path / "cards.json"
|
|
chosen_ids: list[str | None] = []
|
|
|
|
async def fake_download_flashcards(
|
|
notebook_id, output_path, artifact_id=None, output_format="json", **kwargs
|
|
):
|
|
chosen_ids.append(artifact_id)
|
|
Path(output_path).write_text("{}")
|
|
return output_path
|
|
|
|
mock_client.artifacts.list = AsyncMock(
|
|
return_value=[
|
|
make_flashcard_artifact("fc_aaa111", "Deck A"),
|
|
make_flashcard_artifact("fc_bbb222", "Deck B"),
|
|
]
|
|
)
|
|
mock_client.artifacts.download_flashcards = fake_download_flashcards
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
[
|
|
"download",
|
|
"flashcards",
|
|
str(output_file),
|
|
"-a",
|
|
"fc_bbb",
|
|
"-n",
|
|
"nb_123",
|
|
],
|
|
obj=inject_client(mock_client),
|
|
)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert chosen_ids == ["fc_bbb222"]
|
|
|
|
|
|
# =============================================================================
|
|
# DOWNLOAD TYPED ERROR PATH TESTS
|
|
# =============================================================================
|
|
#
|
|
# These tests pin the contract that `download <type>` exception paths route
|
|
# through `cli.error_handler` (the typed handler) rather than the legacy
|
|
# `helpers.handle_error()` shim that always exits 1 with a text-only message
|
|
# regardless of `--json`. The handler is invoked from
|
|
# `_run_artifact_download` and applies to every `download` subcommand
|
|
# (audio/video/slide-deck/...); we exercise `download audio` as a
|
|
# representative because the dispatch is shared.
|
|
#
|
|
# Contract under test:
|
|
# - --json honored on the exception path: emits a JSON envelope of shape
|
|
# {"error": true, "code": "<TYPED_CODE>", "message": "..."} on stdout.
|
|
# - RateLimitError surfaces `retry_after` in the JSON body and "Retry after
|
|
# <N>s" in text mode.
|
|
# - AuthError surfaces a re-authentication hint
|
|
# ("Run 'notebooklm login' to re-authenticate.") in text mode.
|
|
# - Typed exit codes from ``handle_errors``:
|
|
# 1 = library/user error (RateLimit, Auth, Validation, Network, ...)
|
|
# 2 = unexpected/system error (anything else)
|
|
# =============================================================================
|
|
|
|
|
|
class TestDownloadTypedErrorPath:
|
|
"""`download <type>` exception paths route through the typed handler."""
|
|
|
|
def _list_raises(self, exc: Exception):
|
|
"""Build a mock client whose `artifacts.list` raises ``exc``.
|
|
|
|
The exception fires at the first awaited artifact listing inside
|
|
``notebooklm._app.download.execute_download``, which surfaces directly
|
|
to the outer ``_run_artifact_download`` exception handler — the exact
|
|
site under test. The single-download / --all per-artifact try/except
|
|
blocks deliberately swallow API errors into ``{"error": ...}`` rows;
|
|
forcing the failure on ``list`` exercises the *typed* handler path.
|
|
"""
|
|
client = create_mock_client()
|
|
client.artifacts.list = AsyncMock(side_effect=exc)
|
|
return client
|
|
|
|
# ----- RateLimitError --------------------------------------------------
|
|
|
|
def test_rate_limit_error_text_exit_code_1(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""RateLimitError surfaces as exit 1 with retry hint in text mode."""
|
|
from notebooklm.exceptions import RateLimitError
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "-n", "nb_123"],
|
|
obj=inject_client(self._list_raises(RateLimitError("Quota exceeded", retry_after=42))),
|
|
)
|
|
|
|
assert result.exit_code == 1, result.output
|
|
# Text-mode message routes through error_handler._output_error → safe_echo
|
|
# to stderr, which CliRunner mixes into result.output.
|
|
assert "Rate limited" in result.output
|
|
assert "42" in result.output # retry_after surfaced
|
|
|
|
def test_rate_limit_error_json_includes_retry_after(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""`--json` emits a typed JSON error envelope with retry_after."""
|
|
from notebooklm.exceptions import RateLimitError
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--json", "-n", "nb_123"],
|
|
obj=inject_client(self._list_raises(RateLimitError("Quota exceeded", retry_after=42))),
|
|
)
|
|
|
|
assert result.exit_code == 1, result.output
|
|
data = json.loads(result.output)
|
|
assert data["error"] is True
|
|
assert data["code"] == "RATE_LIMITED"
|
|
assert data["retry_after"] == 42
|
|
assert "Rate limited" in data["message"]
|
|
|
|
def test_rate_limit_error_json_no_retry_after_omits_field(
|
|
self, runner, mock_auth, mock_fetch_tokens
|
|
):
|
|
"""No retry_after on the exception → field absent from JSON."""
|
|
from notebooklm.exceptions import RateLimitError
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--json", "-n", "nb_123"],
|
|
obj=inject_client(self._list_raises(RateLimitError("Quota exceeded"))),
|
|
)
|
|
|
|
assert result.exit_code == 1, result.output
|
|
data = json.loads(result.output)
|
|
assert data["code"] == "RATE_LIMITED"
|
|
assert "retry_after" not in data
|
|
|
|
# ----- AuthError -------------------------------------------------------
|
|
|
|
def test_auth_error_text_includes_login_hint(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""AuthError on the download path shows the typed re-auth hint."""
|
|
from notebooklm.exceptions import AuthError
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "-n", "nb_123"],
|
|
obj=inject_client(self._list_raises(AuthError("Token expired"))),
|
|
)
|
|
|
|
assert result.exit_code == 1, result.output
|
|
assert "Authentication error" in result.output
|
|
# error_handler.py ships this exact hint for AuthError.
|
|
assert "notebooklm login" in result.output
|
|
|
|
def test_auth_error_json_emits_typed_code(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""`--json` emits {"error": true, "code": "AUTH_ERROR", ...} for AuthError."""
|
|
from notebooklm.exceptions import AuthError
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--json", "-n", "nb_123"],
|
|
obj=inject_client(self._list_raises(AuthError("Token expired"))),
|
|
)
|
|
|
|
assert result.exit_code == 1, result.output
|
|
data = json.loads(result.output)
|
|
assert data["error"] is True
|
|
assert data["code"] == "AUTH_ERROR"
|
|
assert "Token expired" in data["message"]
|
|
|
|
# ----- Unexpected exceptions (typed exit code 2) ------------------------
|
|
|
|
def test_unexpected_exception_exits_with_code_2(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""Unknown exceptions exit 2 per ``handle_errors`` policy."""
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "-n", "nb_123"],
|
|
obj=inject_client(self._list_raises(RuntimeError("kaboom"))),
|
|
)
|
|
|
|
# Legacy helpers.handle_error always exited 1; the typed handler must
|
|
# distinguish user errors (1) from system bugs (2).
|
|
assert result.exit_code == 2, result.output
|
|
assert "Unexpected error" in result.output
|
|
|
|
def test_unexpected_exception_json_envelope(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""`--json` emits {"code": "UNEXPECTED_ERROR"} with exit 2."""
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--json", "-n", "nb_123"],
|
|
obj=inject_client(self._list_raises(RuntimeError("kaboom"))),
|
|
)
|
|
|
|
assert result.exit_code == 2, result.output
|
|
data = json.loads(result.output)
|
|
assert data["error"] is True
|
|
assert data["code"] == "UNEXPECTED_ERROR"
|
|
assert "kaboom" in data["message"]
|
|
|
|
# ----- ValidationError / NetworkError sanity (typed dispatch) ----------
|
|
|
|
def test_validation_error_typed_envelope(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""ValidationError reaches the typed handler with its own code."""
|
|
from notebooklm.exceptions import ValidationError
|
|
|
|
result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "--json", "-n", "nb_123"],
|
|
obj=inject_client(self._list_raises(ValidationError("bad input"))),
|
|
)
|
|
|
|
assert result.exit_code == 1, result.output
|
|
data = json.loads(result.output)
|
|
assert data["code"] == "VALIDATION_ERROR"
|
|
|
|
def test_network_error_typed_envelope(self, runner, mock_auth, mock_fetch_tokens):
|
|
"""NetworkError reaches the typed handler and surfaces its hint in text."""
|
|
from notebooklm.exceptions import NetworkError
|
|
|
|
text_result = runner.invoke(
|
|
cli,
|
|
["download", "audio", "-n", "nb_123"],
|
|
obj=inject_client(self._list_raises(NetworkError("DNS down"))),
|
|
)
|
|
|
|
assert text_result.exit_code == 1, text_result.output
|
|
assert "Network error" in text_result.output
|
|
assert "internet connection" in text_result.output # error_handler hint
|
|
|
|
# ----- JSON happy-path preservation (must not regress shape) -----
|
|
#
|
|
# The single-download JSON happy-path envelope (operation/status/artifact +
|
|
# no error/code keys) is asserted by ``TestDownloadStandardTypes`` above
|
|
# for every per-type command, so no standalone audio happy-path test is
|
|
# needed here. Only the *returned-error* envelope path remains below, since
|
|
# it pins a materially different (legacy free-form ``error`` string) shape.
|
|
|
|
def test_json_returned_error_envelope_unchanged_exit_1(
|
|
self, runner, mock_auth, mock_fetch_tokens
|
|
):
|
|
"""The pre-existing "no completed artifacts" JSON error path is preserved.
|
|
|
|
That branch surfaces a *returned* dict-shaped error from
|
|
the ``DownloadOutcome.NO_ARTIFACTS`` branch (not a raised exception),
|
|
so it must NOT be re-routed through the typed handler — exit 1 with
|
|
the legacy ``error: "<msg>"`` JSON shape is the documented behavior.
|
|
"""
|
|
mock_client = create_mock_client()
|
|
mock_client.artifacts.list = AsyncMock(return_value=[])
|
|
|
|
result = runner.invoke(
|
|
cli, ["download", "audio", "--json", "-n", "nb_123"], obj=inject_client(mock_client)
|
|
)
|
|
|
|
assert result.exit_code == 1, result.output
|
|
data = json.loads(result.output)
|
|
# Legacy returned-dict shape: free-form "error" string, no "code" envelope.
|
|
assert "error" in data
|
|
assert "No completed audio artifacts" in data["error"]
|
|
|
|
# ----- Missing-storage auth bootstrap (regression guard) ---------------
|
|
|
|
def test_missing_storage_routes_to_auth_error_exit_1(self, runner):
|
|
"""A missing ``storage_state.json`` exits 1 via ``handle_auth_error``.
|
|
|
|
Regression guard for the integration-test failure that surfaced after
|
|
the typed-handler swap: the shared auth loader raises
|
|
``FileNotFoundError`` when no auth file exists, and the typed handler
|
|
would otherwise classify it as ``UNEXPECTED_ERROR`` (exit 2). The
|
|
shared runtime catches this exact case and routes it through
|
|
``handle_auth_error`` — ``download`` must do the same so
|
|
``tests/integration/cli_vcr/test_downloads.py`` (which asserts
|
|
``exit_code in (0, 1)`` for unauth invocations) keeps passing.
|
|
"""
|
|
with patch.object(helpers_module, "get_auth_tokens") as mock_get_auth_tokens:
|
|
mock_get_auth_tokens.side_effect = FileNotFoundError(
|
|
"Storage file not found: /tmp/missing/storage_state.json"
|
|
)
|
|
result = runner.invoke(cli, ["download", "audio", "-n", "nb_123"])
|
|
|
|
assert result.exit_code == 1, result.output
|
|
# Rich auth UX (from handle_auth_error) — the literal "Not logged in"
|
|
# header plus the "notebooklm login" remediation hint.
|
|
assert "Not logged in" in result.output
|
|
assert "notebooklm login" in result.output
|
|
|
|
def test_missing_storage_json_emits_auth_required_envelope(self, runner):
|
|
"""Missing storage in --json mode emits AUTH_REQUIRED envelope, exit 1."""
|
|
with patch.object(helpers_module, "get_auth_tokens") as mock_get_auth_tokens:
|
|
mock_get_auth_tokens.side_effect = FileNotFoundError("Storage file not found")
|
|
result = runner.invoke(cli, ["download", "audio", "--json", "-n", "nb_123"])
|
|
|
|
assert result.exit_code == 1, result.output
|
|
data = json.loads(result.output)
|
|
# `helpers.json_error_response` ships shape:
|
|
# {"error": True, "code": "AUTH_REQUIRED", "message": "...", ...}
|
|
assert data["error"] is True
|
|
assert data["code"] == "AUTH_REQUIRED"
|
|
assert "notebooklm login" in data["message"]
|