Skip to content

Fix test issues#270

Merged
dangusev merged 5 commits intomainfrom
fix/fix-tests
Dec 23, 2025
Merged

Fix test issues#270
dangusev merged 5 commits intomainfrom
fix/fix-tests

Conversation

@dangusev
Copy link
Contributor

@dangusev dangusev commented Dec 23, 2025

  • Skip long-running MoondreamLocal tests from execution.
  • Converted many Moondream unit tests to async to ensure proper awaitable cleanup.
  • Fixed SmartTurn tests to use pytest's tmp_path fixture instead of direct access to the temp directory
  • Most important - applied a hack that cleans unused files from GH runner before tests start. This allows us to run integration tests again without no space left on device issues.
  • Updated blockbuster to >=1.5.26 to fix some regressions.

Summary by CodeRabbit

  • Tests

    • Converted test suite to asynchronous execution patterns.
    • Long-running tests configured to skip during test runs.
    • Enhanced test setup and resource management practices.
  • Chores

    • Updated development dependency versions.
    • Implemented CI/CD pipeline cache cleanup for build optimization.

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

@coderabbitai
Copy link

coderabbitai bot commented Dec 23, 2025

Walkthrough

Skips Moondream local tests via a pytest skip marker, converts many Moondream unit tests to async, bumps dev dependency blockbuster lower bound to >=1.5.26 in pyproject.toml, adds a GitHub Actions cleanup step removing specific tool/cache directories, and tweaks SmartTurn test imports and a test signature.

Changes

Cohort / File(s) Summary
Moondream — local skip
plugins/moondream/tests/test_moondream_local.py
Added a pytest skip marker on TestMoondreamLocalProcessor with message "Skip Moondream local tests because they take too long to run".
Moondream — async tests
plugins/moondream/tests/test_moondream.py
Converted numerous synchronous test functions to async def, replaced processor.close() with await processor.close(), and adjusted pytest async markers accordingly.
SmartTurn tests
plugins/smart_turn/tests/test_smart_turn.py
Reordered imports to TurnEndedEvent, TurnStartedEvent; replaced tempfile usage with tmp_path fixture and updated test_silero_predict signature.
Dependency management
pyproject.toml
Bumped dev dependency blockbuster lower bound from >=1.5.5 to >=1.5.26 (upper bound <1.6 unchanged).
CI workflow
.github/workflows/run_tests.yml
Added "Clean space" step after checkout that removes /usr/share/dotnet, /usr/local/lib/android, /usr/local/share/boost, and $AGENT_TOOLSDIRECTORY (ignores missing paths) before dependency install.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • CI improvements #197 — Modifies the same CI workflow (.github/workflows/run_tests.yml) with overlapping test-job changes; likely related to the added cleanup step.

Suggested reviewers

  • tschellenbach
  • Nash0x7E2

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Fix test issues' is vague and generic, using non-descriptive language that does not convey meaningful information about the specific changes made in the changeset. Consider a more descriptive title that highlights the primary change, such as 'Convert Moondream tests to async and skip long-running tests' or 'Fix test infrastructure and async cleanup issues'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 91.67% which is sufficient. The required threshold is 80.00%.
✨ 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/fix-tests

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0b65ebb and a3c4b5a.

📒 Files selected for processing (1)
  • plugins/moondream/tests/test_moondream.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*test*.py

📄 CodeRabbit inference engine (.cursor/rules/python.mdc)

**/*test*.py: Never mock in tests; use pytest for testing
Mark integration tests with @pytest.mark.integration decorator
@pytest.mark.asyncio is not needed - it is automatic

Files:

  • plugins/moondream/tests/test_moondream.py
**/*.py

📄 CodeRabbit inference engine (.cursor/rules/python.mdc)

**/*.py: Never adjust sys.path in Python code
Never write except Exception as e - use specific exception handling
Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access in Python
Docstrings should follow the Google style guide for docstrings

Files:

  • plugins/moondream/tests/test_moondream.py
🧠 Learnings (1)
📚 Learning: 2025-11-24T17:04:43.030Z
Learnt from: CR
Repo: GetStream/Vision-Agents PR: 0
File: .cursor/rules/python.mdc:0-0
Timestamp: 2025-11-24T17:04:43.030Z
Learning: Applies to **/*test*.py : pytest.mark.asyncio is not needed - it is automatic

Applied to files:

  • plugins/moondream/tests/test_moondream.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: unit / Test "not integration"
  • GitHub Check: unit / Mypy
  • GitHub Check: unit / Test "not integration"
  • GitHub Check: unit / Mypy
🔇 Additional comments (4)
plugins/moondream/tests/test_moondream.py (4)

41-45: Async conversion and cleanup pattern looks correct.

The conversion of these tests to async with await processor.close() ensures proper cleanup of async resources and prevents resource leaks. This pattern is consistent and correctly implemented throughout the test suite.

