Add unit tests for Global HMAC authentication methods#164
Conversation
Co-authored-by: DasBasti <1713093+DasBasti@users.noreply.github.com>
Co-authored-by: DasBasti <1713093+DasBasti@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive unit test coverage for the Global HMAC authentication methods introduced in PR #161. The tests validate the _login_global() and _refresh_access_token_global() authentication flows that were previously untested.
Changes:
- Added 8 unit tests covering success paths, error conditions, and auth mode routing for Global HMAC authentication
- Created mock JSON responses (
global_login_result.jsonandglobal_refresh_result.json) to simulate Global API endpoints - Tests verify proper token extraction, error handling for missing fields, and correct routing based on
SmartAuthMode.GLOBAL_HMAC
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| pysmarthashtag/tests/test_authentication.py | Adds comprehensive test suite with 8 tests covering Global HMAC login, token refresh, error handling, and auth mode routing |
| pysmarthashtag/tests/replys/global_login_result.json | Mock response for successful global login with all required token fields |
| pysmarthashtag/tests/replys/global_refresh_result.json | Mock response for successful global token refresh with refreshed token values |
| ) | ||
|
|
||
| # Should raise SmartAPIError when access_token is missing | ||
| with pytest.raises(Exception) as exc_info: |
There was a problem hiding this comment.
The test should catch the specific exception type SmartAPIError rather than the generic Exception. The implementation in api/authentication.py lines 406-407 raises SmartAPIError, so the test should verify that specific exception type is raised. This provides more precise testing and better documents the expected behavior.
There was a problem hiding this comment.
Changed all three exception tests to use specific SmartAPIError type and removed unnecessary API session endpoint mocks from global authentication tests in commit c4b8fd9.
| ) | ||
|
|
||
| # Should raise SmartAPIError when userId is missing | ||
| with pytest.raises(Exception) as exc_info: |
There was a problem hiding this comment.
The test should catch the specific exception type SmartAPIError rather than the generic Exception. The implementation in api/authentication.py lines 406-407 raises SmartAPIError, so the test should verify that specific exception type is raised. This provides more precise testing and better documents the expected behavior.
| ) | ||
|
|
||
| # Should raise SmartAPIError when data is missing | ||
| with pytest.raises(Exception) as exc_info: |
There was a problem hiding this comment.
The test should catch the specific exception type SmartAPIError rather than the generic Exception. The implementation in api/authentication.py lines 396-399 raises SmartAPIError, so the test should verify that specific exception type is raised. This provides more precise testing and better documents the expected behavior.
| SmartAuthMode, | ||
| SmartRegion, | ||
| get_endpoint_urls_for_region, | ||
| ) |
There was a problem hiding this comment.
To use SmartAPIError in the exception tests (lines 233, 260, 284), you need to add it to the imports. Add from pysmarthashtag.models import SmartAPIError to the imports section.
| ) | |
| ) | |
| from pysmarthashtag.models import SmartAPIError |
| # Mock API session endpoint | ||
| respx.post(GLOBAL_API_BASE_URL + "/iam/service/api/v1/session?identity_type=smart").mock( | ||
| return_value=Response(200, json=load_response(RESPONSE_DIR / "api_access.json")) | ||
| ) |
There was a problem hiding this comment.
This mock for the API session endpoint is unnecessary for the global login flow. Unlike the EU OAuth flow, _login_global() does not call _get_api_session() - it returns the access token directly as the api_access_token (see lines 413-421 in api/authentication.py). This mock can be safely removed as it adds confusion about what endpoints are actually being called during global authentication.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: DasBasti <1713093+DasBasti@users.noreply.github.com>
_login_global()method_refresh_access_token_global()methodSmartAPIErrorexception type in tests💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.