build: configure pytest-django and add dev dependencies by SherryShijiarui · Pull Request #40 · Tech-JI/CourseReview

SherryShijiarui · 2026-01-03T23:09:26Z

No description provided.

apps/web/models/course.py

apps/web/models/course_offering.py

A-lexisL

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
...

apps/web/tests/test_auth.py

apps/web/tests/test_course.py

A-lexisL · 2026-01-19T02:56:15Z

apps/web/tests/test_course.py

+        assert "results" in response.data
+
+    def test_filter_courses_by_department(self, base_client, course_factory):
+        """Verify filtering courses by department code."""


For now, only filter courses by department code for nonauthenticated user exists. But tests for all the following params are needed for authenticated/nonauthenticated users.

- department (string): Filter by department code (case-insensitive) - code (string): Filter by course code (partial match) - min_quality (integer): Filter by minimum quality score (authenticated only) - min_difficulty (integer): Filter by minimum difficulty score (authenticated only) - sort_by (string): Sort field ("course_code", "review_count"),("quality_score", "difficulty_score")(authenticated only) - sort_order (string): "asc" or "desc" (default: "asc") - page (integer): Page number for pagination

(docstring of CoursesListAPI in apps/web/views.py)

Same for other apis

Added coverage for all documented query params across auth states in apps/web/tests/test_course.py (filters, min_quality/min_difficulty, review_count/quality_score/difficulty_score sorts, sort_order). Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Added concrete diff for parameter coverage:

+@pytest.mark.django_db +class TestCourseAPIUnauthenticated: + def test_filter_courses_by_department(...): + def test_filter_courses_by_code(...): + def test_sort_courses_by_review_count_anonymous(...): + def test_sort_courses_by_score_anonymous(...): + def test_filter_courses_by_score_anonymous(...): + def test_filter_courses_by_difficulty_anonymous(...): + +@pytest.mark.django_db +class TestCourseAPIAuthenticated: + def test_filter_courses_by_department_authenticated(...): + def test_filter_courses_by_code_authenticated(...): + def test_sort_by_review_count(...): + def test_sort_courses_by_score(...): + def test_sort_courses_by_difficulty_score(...): + def test_filter_courses_by_quality(...): + def test_filter_courses_by_difficulty(...): + def test_sort_order_asc_and_desc(...):

These tests are now split by auth state and cover the documented query parameters.

apps/web/tests/factories.py

apps/web/tests/conftest.py

apps/web/tests/test_review.py

A-lexisL · 2026-01-19T03:32:40Z

apps/web/tests/test_review.py

+        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


Missing tests for base_client

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.

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.

apps/web/tests/test_vote.py

A-lexisL · 2026-01-19T03:40:44Z

apps/web/tests/test_course.py

+
+        assert response.status_code == 200
+        # Verify the title matches the fixture-created course
+        assert response.data["course_title"] == course.course_title


base_client and auth_client will receive different response data:

auth_client: full details

base_client: "quality_score" ,"difficulty_score","difficulty_vote","quality_vote","quality_vote_count","difficulty_vote_count" fields are removed(the docstrings for CoursesDetailAPI is not complete)

Added course detail field assertions for auth vs unauth in apps/web/tests/test_course.py (scores and votes hidden for base_client, present for auth_client). Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Added concrete diff for auth vs unauth response fields:

+def test_course_detail_fields_unauthenticated(self, base_client, course): + hidden_fields = [ + "quality_score", "difficulty_score", + "difficulty_vote", "quality_vote", + "quality_vote_count", "difficulty_vote_count", + ] + for field in hidden_fields: + assert field not in response.data + +def test_course_detail_fields_authenticated(self, auth_client, course): + required_fields = [ + "quality_score", "difficulty_score", + "difficulty_vote", "quality_vote", + "quality_vote_count", "difficulty_vote_count", + ] + for field in required_fields: + assert field in response.data

This directly validates the serializer behavior difference.

…ated test

…fix the format of the course code later

…ew.py

…sc and desc) pagnition)

A-lexisL

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

A-lexisL · 2026-02-02T06:04:13Z

apps/web/tests/lib_tests/test_terms.py

            term_data
            and term_data.group("year") == "16"
            and term_data.group("term") == "w"
+            and term_data.group("term") == "F"


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.

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.

A-lexisL · 2026-02-02T06:04:52Z

apps/web/tests/conftest.py

+@pytest.fixture
+def department_mixed_courses(db):
+    """Returns a mixed set of courses for filtering/sorting tests."""
+    # Note: Using 'title' to match your original course.py


since title is changed to course_title, this comment is outdated

Updated the fixture comment to reference course_title in apps/web/tests/conftest.py. Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Added concrete diff for the outdated fixture comment:

-# Note: Using title to match your original course.py +# Note: Using course_title to match current course model field

A-lexisL · 2026-02-02T06:42:55Z

apps/web/tests/test_course.py

+        resp_p1 = auth_client.get(url, {"page": 1})
+
+        assert resp_p1.status_code == 200
+        assert len(resp_p1.data["results"]) == 10


