feat(tests): test authentication #19
Conversation
WalkthroughThe changes make the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Function
participant ApiLib as ApiLib Instance
participant Server as REST API Server
Test->>ApiLib: Create instance (with or without token)
Test->>ApiLib: Call _call("/authenticated")
ApiLib->>Server: HTTP GET /authenticated (with/without Authorization header)
Server-->>ApiLib: 200 OK (if authorized) or 401 Unauthorized (if not)
ApiLib-->>Test: Return response
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
test/.env (2)
3-3: Fix the typo in the environment variable name.The variable name
REST_API_TOKNappears to be missing the 'E' in TOKEN. Consider renaming it toREST_API_TOKENfor consistency and clarity.-REST_API_TOKN="test_token" +REST_API_TOKEN="test_token"
1-3: Consider adding a comment about the test-only nature of these credentials.While this is clearly in a test directory, adding a comment clarifying that these are test-only credentials would improve clarity and prevent accidental misuse.
+# Test environment configuration - DO NOT use in production REST_API_HOST="localhost" REST_API_PORT="5001" REST_API_TOKN="test_token"test/config/rest_api.py (1)
10-16: Consider enhancing the authentication validation.The current implementation only checks for the presence of an Authorization header but doesn't validate its content. For more realistic testing, consider validating the token value against the expected test token.
@app.route("/authenticated", methods=["GET"]) def authenticated(): token = request.headers.get("Authorization") - if not token: + if not token or token != "Bearer test_token": return jsonify({"error": "Unauthorized"}), 401 return jsonify({"message": "Authenticated successfully"}), 200test/test_api_lib.py (1)
91-97: Consider testing response content for completeness.While status code verification is essential, consider also asserting the response content to ensure the endpoint returns expected messages.
@pytest.mark.asyncio async def test_authenticated_call(api: ApiLib, api_not_authenticated: ApiLib): status, resp = await api._call(Method.GET, "/authenticated") assert status == 200 + assert resp["message"] == "Authenticated successfully" status, resp = await api_not_authenticated._call(Method.GET, "/authenticated") assert status == 401 + assert resp["error"] == "Unauthorized"test/test_objects/test_request.py (1)
5-5: Consider extracting test values as constants for maintainability.While direct instantiation is good, the repeated hardcoded values across tests could be improved by extracting them as constants at the module level.
+# Test constants +TEST_FIELD_VALUE = "test_value" +TEST_PATH_VALUE = "path_value" + def test_request_data_as_dict(): - request_object = RequestClass("test_value", "path_value") + request_object = RequestClass(TEST_FIELD_VALUE, TEST_PATH_VALUE)Apply similar changes to other test functions for consistency.
Also applies to: 17-17, 22-22, 27-27
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
api_lib/api_lib.py(2 hunks)test/.env(1 hunks)test/config/rest_api.py(2 hunks)test/conftest.py(2 hunks)test/test_api_lib.py(1 hunks)test/test_headers/test_accept.py(0 hunks)test/test_objects/test_request.py(2 hunks)test/test_objects/test_response.py(0 hunks)
💤 Files with no reviewable changes (2)
- test/test_headers/test_accept.py
- test/test_objects/test_response.py
🧰 Additional context used
🧬 Code Graph Analysis (3)
test/test_api_lib.py (3)
test/conftest.py (2)
api(21-22)api_not_authenticated(31-32)api_lib/api_lib.py (2)
ApiLib(14-201)_call(28-57)api_lib/method.py (1)
Method(4-9)
api_lib/api_lib.py (1)
api_lib/headers/authorization.py (1)
Authorization(4-5)
test/test_objects/test_request.py (2)
test/config/request.py (1)
RequestClass(13-16)api_lib/objects/request.py (2)
as_header_string(26-27)as_query_parameters(30-31)
🔇 Additional comments (9)
test/config/rest_api.py (2)
4-4: LGTM: Import addition is appropriate.The addition of
requestto the Flask imports is correct and necessary for the new authenticated endpoint.
57-59: LGTM: Function parameterization improves flexibility.The updated
run_serverfunction withhostandportparameters provides better configurability for the test infrastructure.api_lib/api_lib.py (3)
20-20: LGTM: Type annotation correctly reflects optional token.The change to
Optional[Authorization]with defaultNoneproperly supports unauthenticated API clients.
44-44: LGTM: Safe token header access prevents AttributeError.The use of
getattr(self.token, "header", {})correctly handles the case whenself.tokenisNone, providing an empty dictionary as fallback headers.
20-25: Verify consistency with the typo in environment variable name.The constructor accepts an
Authorizationtoken, but the test environment file usesREST_API_TOKN(missing 'E'). Ensure the environment variable name is corrected to maintain consistency.#!/bin/bash # Description: Check if the environment variable typo affects other files # Search for usage of the misspelled environment variable name rg "REST_API_TOKN" --type py # Search for the correct spelling to see if there's inconsistency rg "REST_API_TOKEN" --type pytest/test_api_lib.py (1)
91-97: LGTM: Comprehensive authentication test with both positive and negative cases.The test correctly verifies that:
- Authenticated requests to
/authenticatedendpoint return 200 status- Unauthenticated requests return 401 status
This provides good coverage for the new optional authentication feature.
test/test_objects/test_request.py (1)
4-5: LGTM: Direct instantiation simplifies test setup.The change from fixture-based to direct instantiation of
RequestClassis clear and reduces unnecessary test dependencies.test/conftest.py (2)
31-32: LGTM! Good addition of unauthenticated fixture.The
api_not_authenticatedfixture is well-implemented and provides a clean way to test API calls without authentication tokens.
37-43: Verify the server startup time reduction.The server startup wait time was reduced from 1 second to 0.2 seconds. While this improves test performance, ensure this is sufficient for reliable server startup across different environments.
Consider running tests multiple times to verify 0.2 seconds is consistently adequate:
#!/bin/bash # Description: Test server startup reliability with reduced wait time # Expected: All tests should pass consistently for i in {1..10}; do echo "Test run $i:" python -m pytest test/ -v --tb=short if [ $? -ne 0 ]; then echo "Test failed on run $i" break fi done
Summary by CodeRabbit
New Features
Bug Fixes
Chores