-
Notifications
You must be signed in to change notification settings - Fork 1.4k
ci: fix test workflow triggers and acknowledge test warnings #2202
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.
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
base: main
Are you sure you want to change the base?
Conversation
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>
There was a problem hiding this 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 |
There was a problem hiding this comment.
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)
only run unit/integration tests on library/test/ci changes and fix warnings
ai yap summary
Changes
CI Workflow
Added
pathsfilter topull_requesttrigger in.github/workflows/run-tests.ymlso tests only run when:src/**tests/**uv.lockpyproject.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():Deprecation warning for
server.sse_app()intests/client/test_sse.pyUserWarning for in-memory token storage in OAuth tests:
tests/client/auth/test_oauth_client.pytests/client/test_client.pytests/test_mcp_config.pyThese changes ensure warnings are intentionally tested rather than silently suppressed, confirming they work as designed.
🤖 Generated with Claude Code