fix: Remove test warnings from improper patching #1342
Conversation
jonpspri
requested review from
crivetimihai,
kevalmahajan and
madhav165
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR fixes test warnings caused by improper mock patching in the E2E test suite. The main issue was that the mock_settings fixture used nested patch contexts incorrectly, which has been resolved by using MagicMock(wraps=) instead. Additionally, the PR includes code cleanup: removing duplicate UTF-8 encoding pragmas, consolidating duplicate JWT generation functions, adding type ignore comments for pyrefly, and applying consistent code formatting.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/e2e/test_main_apis.py
Outdated
|
|
||
| resource_data = { | ||
| "resource": urllib.parse.quote_plus('{"uri":"config/formtest","name":"form_test","description":"Form resource","mimeType":"application/json","content":"{\"key\":\"value\"}"}'), | ||
| "resource": urllib.parse.quote_plus('{"uri":"config/formtest","name":"form_test","description":"Form resource","mimeType":"application/json","content":"{"key":"value"}"}'), |
There was a problem hiding this comment.
The JSON string contains unescaped double quotes inside the 'content' field, which will cause a syntax error. The inner JSON object's quotes need to be escaped: \"content\":\"{\\\"key\\\":\\\"value\\\"}\"
There was a problem hiding this comment.
Corrected using a python raw string and backslashes.
|
Holding behind PR #1331 |
jonpspri
force-pushed
the
fix/test_main_apis
branch
from
25f5928 to
0b18db8
Compare
Signed-off-by: Jonathan Springer <[email protected]>
Fixed issues in e2e tests caused by multi-tenancy (team) implementation: - Added "teams": [] to JWT token payloads in test_admin_apis.py and test_main_apis.py to allow access to public resources and own private resources - Set "visibility": "public" for test servers, prompts, and resources to ensure proper access with public-only tokens - Simplified settings mock in test_admin_apis.py (replaced patch with MagicMock) - Fixed string escaping in resource data test (test_main_apis.py:979) - Cleaned up logging setup in test_admin_apis.py Added missing pytest-timeout>=2.4.0 package to dev dependencies to ensure timeout decorations are properly honored during test execution. Signed-off-by: Jonathan Springer <[email protected]>
crivetimihai
force-pushed
the
fix/test_main_apis
branch
from
0b18db8 to
2e56af2
Compare
2 participants
Summary
Resolves warnings in
tests/e2e/test_main_apis.pycaused by improper mock patching and fixes e2e test failures caused by multi-tenancy implementation.Changes
Multi-Tenancy Test Fixes
"teams": []to JWT token payloads intest_admin_apis.pyandtest_main_apis.pyto allow access to public resources and own private resources"visibility": "public"for test servers, prompts, and resources to ensure proper access with public-only tokenstest_main_apis.py:979)Test Fixes (
tests/e2e/test_main_apis.pyandtests/e2e/test_admin_apis.py)mock_settingsfixture to useMagicMock(wraps=)instead of nested patch contexttest_admin_apis.py(replaced patch with MagicMock)test_admin_apis.pyDependencies
pytest-timeout>=2.4.0package to dev dependencies to ensure timeout decorations are properly honored during test executionTest Plan
tests/e2e/test_main_apis.pyandtests/e2e/test_admin_apis.pyexecute without warningsFixes warnings in test execution and multi-tenancy test failures that were causing noise in test output and CI logs.
Signed-off-by: Jonathan Springer [email protected]