test(auth): overhaul auth tests and certificate parser with full cove…#290
test(auth): overhaul auth tests and certificate parser with full cove…#290zennintoji29-create wants to merge 1 commit intoEswaramuthu:mainfrom
Conversation
…rage - Restructure test_auth.py and test_protected_routes.py into classes - Add cross-role, logout, edge-case and session integrity tests - Refactor certificate_parser.py to return typed ParsedCertificate - Add logging, datetime parsing, and whitespace normalisation to parser - Apply type hints and docstrings throughout per CONTRIBUTING guidelines
|
@zennintoji29-create is attempting to deploy a commit to the 007's projects Team on Vercel. A member of the Team first needs to authorize it. |
Thanks for creating a PR for your Issue!
|
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
|
|
||
| # ── Session seed helpers ────────────────────────────────────────────────────── | ||
|
|
||
| def _seed_student_session(client, student_id: str = "S12345") -> None: |
There was a problem hiding this comment.
Duplicate Code:
This function _seed_student_session duplicates existing code.
📍 Original Location:
tests/conftest.py:91-98
Function: auth_student_client
💡 Recommendation:
Replace _seed_student_session with a call to the auth_student_client fixture (inject it as a test parameter). Alternatively, keep the helper but have it delegate to the conftest fixture to avoid the duplicated session-key dictionary.
Consider importing and reusing the existing function instead of duplicating the logic.
|
|
||
| def test_authenticated_teacher_access(client): | ||
| """Test that authenticated teacher can access their dashboard.""" | ||
| def _seed_teacher_session(client, teacher_id: str = "T001") -> None: |
There was a problem hiding this comment.
Duplicate Code:
This function _seed_teacher_session duplicates existing code.
📍 Original Location:
tests/conftest.py:100-108
Function: auth_teacher_client
💡 Recommendation:
Use the auth_teacher_client fixture from conftest instead of the duplicate helper function.
Consider importing and reusing the existing function instead of duplicating the logic.
| # ── Unauthenticated redirect tests ──────────────────────────────────────────── | ||
|
|
||
| class TestUnauthenticatedRedirects: | ||
| """All protected routes must redirect an anonymous visitor to login.""" | ||
|
|
||
| def test_student_dashboard_redirects_to_login(self, client) -> None: |
There was a problem hiding this comment.
Duplicate Code:
This function TestUnauthenticatedRedirects.test_student_dashboard_redirects_to_login duplicates existing code.
📍 Original Location:
tests/test_protected_routes.py:2-6
Function: test_student_dashboard_protected
💡 Recommendation:
Remove the existing test_student_dashboard_protected and keep the PR version inside the TestUnauthenticatedRedirects class for better organisation. No logic changes needed.
Consider importing and reusing the existing function instead of duplicating the logic.
| response = client.get("/student-dashboard", follow_redirects=False) | ||
|
|
||
| assert response.status_code == 302 | ||
| assert "/student" in response.location | ||
|
|
||
| def test_teacher_dashboard_redirects_to_login(self, client) -> None: |
There was a problem hiding this comment.
Duplicate Code:
This function TestUnauthenticatedRedirects.test_teacher_dashboard_redirects_to_login duplicates existing code.
📍 Original Location:
tests/test_protected_routes.py:8-12
Function: test_teacher_dashboard_protected
💡 Recommendation:
Remove test_teacher_dashboard_protected from the repo and keep the PR version. No logic changes needed.
Consider importing and reusing the existing function instead of duplicating the logic.
1 participant
…rage
Which issue does this PR close?
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?