Also applies to: 59-64, 85-98, 101-128, 131-156, 159-187, 190-210, 322-328, 331-337, 340-344, 347-351, 354-360


72-75: Verify mocking pattern against coding guidelines.

The coding guidelines state "Never mock in tests; use pytest for testing." These tests use manual function replacement for mocking API calls. While this enables fast unit tests without API credentials (integration tests separately verify real API behavior), please confirm whether:

  1. The guideline means to use pytest's mocking facilities (e.g., monkeypatch, pytest-mock) instead of manual replacement, or
  2. The tests should be restructured to avoid mocking entirely

Also applies to: 90-91, 218-223

As per coding guidelines, never mock in tests.

Also applies to: 90-91, 218-223


236-273: Integration tests properly structured.

The integration tests are correctly marked with @pytest.mark.integration and include appropriate conditional skips when the API key is unavailable. The pattern of testing against the real Moondream service with proper cleanup is well-implemented.

Also applies to: 276-310, 381-419, 422-467

Also applies to: 276-310, 381-419, 422-467


313-319: Exception-testing pattern correctly remains synchronous.

Tests that expect exceptions during processor initialization appropriately remain synchronous, as no processor instance is successfully created and no async cleanup is needed.

Also applies to: 363-369, 372-378

Also applies to: 363-369, 372-378


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

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
plugins/moondream/tests/test_moondream_local.py (1)

32-32: Consider skipping only integration tests instead of the entire class.

The class contains unit tests (lines 182-293) for device detection logic that use monkeypatching and don't load models—these should execute quickly. Skipping the entire class prevents these fast unit tests from running.

🔎 Alternative approach: Skip only integration tests

Remove the class-level skip decorator and rely on pytest's marker filtering instead:

-@pytest.mark.skip("Skip Moondream local tests because they take too long to run")
 class TestMoondreamLocalProcessor:
     """Test cases for MoondreamLocalProcessor."""

Then run tests with:

# Run only fast unit tests (skip integration tests)
pytest plugins/moondream/tests/test_moondream_local.py -m "not integration"

# Run all tests including integration tests
pytest plugins/moondream/tests/test_moondream_local.py

This preserves the ability to run fast unit tests in CI while allowing integration tests to be skipped by default using pytest marker configuration.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b5649a8 and 20adb43.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • plugins/moondream/tests/test_moondream_local.py
  • pyproject.toml
🧰 Additional context used
📓 Path-based instructions (2)
**/*test*.py

📄 CodeRabbit inference engine (.cursor/rules/python.mdc)

**/*test*.py: Never mock in tests; use pytest for testing
Mark integration tests with @pytest.mark.integration decorator
@pytest.mark.asyncio is not needed - it is automatic

Files:

  • plugins/moondream/tests/test_moondream_local.py
**/*.py

📄 CodeRabbit inference engine (.cursor/rules/python.mdc)

**/*.py: Never adjust sys.path in Python code
Never write except Exception as e - use specific exception handling
Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access in Python
Docstrings should follow the Google style guide for docstrings

Files:

  • plugins/moondream/tests/test_moondream_local.py