10 is hardcoded.
Its not very necessary to test pagination, as it has nothing to do with data correctness/authentication access

Removed the pagination hardcode expectation; course list tests now focus on filter and sort behavior in apps/web/tests/test_course.py. Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Added concrete diff for removing pagination hardcode:

-def test_pagination_with_default_settings(self, auth_client, course_factory): - for i in range(11): - course_factory(course_code=f"CODE_{i:02d}") - resp_p1 = auth_client.get(reverse("courses_api"), {"page": 1}) - assert len(resp_p1.data["results"]) == 10

Course tests now focus on correctness of filtering and sorting behavior.

A-lexisL · 2026-02-02T06:45:53Z

apps/web/tests/test_review.py

+            status.HTTP_403_FORBIDDEN,
+        ]
+
+    def test_department_api_empty(self, base_client, db):


This could be removed: department_api is already tested in test_course.py

Removed department_api checks from apps/web/tests/test_review.py; department coverage is in apps/web/tests/test_course.py. Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Added concrete diff for removing department API assertions from review tests:

-def test_department_api_empty(self, base_client, db): - ... - -def test_department_api_sorting(self, base_client, db): - ...

Department API coverage is kept in apps/web/tests/test_course.py.

A-lexisL · 2026-02-02T06:51:57Z

apps/web/tests/test_review.py

+        """5. Verify user can list their own reviews."""
+        response = auth_client.get(personal_reviews_list_url)
+        assert response.status_code == status.HTTP_200_OK
+        assert any(r["id"] == review.id for r in response.data)


should check if the api returns and only returns personal reviews. e.g. review + other_review and the api only returns review

Updated test_list_personal_reviews to include other_review and assert only the current user review is returned in apps/web/tests/test_review.py. Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Added concrete diff below:

-def test_list_personal_reviews(self, auth_client, personal_reviews_list_url, review): +def test_list_personal_reviews(self, auth_client, personal_reviews_list_url, review, other_review): response = auth_client.get(personal_reviews_list_url) assert response.status_code == status.HTTP_200_OK assert any(r["id"] == review.id for r in response.data) + assert all(r["id"] != other_review.id for r in response.data)

This now verifies the API returns only personal reviews.

A-lexisL · 2026-02-02T07:04:09Z

apps/web/tests/test_review.py

+
+        ReviewFactory(course=course, professor="UniqueProf", comments="c" * min_len)
+        response = auth_client.get(course_reviews_url, {"q": "UniqueProf"})
+        assert any(r["professor"] == "UniqueProf" for r in response.data)


should be all
Although search review could also match partial UniqueProf in professor/comment, in this case only reviews that ["professor"] == "UniqueProf" would be returned

Search test now uses all(...) and adds a different professor so only UniqueProf results are returned. Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Added concrete diff for strict search assertion:

ReviewFactory(course=course, professor="UniqueProf", comments="c" * min_len) +ReviewFactory(course=course, professor="OtherProf", comments="c" * min_len) response = auth_client.get(course_reviews_url, {"q": "UniqueProf"}) -assert any(r["professor"] == "UniqueProf" for r in response.data) +assert all(r["professor"] == "UniqueProf" for r in response.data)

A-lexisL · 2026-02-02T07:09:35Z

apps/web/tests/test_review.py

+        self, auth_client, personal_review_detail_url, review, valid_review_data
+    ):
+        """9. Verify successful update of user's own review."""
+        valid_review_data["comments"] = "Updated content that is long enough."


min length is according to config. So this comment may not be long enough. Use 'b' * min_len

Updated comment length usage to min_len so validation aligns with config in apps/web/tests/test_review.py. Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Added concrete diff for config-safe update payload length:

-valid_review_data["comments"] = "Updated content that is long enough." +valid_review_data["comments"] = "b" * len(valid_review_data["comments"])

This keeps the updated value aligned with the configured minimum length.

A-lexisL · 2026-02-02T07:11:24Z

apps/web/tests/test_review.py

+        ]
+        assert Review.objects.filter(id=review.id).exists()
+
+    def test_department_api_sorting(self, base_client, db):


could be removed.

Removed the redundant non-existent delete test in apps/web/tests/test_review.py. Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Added concrete diff for removing the redundant test:

-def test_delete_non_existent_id(self, auth_client): - url = reverse("user_review_api", kwargs={"review_id": 99999}) - response = auth_client.delete(url) - assert response.status_code == status.HTTP_404_NOT_FOUND

A-lexisL · 2026-02-02T07:13:02Z

apps/web/tests/test_review.py

