Skip to content

fix: aiohttp test_init_with_loop broken test - #1697

Merged
WilliamBergamin merged 1 commit into
mainfrom
fix-broken-test-related-to-aiohttp
Jun 2, 2025
Merged

fix: aiohttp test_init_with_loop broken test#1697
WilliamBergamin merged 1 commit into
mainfrom
fix-broken-test-related-to-aiohttp

Conversation

@WilliamBergamin

Copy link
Copy Markdown
Contributor

Summary

tests/slack_sdk_async/socket_mode/test_aiohttp.py:test_init_with_loop was broken by aiohttpversion 3.12.6 with the following error

RuntimeError: Session and connector has to use same event loop
env_3.13.1/lib/python3.13/site-packages/aiohttp/client.py:368: RuntimeError

The test aims to validate that a loop can be passed to the SocketModeClient, previously a new loop was created and passed to the SocketModeClient constructor, this PR changes this behavior by passing the existing loop related to the unit tests.

Testing

N/A

Category

  • slack_sdk.web.WebClient (sync/async) (Web API client)
  • slack_sdk.webhook.WebhookClient (sync/async) (Incoming Webhook, response_url sender)
  • slack_sdk.socket_mode (Socket Mode client)
  • slack_sdk.signature (Request Signature Verifier)
  • slack_sdk.oauth (OAuth Flow Utilities)
  • slack_sdk.models (UI component builders)
  • slack_sdk.scim (SCIM API client)
  • slack_sdk.audit_logs (Audit Logs API client)
  • slack_sdk.rtm_v2 (RTM client)
  • /docs (Documents)
  • /tutorial (PythOnBoardingBot tutorial)
  • tests/integration_tests (Automated tests for this library)

Requirements

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh after making the changes.

@codecov

codecovBot commented Jun 2, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.33%. Comparing base (443ca03) to head (4ebf647).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@ Coverage Diff @@## main #1697 +/- ##
=======================================
Coverage 85.33% 85.33% =======================================
Files 113 113 Lines 12808 12808 =======================================
Hits 10930 10930 Misses 1878 1878 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zimegzimeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@WilliamBergamin LGTM! And thanks for a fast find and fix 🙏 ✨

I notice we're using a ranged version for testing requirements which seems alright, though I'm wondering if we might want to pin this to the current latest and attempt updates on a schedule?

pytest-asyncio<1 # for async

AFAICT we run tests with the latest but I'm not sure if we're signaling support for earlier versions with this? No blocker - all questions! 🚢 💨

@WilliamBergamin

Copy link
Copy Markdown
ContributorAuthor

Yess this is something I think I will bring up internally 💯

Having some sort of regression testing may help us detect broken tests like this before they are triggered by unrelated changes

@WilliamBergamin
WilliamBergamin merged commit 27fae2f into mainJun 2, 2025
@WilliamBergamin
WilliamBergamin deleted the fix-broken-test-related-to-aiohttp branch June 2, 2025 20:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:asyncsocket-modetestsM-T: Testing work only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@WilliamBergamin@zimeg