Skip to content

Conversation

@zzstoatzz
Copy link
Collaborator

@zzstoatzz zzstoatzz commented Oct 22, 2025

only run unit/integration tests on library/test/ci changes and fix warnings

ai yap summary

  • Fix CI workflow to only run tests when relevant files change
  • Explicitly acknowledge expected warnings in tests instead of suppressing them

Changes

CI Workflow

Added paths filter to pull_request trigger in .github/workflows/run-tests.yml so tests only run when:

  • src/**
  • tests/**
  • uv.lock
  • pyproject.toml
  • .github/workflows/**

This prevents unnecessary test runs on PRs that only touch examples or documentation.

Test Warnings

Replaced warning suppression with explicit assertions using pytest.warns():

  1. Deprecation warning for server.sse_app() in tests/client/test_sse.py

    • Validates that the deprecation warning is correctly shown to users
  2. UserWarning for in-memory token storage in OAuth tests:

    • tests/client/auth/test_oauth_client.py
    • tests/client/test_client.py
    • tests/test_mcp_config.py
    • Validates that users get warned about production usage

These changes ensure warnings are intentionally tested rather than silently suppressed, confirming they work as designed.

🤖 Generated with Claude Code

zzstoatzz and others added 3 commits October 21, 2025 20:33
only run tests when relevant files change (src/, tests/, uv.lock, pyproject.toml, .github/workflows/)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- deprecation warning for sse_app method in test_sse.py
- in-memory token storage warning in oauth client tests

instead of suppressing warnings in pytest config, explicitly assert that expected warnings are raised. this validates that the warnings are working as intended.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
add pytest.warns assertions for remaining OAuth tests that trigger the in-memory token storage warning in test_client.py and test_mcp_config.py

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@marvin-context-protocol marvin-context-protocol bot added enhancement Improvement to existing functionality. For issues and smaller PR improvements. tests labels Oct 22, 2025
@zzstoatzz zzstoatzz marked this pull request as ready for review October 22, 2025 01:41
@zzstoatzz zzstoatzz requested a review from Copilot October 22, 2025 01:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves CI efficiency and test quality by filtering test runs to relevant file changes and explicitly validating expected warnings instead of suppressing them. The changes ensure warnings are intentionally tested and documented, confirming they work as designed.

Key changes:

  • Added path filters to the GitHub Actions workflow to skip test runs on documentation-only changes
  • Converted warning suppressions to explicit pytest.warns() assertions for deprecation and security warnings

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
.github/workflows/run-tests.yml Added path filters to only trigger tests on source, test, or configuration changes
tests/client/test_sse.py Added explicit assertion for sse_app() deprecation warning
tests/client/test_client.py Added explicit assertions for in-memory OAuth token storage warnings across multiple test cases
tests/client/auth/test_oauth_client.py Added explicit assertion for in-memory token storage warning in fixture
tests/test_mcp_config.py Added explicit assertion for in-memory token storage warning in config test
- "pyproject.toml"
- ".github/workflows/**"

# run on all pull requests because these checks are required and will block merges otherwise
Copy link
Owner

Choose a reason for hiding this comment

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

@zzstoatzz the issue is that if someone opens a doc PR, these tests wont run and they're required to pass in order to merge (I can override, and do, but i wish there was a better way)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement to existing functionality. For issues and smaller PR improvements. tests

3 participants