fix: allow non s prefixed models to fail websocket integration tests

[twangodev]

Summary by CodeRabbit

Release Notes

• Tests
  • Refactored WebSocket streaming tests to provide consistent, model-specific validation across different scenarios.
  • Enhanced test reporting by explicitly marking known unstable model scenarios as expected failures, improving clarity and reliability of test results.
  • Simplified test execution flow while maintaining comprehensive coverage validation.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The WebSocket TTS integration test was refactored to use pytest parametrization instead of internal model iteration. Test methods now run once per model with xfail marks applied to non-s1 prefixed models, replacing the previous loop-based approach while maintaining equivalent coverage.

Changes

Cohort / File(s)

Summary

Test refactoring tests/integration/test_tts_websocket_integration.py

Renamed test_websocket_streaming_with_different_models and test_async_websocket_streaming_with_different_models to test_websocket_streaming_with_model and test_async_websocket_streaming_with_model respectively. Added @pytest.mark.parametrize("model", [...]) decorators with xfail marks for non-s1 models. Removed internal model iteration loops; tests now accept a single model parameter and stream using that model directly. Adjusted audio saving to occur after streaming completes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Verify parametrize decorator logic correctly marks non-s1 models as xfail
• Confirm test coverage remains equivalent despite the refactoring from loop-based to parametrized approach
• Review the model list construction to ensure no models are omitted from testing

Possibly related PRs

• feat: add tests for testing all wss model backends #53: Modifies the same WebSocket TTS integration test file, directly related to model iteration test refactoring

Poem

🐰 A rabbit hops with joy today,
Tests now parametrize the way!
No loops to lose, just models clean,
Xfail marks shine, a neat machine!
Coverage holds, the streams still flow,
One test per model—bravo! 🎉

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: converting parametrized tests to selectively mark non-s1 prefixed models as xfail, allowing expected failures while maintaining test coverage.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/integration-tests

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11ed28b and 9dbe67a.

📒 Files selected for processing (1)

• tests/integration/test_tts_websocket_integration.py (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

tests/integration/test_tts_websocket_integration.py (2)

tests/integration/conftest.py (2)

• client (33-41)
• save_audio (57-82)

src/fishaudio/resources/tts.py (2)

• stream_websocket (206-354)
• stream_websocket (509-650)

🔇 Additional comments (4)

tests/integration/test_tts_websocket_integration.py (4)

52-62: LGTM! Well-structured parametrized test.

The refactoring correctly uses the parametrized model parameter, maintains equivalent functionality, and provides model-specific assertions and output filenames for better test traceability.

---

230-243: LGTM! Consistent parametrization with sync test.

The async test uses the same parametrization pattern as the synchronous version, ensuring consistency across both test classes.

---

244-261: LGTM! Async test properly refactored.

The async test method correctly mirrors the synchronous version's refactoring, using the parametrized model parameter and maintaining equivalent async functionality with proper model-specific assertions and output.

---

38-51: Parametrization approach is correct and well-designed.

The Model type contains three models—"speech-1.5", "speech-1.6", and "s1"—and the parametrization logic correctly identifies "s1" as the modern model for positive test coverage while marking legacy models with xfail. The test properly receives and uses the model parameter.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR refactors WebSocket integration tests to use pytest's parametrize decorator instead of looping through models within a single test. It marks legacy models (non-"s1" prefixed) as expected failures using xfail, allowing the test suite to pass while acknowledging known unreliability issues with WebSocket connections for older models.

Key Changes:

• Converted loop-based model testing to parametrized tests for better isolation
• Added conditional xfail markers for models that don't start with "s1"
• Removed inter-test delays (time.sleep/asyncio.sleep) as pytest parametrize provides proper test isolation

---

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