🧠 Learnings (1)
📚 Learning: 2025-11-24T17:04:43.030Z
Learnt from: CR
Repo: GetStream/Vision-Agents PR: 0
File: .cursor/rules/python.mdc:0-0
Timestamp: 2025-11-24T17:04:43.030Z
Learning: Applies to **/*test*.py : Mark integration tests with pytest.mark.integration decorator

Applied to files:

  • plugins/moondream/tests/test_moondream_local.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: unit / Test "not integration"
  • GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
  • GitHub Check: unit / Mypy
  • GitHub Check: unit / Ruff
  • GitHub Check: unit / Ruff
  • GitHub Check: unit / Mypy
  • GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
  • GitHub Check: unit / Test "not integration"
🔇 Additional comments (1)
pyproject.toml (1)

97-97: Blockbuster version 1.5.26 is confirmed safe to use.

Version 1.5.26 exists on PyPI with no known vulnerabilities or CVEs. The version bump is valid.

@github-actions github-actions bot added the ci label Dec 23, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
plugins/moondream/tests/test_moondream.py (4)

74-77: Avoid mocking in tests per project guidelines.

The project's coding guidelines explicitly state "Never mock in tests; use pytest for testing." Consider using pytest fixtures or testing against the actual implementation instead of mocking internal methods.

As per coding guidelines, never mock in tests; use pytest for testing.


92-93: Avoid mocking in tests per project guidelines.

This mock violates the project's coding guidelines that state "Never mock in tests; use pytest for testing."

As per coding guidelines, never mock in tests; use pytest for testing.


220-225: Avoid mocking in tests per project guidelines.

This mock violates the project's coding guidelines that explicitly state "Never mock in tests; use pytest for testing."

As per coding guidelines, never mock in tests; use pytest for testing.


242-242: Remove unnecessary @pytest.mark.asyncio decorators from integration tests.

Per the project's coding guidelines, @pytest.mark.asyncio is not needed as asyncio handling is automatic. The @pytest.mark.integration decorator is correct and should be kept, but the @pytest.mark.asyncio decorator should be removed for consistency.

As per coding guidelines, @pytest.mark.asyncio is not needed - it is automatic.

🔎 Proposed fix
 @pytest.mark.integration
 @pytest.mark.skipif(
     not os.getenv("MOONDREAM_API_KEY"), reason="MOONDREAM_API_KEY not set"
 )
-@pytest.mark.asyncio
 async def test_live_detection_api():

Apply similar changes to:

  • test_live_detection_with_annotation (Line 283)
  • test_custom_object_detection (Line 389)
  • test_multiple_object_detection (Line 431)

Also applies to: 283-283, 389-389, 431-431

🧹 Nitpick comments (1)
plugins/smart_turn/tests/test_smart_turn.py (1)

15-16: Consider adding @pytest.mark.integration to the class.

Per coding guidelines, integration tests should be marked with @pytest.mark.integration. All three tests in this class exercise real audio processing, VAD models, and the full turn detection pipeline—characteristics of integration tests rather than unit tests. The @skip_blockbuster marker serves a different purpose (handling unavoidable blocking calls) and can coexist with the integration marker.

As per coding guidelines, integration tests should be marked with the decorator.

🔎 Proposed addition
 @skip_blockbuster
+@pytest.mark.integration
 class TestSmartTurn:
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3eedfd4 and 0b65ebb.

📒 Files selected for processing (2)
  • plugins/moondream/tests/test_moondream.py
  • plugins/smart_turn/tests/test_smart_turn.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*test*.py

📄 CodeRabbit inference engine (.cursor/rules/python.mdc)

**/*test*.py: Never mock in tests; use pytest for testing
Mark integration tests with @pytest.mark.integration decorator
@pytest.mark.asyncio is not needed - it is automatic

Files:

  • plugins/smart_turn/tests/test_smart_turn.py
  • plugins/moondream/tests/test_moondream.py
**/*.py

📄 CodeRabbit inference engine (.cursor/rules/python.mdc)

**/*.py: Never adjust sys.path in Python code
Never write except Exception as e - use specific exception handling
Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access in Python
Docstrings should follow the Google style guide for docstrings

Files:

  • plugins/smart_turn/tests/test_smart_turn.py
  • plugins/moondream/tests/test_moondream.py
🧬 Code graph analysis (1)
plugins/smart_turn/tests/test_smart_turn.py (4)
agents-core/vision_agents/core/turn_detection/events.py (2)
  • TurnEndedEvent (29-46)
  • TurnStartedEvent (11-25)
agents-core/vision_agents/core/vad/silero.py (1)
  • prepare_silero_vad (87-96)
conftest.py (2)
  • skip_blockbuster (29-46)
  • mia_audio_16khz (193-197)
tests/base_test.py (1)
  • mia_audio_16khz (18-58)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: unit / Test "not integration"
  • GitHub Check: unit / Mypy
  • GitHub Check: unit / Mypy
  • GitHub Check: unit / Test "not integration"
🔇 Additional comments (4)
plugins/smart_turn/tests/test_smart_turn.py (1)

23-24: LGTM! Clean migration to pytest's tmp_path fixture.

The shift from manual temporary directory management to pytest's tmp_path fixture improves test isolation and ensures automatic cleanup. The path conversion via as_posix() correctly matches the string parameter expected by prepare_silero_vad.

plugins/moondream/tests/test_moondream.py (3)

41-45: LGTM! Async conversion is clean.

The test has been properly converted to async, and correctly omits the @pytest.mark.asyncio decorator per the project's automatic asyncio handling.


103-130: LGTM! Annotation tests properly converted to async.

All four annotation test functions have been cleanly converted to async, correctly call await processor.close(), and properly omit the @pytest.mark.asyncio decorator.

Also applies to: 133-158, 161-189, 192-212


326-332: LGTM! API key and detect_objects tests properly converted to async.

All five test functions have been cleanly converted to async with proper await processor.close() calls and no unnecessary decorators.

Also applies to: 335-341, 344-348, 351-355, 358-364

@dangusev dangusev changed the title Skip MoondreamLocalProcessor tests for now Fix test issues Dec 23, 2025
@dangusev dangusev merged commit b6116b5 into main Dec 23, 2025
10 checks passed
@dangusev dangusev deleted the fix/fix-tests branch December 23, 2025 14:51
@coderabbitai coderabbitai bot mentioned this pull request Dec 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants