PR 4: (#150) Implementing Unit Tests for Lessons Model (Part 2)#152
PR 4: (#150) Implementing Unit Tests for Lessons Model (Part 2)#152cj-ballesteros merged 8 commits intomainfrom
Conversation
EthanZ23
added
enhancement
ChrisCRL
left a comment
ChrisCRL
left a comment
There was a problem hiding this comment.
🔍 Peer Review: Assignment System API Tests
Hey Ethan👋 — Here's my feedback:
✅ What Looks Great
- Test coverage is extensive, covering all CRUD operations for Assignments, Quizzes, Questions, Choices, and Solutions.
- Authentication handling is properly implemented in the setUp method, ensuring tests run with proper permissions.
- Test organization is excellent, with clear delineation between different model test sections.
- Verification steps go beyond simply checking response status codes - you're verifying data integrity and confirming deletion.
💡 Suggestions for Improvement
- Consider adding negative test cases (e.g., attempting operations without proper authentication or permissions).
- Add tests for validation errors (e.g., creating questions with invalid data, choices without required fields).
🧪 Tested This Feature
I ran the following test steps:
- ✅ Reviewed all test cases for proper setup and teardown
- ✅ Verified test coverage across all models in the assignment system
- ✅ Confirmed proper test isolation to prevent test interdependence
- ✅ Checked that all API endpoints are properly tested
- ✅ Validated that permissions are correctly tested where applicable
🚀 Final Thoughts
The tests are well-structured and maintain a good balance between thoroughness and readability. With the addition of some negative test cases and validation testing, this model will be perfect. Approved :) SEND ITTTTTTTT (after the 2nd reviewer)
| return f"Solution for Question {self.question.id}: (No Answer Set)" | ||
| quiz_title = self.question.quiz.assignment.title if self.question.quiz and self.question.quiz.assignment else "Unknown Quiz" | ||
| # (debugging) try-except can be removed once confident there's no circular logic | ||
| try: |
There was a problem hiding this comment.
do we still need this code for debugging? if not, you can remove it
There was a problem hiding this comment.
After I finish connecting with the frontend and running a few user tests, I’ll gladly remove it if it turns out to be unnecessary and not needed.
| # self.list_url = '/lessons/assignments/' | ||
| #self.detail_url = f'/lessons/assignments/{self.assignment.pk}/' | ||
| #self.create_url = '/lessons/assignments/create/' | ||
| # self.detail_url = f'/lessons/assignments/{self.assignment.pk}/' |
There was a problem hiding this comment.
You can remove this since you used reverse() above
There was a problem hiding this comment.
Thanks for pointing this out! I had originally left it as a heads-up for reviewers, but I’ll go ahead and remove it moving forward.
|
|
||
| def __str__(self): | ||
| return f"Choice: {self.choice_text} ({'Correct' if self.is_correct else 'Incorrect'})" | ||
| quiz_title = self.question.quiz.assignment.title if self.question.quiz and self.question.quiz.assignment else "Unknown Quiz" |
There was a problem hiding this comment.
nice edge case handling here
cj-ballesteros
left a comment
cj-ballesteros
left a comment
There was a problem hiding this comment.
Hey Ethan! — just finished reviewing your PR titled "# Implementing Unit Tests for Lessons Model ". Here’s my feedback:
💡 Suggestions for Improvement
- Assertion Clarity: Use clear and specific assertions to validate expected outcomes, which improves test readability and maintainability. More assertions beyond testing http code 200.
- None that I haven't already said in my other PR, but looks like many of it has been improved.
🧪 Tested This Feature
I followed the test instructions:
-
✅ Ran test cases using
python3 manage.py test apps.lessons -
✅ Created, deleted, and updated new instances having to do with the lessons model.
-
✅ Reviewed code for anything that could potentially go wrong/debug code still left.
🚀 Final Thoughts
Everything else looks great!
3 participants
Implementing Unit Testing for Lessons Model (Quiz, Question, Choice, Solution)
Overview
What does this PR do?
This PR adds unit tests for the Lessons app API endpoints, specifically targeting the Quiz, Question, Choice, and Solution models. It also introduces the missing views, serializers, and URLs for the Quiz and Solution components to ensure complete coverage. These tests validate the correctness of CRUD operations and enhance the stability of the Quiz, Question, Choice, and Solution models.
Key Features & Changes
What has been added or changed?
✔ Unit Test Implementation:
✔ Quiz Views, Serializers, and URLs:
✔ Solution Views, Serializers, and URLs:
✔ Test Framework Setup:
Why This Is Needed
What problem does this solve?
These additions improve our test coverage for the lessons-related API endpoints and help ensure consistent, predictable behavior as the lessons application evolves. It also closes out the remaining scope outlined in issue #150 and complements previous testing done for Assignments.
Related Issue
🔗 This PR addresses [Issue #150 - Continue implementing unit tests for the lessons app API endpoints, focusing on Quizzes, Questions, Choices, and Solutions.]
Implementation Details
How was this feature developed?
Design Documentation (If Applicable)
Diagrams & Visuals (If the PR involves a major feature, link UML diagrams)
How to Test This PR
Step-by-step testing guide
1️⃣Navigate to
backenddirectory.2️⃣Enable virtual environment. For Mac/Linux (bash/zsh) users, this is
source env/bin/activate. For Windows (PowerShell) users, this is.\env\Scripts\Activate.ps13️⃣(If needed) Install dependencies:
pip install -r requirements.txt4️⃣Apply migrations:
python manage.py migrate5️⃣Run tests:
python manage.py test apps.lessons6️⃣Confirm all 21 tests pass successfully.
Files Added/Modified
📂 List of important files changed in this PR
backend/apps/lessons/tests.pybackend/apps/lessons/urls.pybackend/apps/lessons/views.pybackend/apps/lessons/serializers.pyAI Code Usage
🤖 Was AI-generated code used in this PR?
Checklist for Reviewers
Things to check before approving this PR:
Next Steps & Future Enhancements
What improvements can be made after merging?
Final Notes
Thanks for reviewing! Please leave feedback if you have any suggestions or concerns.
-- Ethan