Skip to content

feat: add integration tests for ASR functionality using sample audio files #120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
twangodev wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add integration tests for ASR functionality using sample audio files #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added samples/jfk.wav
View file
Open in desktop
Binary file not shown.
59 changes: 59 additions & 0 deletions tests/integration/test_asr_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
"""Integration tests for ASR functionality."""

from pathlib import Path

import pytest

from fishaudio.types import ASRResponse, TTSConfig

SAMPLES_DIR = Path(__file__).resolve().parents[2] / "samples"


class TestASRIntegration:
"""Test ASR with real API."""
Expand Down Expand Up @@ -45,6 +49,61 @@ def test_asr_without_timestamps(self, client, sample_audio):
# Segments might still be present but potentially empty or without timing


class TestASRFromFileIntegration:
"""Test ASR using a sample audio file with known content."""

@pytest.fixture
def jfk_audio(self):
"""Load the JFK sample audio file."""
return (SAMPLES_DIR / "jfk.wav").read_bytes()
Comment on lines +55 to +58
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

The jfk_audio fixture is duplicated in both sync and async test classes. Consider defining it once (e.g., at module level, optionally with a broader scope like module/session) to avoid repetition and repeated disk I/O.

Copilot uses AI. Check for mistakes.

def test_asr_from_file(self, client, jfk_audio):
"""Test transcription of a known audio file."""
result = client.asr.transcribe(audio=jfk_audio, language="en")

assert isinstance(result, ASRResponse)
assert result.duration > 0
# JFK's famous quote
text = result.text.lower()
assert "ask not what your country can do for you" in text

def test_asr_from_file_with_timestamps(self, client, jfk_audio):
"""Test transcription with timestamps from a known audio file."""
result = client.asr.transcribe(audio=jfk_audio, language="en")
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

test_asr_from_file_with_timestamps relies on the default include_timestamps=True. Making this explicit in the call would prevent the test from silently changing meaning if the client default ever changes.

Suggested change
result = client.asr.transcribe(audio=jfk_audio, language="en")
result = client.asr.transcribe(audio=jfk_audio, language="en", include_timestamps=True)

Copilot uses AI. Check for mistakes.
Comment on lines +70 to +72
Copy link

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Pass include_timestamps=True explicitly.

Line 72 currently depends on the SDK default. If that default changes, this test stops exercising the timestamped path and becomes a false positive.

🛠️ Suggested fix 
-        result = client.asr.transcribe(audio=jfk_audio, language="en")
+        result = client.asr.transcribe(
+            audio=jfk_audio,
+            language="en",
+            include_timestamps=True,
+        )
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/integration/test_asr_integration.py` around lines 70 - 72, The test
function test_asr_from_file_with_timestamps should explicitly pass
include_timestamps=True to the SDK call so it exercises the
timestamped-transcription path regardless of SDK defaults; update the call to
client.asr.transcribe(audio=jfk_audio, language="en", include_timestamps=True)
to ensure the test asserts timestamp behavior deterministically.


assert len(result.segments) > 0
for segment in result.segments:
assert segment.text
assert segment.start >= 0
assert segment.end > segment.start

def test_asr_from_file_without_timestamps(self, client, jfk_audio):
"""Test transcription without timestamps from a known audio file."""
result = client.asr.transcribe(audio=jfk_audio, include_timestamps=False)

assert isinstance(result, ASRResponse)
assert result.text
Comment on lines +80 to +85
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

test_asr_from_file_without_timestamps doesn’t currently assert anything about timestamps/segments (it only checks result.text). To validate include_timestamps=False, assert that result.segments is empty (or that segment timing fields are absent, depending on the API contract).

Copilot uses AI. Check for mistakes.
Comment on lines +80 to +85
Copy link

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Assert that timestamps are actually omitted.

This test still passes if include_timestamps=False is ignored and segments are returned, so it doesn't verify the behavior its name promises. src/fishaudio/types/asr.py:20-31 documents segments as empty when timestamps are disabled. While touching this, pass language="en" here too so the test only varies the timestamp flag.

🛠️ Suggested fix 
-        result = client.asr.transcribe(audio=jfk_audio, include_timestamps=False)
+        result = client.asr.transcribe(
+            audio=jfk_audio,
+            language="en",
+            include_timestamps=False,
+        )
 
         assert isinstance(result, ASRResponse)
         assert result.text
+        assert not result.segments
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/integration/test_asr_integration.py` around lines 80 - 85, Update the
test_asr_from_file_without_timestamps test to also assert that the ASRResponse
contains no segments when include_timestamps=False and to pass language="en" to
transcribe; specifically, call client.asr.transcribe(audio=jfk_audio,
include_timestamps=False, language="en") and then assert isinstance(result,
ASRResponse), assert result.text, and assert that result.segments is empty
(e.g., len(result.segments) == 0 or result.segments == []), referencing the
ASRResponse and client.asr.transcribe usage.



class TestAsyncASRFromFileIntegration:
"""Test async ASR using a sample audio file."""

@pytest.fixture
def jfk_audio(self):
"""Load the JFK sample audio file."""
return (SAMPLES_DIR / "jfk.wav").read_bytes()

@pytest.mark.asyncio
async def test_async_asr_from_file(self, async_client, jfk_audio):
"""Test async transcription of a known audio file."""
result = await async_client.asr.transcribe(audio=jfk_audio, language="en")

assert isinstance(result, ASRResponse)
assert result.text
assert result.duration > 0
assert "ask not what your country can do for you" in result.text.lower()


class TestAsyncASRIntegration:
"""Test async ASR with real API."""

Expand Down