Skip to content

build: configure pytest-django and add dev dependencies#40

Merged
A-lexisL merged 36 commits intodevfrom
feat/web-view-pytest
Mar 1, 2026
Merged

build: configure pytest-django and add dev dependencies#40
A-lexisL merged 36 commits intodevfrom
feat/web-view-pytest

Conversation

@SherryShijiarui
Copy link
Copy Markdown

No description provided.

Comment thread apps/web/models/course.py
Comment thread apps/web/models/course_offering.py Outdated
Copy link
Copy Markdown
Contributor

@A-lexisL A-lexisL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. tests should be comprehensive, so more tests are needed. Please arrange them into classes based on API and Authenticated status.
    e.g.
class TestCourseListAuthenticated
class TestCourseListNotAuthenticated
class TestCourseDetailAuthenticated
...

Comment thread apps/web/tests/test_auth.py Outdated
Comment thread apps/web/tests/test_course.py Outdated
Comment thread apps/web/tests/test_course.py Outdated
Comment thread apps/web/tests/test_course.py
Comment thread apps/web/tests/factories.py Outdated
Comment thread apps/web/tests/conftest.py
Comment thread apps/web/tests/test_review.py Outdated
Comment thread apps/web/tests/test_review.py Outdated
url = reverse("user_review_api", kwargs={"review_id": review.id})
response = auth_client.delete(url)
assert response.status_code == 204
assert Review.objects.filter(id=review.id).count() == 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing tests for base_client

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added unauthenticated coverage in TestReviewAPIUnauthenticated for list, personal list, delete, and detail in apps/web/tests/test_review.py. Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Copy link
Copy Markdown
Contributor

@zzjc1234 zzjc1234 Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added concrete diff for missing base_client tests:

+class TestReviewAPIUnauthenticated:
+    def test_get_course_reviews_anonymous(...):
+    def test_get_personal_reviews_anonymous(...):
+    def test_delete_review_anonymous_forbidden(...):
+    def test_review_detail_anonymous_forbidden(...):

This adds unauthenticated coverage for the review endpoints.

Comment thread apps/web/tests/test_vote.py
Comment thread apps/web/tests/test_course.py
@A-lexisL A-lexisL moved this from Todo to In Progress in Course Review Jan 19, 2026
Copy link
Copy Markdown
Contributor

@A-lexisL A-lexisL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are comprehensive. But some test cases still need polishing.

term_data
and term_data.group("year") == "16"
and term_data.group("term") == "w"
and term_data.group("term") == "F"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the term regex test to check 16W, 16w, and 16F separately in apps/web/tests/lib_tests/test_terms.py. Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Copy link
Copy Markdown
Contributor

@zzjc1234 zzjc1234 Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added concrete diff for the term assertion fix:

 term_data = terms.term_regex.match("16w")
 self.assertTrue(
     term_data
     and term_data.group("year") == "16"
-    and term_data.group("term") == "w"
-    and term_data.group("term") == "F"
+    and term_data.group("term") == "w"
 )
+term_data = terms.term_regex.match("16F")
+self.assertTrue(
+    term_data
+    and term_data.group("year") == "16"
+    and term_data.group("term") == "F"
+)

This removes the contradictory condition and tests both cases explicitly.

Comment thread apps/web/tests/conftest.py Outdated
Comment thread apps/web/tests/test_course.py Outdated
Comment thread apps/web/tests/test_review.py Outdated
Comment thread apps/web/tests/test_review.py
Comment thread apps/web/tests/test_review.py Outdated
Comment thread apps/web/tests/test_review.py Outdated
Comment thread apps/web/tests/test_review.py Outdated
Comment thread apps/web/tests/test_review.py Outdated
Comment thread apps/web/tests/test_review.py
@A-lexisL A-lexisL merged commit c83f2b9 into dev Mar 1, 2026
6 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Course Review Mar 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants