Fix: Port conflict in tracer tests #271
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the MockOpenAICompatibleServer to support dynamic port allocation and updates the global OPENAI_BASE_URL configuration to use the dynamically allocated port. This allows multiple test instances to run concurrently without port conflicts.
- Modified
MockOpenAICompatibleServerto accept an optional port parameter and dynamically allocate one if not provided - Added logic to update and restore the global
OPENAI_BASE_URLwhen the mock server starts and stops - Imported the
socketmodule to enable dynamic port allocation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Update the module-level base URL so downstream clients use the live port. | ||
| global OPENAI_BASE_URL | ||
| self._prev_openai_base_url = OPENAI_BASE_URL | ||
| OPENAI_BASE_URL = f"http://{self.host}:{self.port}/v1" |
There was a problem hiding this comment.
Modifying the global OPENAI_BASE_URL creates a race condition when multiple test instances run concurrently. If tests run in parallel, they will overwrite each other's base URLs, causing tests to connect to the wrong server instances. Consider passing the base URL as a parameter to agent functions or using thread-local storage instead of modifying a global variable.
| if self._prev_openai_base_url is not None: | ||
| global OPENAI_BASE_URL | ||
| OPENAI_BASE_URL = self._prev_openai_base_url | ||
| self._prev_openai_base_url = None |
There was a problem hiding this comment.
The restoration of OPENAI_BASE_URL in __aexit__ does not guarantee correct cleanup in concurrent scenarios. If another test instance modified OPENAI_BASE_URL between __aenter__ and __aexit__, this will overwrite the other instance's URL with a stale value. This cleanup logic is not thread-safe.
|
/ci |
|
🚀 CI Watcher for correlation id-3488946253-mhled1mh triggered by comment 3488946253
✅ All runs completed. |
(cherry picked from commit f717f99)
|
Successfully created backport PR for |
(cherry picked from commit f717f99) Co-authored-by: Yuge Zhang <[email protected]>
No description provided.