+    ):
+        """14. Verify PUT fails if required fields (professor) are missing."""
+        response = auth_client.put(
+            personal_review_detail_url, {"comments": "Valid length..."}, format="json"


still min length problem.
Dont know whether 400 is caused by min length or missing fields

Missing-field validation test now uses min_len comments so 400 is due to missing required fields, not length. Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Added concrete diff to isolate missing-field validation from length validation:

-def test_update_validation_missing_field(self, auth_client, personal_review_detail_url): +def test_update_validation_missing_field(self, auth_client, personal_review_detail_url, min_len): response = auth_client.put( personal_review_detail_url, - {"comments": "Valid length..."}, + {"comments": "b" * min_len}, format="json", )\n```\nNow the 400 is specifically from missing required fields.

A-lexisL · 2026-02-02T07:16:17Z

apps/web/tests/test_review.py

+    ):
+        """19. Verify vote statistics are included in the response."""
+        response = auth_client.get(personal_review_detail_url)
+        assert "kudos_count" in response.data


also test dislike_count

Added dislike_count assertion in test_review_response_contains_votes in apps/web/tests/test_review.py. Evidence: .venv/bin/pytest apps/web/tests -> 52 passed.

Added concrete diff for vote stats assertion:

response = auth_client.get(personal_review_detail_url) assert "kudos_count" in response.data +assert "dislike_count" in response.data

SherryShijiarui added 12 commits January 4, 2026 07:05

build: configure pytest-django and add dev dependencies

1149dab

fix: update factory compatibility

8d3ed21

fix: increase field lengths and update Django+migrations

a3fc806

build: test_auth.py

54059f0

fix: resolve model naming and factory syntax errors

5459ae8

fix: implement robust data factories for course and student

cbd6269

build: test_auth.py test_course.py test_review.py

3ab511e

fix: test_review.py

bcc4148

fix: add more function to test_auth.py and test_course.py

6439b8f

fix: add more function to test_review.py

6a44f11

fix: add more function to test_vote.py

706f8c5

reset to original database

656cb5d

PACHAKUTlQ reviewed Jan 12, 2026

View reviewed changes

apps/web/models/course.py Show resolved Hide resolved

PACHAKUTlQ reviewed Jan 12, 2026

View reviewed changes

apps/web/models/course_offering.py Outdated Show resolved Hide resolved

SherryShijiarui added 3 commits January 12, 2026 22:07

reset to original py

7a3cf02

fix: fix test.review.py

2cff119

fix: push 0012_vote_web_vote_course__b117a9_idx.py

74cb780

A-lexisL requested changes Jan 19, 2026

View reviewed changes

A-lexisL added this to Course Review Jan 19, 2026

github-project-automation bot moved this to Todo in Course Review Jan 19, 2026

A-lexisL moved this from Todo to In Progress in Course Review Jan 19, 2026

liaotian756 and others added 5 commits January 19, 2026 16:32

test: add new vote tests (difficulty, change, cancel) and unauthentic…

088539c

…ated test

add tests(filtering and sorting courses with permission check) TODO: …

bdf5b2e

…fix the format of the course code later

fix: fix course_code in factories.py

8997bda

fix: make test more precise in test_auth.py

38da811

FIX: use the right format of course code

bc0334e

liaotian756 force-pushed the feat/web-view-pytest branch from d40df23 to bc0334e Compare January 20, 2026 01:10

SherryShijiarui added 3 commits January 20, 2026 09:22

fix: make test_auth.py more conprehensive with 0 and multiple cases

23d6f05

build: add more fixture to conftest.py

a314ed8

build: add more fixture to conftest.py

97460ba

SherryShijiarui added 5 commits January 20, 2026 10:47

build: add more fixture to conftest.py and add more test to test_revi…

8d3f870

…ew.py

fix: fix course_title in conftest.py and course code in test_review.py

08bd759

fix: fix test

82b7d1f

build: add base_client in test_review.py and test_vote.py

5aec605

add tests: sort by review_count filter by min_difficulty sort order(a…

fa1f27d

…sc and desc) pagnition)

A-lexisL requested changes Feb 2, 2026

View reviewed changes

delete test: pagnition

5a7a0b1

liaotian756 force-pushed the feat/web-view-pytest branch from f0ba2fa to 5a7a0b1 Compare February 2, 2026 14:41

zzjc1234 added 3 commits February 17, 2026 22:27

test: group API tests by auth state and add coverage

a7cbe24

test: fix term assertions and fixture comment

2ad2872

build: update uv lockfile

88ba7f6

Conversation

SherryShijiarui commented Jan 3, 2026

Uh oh!

Uh oh!

Uh oh!

A-lexisL left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zzjc1234 Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zzjc1234 Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zzjc1234 Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

A-lexisL left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zzjc1234 Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zzjc1234 Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zzjc1234 Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zzjc1234 Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zzjc1234 Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

zzjc1234 Feb 17, 2026 •

edited

Loading

zzjc1234 Feb 17, 2026 •

edited

Loading

zzjc1234 Feb 17, 2026 •

edited

Loading

zzjc1234 Feb 17, 2026 •

edited

Loading

zzjc1234 Feb 17, 2026 •

edited

Loading

zzjc1234 Feb 17, 2026 •

edited

Loading

zzjc1234 Feb 17, 2026 •

edited

Loading

zzjc1234 Feb 17, 2026 •

edited

Loading

zzjc1234 Feb 17, 2026 •

edited

Loading

zzjc1234 Feb 17, 2026 •

edited

Loading

zzjc1234 Feb 17, 2026 •

edited

Loading

zzjc1234 Feb 17, 2026 •

edited

Loading