-
Notifications
You must be signed in to change notification settings - Fork 305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Development
: Fix exam assessment e2e tests failing
#9462
Conversation
WalkthroughThe changes in this pull request enhance the assessment process in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used📓 Learnings (2)
🔇 Additional comments (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range comments (1)
src/test/playwright/e2e/exam/ExamAssessment.spec.ts (1)
Line range hint
1-478
: Consider adding a specific test for the race condition.While the change effectively addresses the reported issue, it might be beneficial to add a specific test case that verifies the resolution of the race condition. This would ensure that future changes don't inadvertently reintroduce the problem.
Consider adding a test case that:
- Creates an exam with multiple exercises
- Registers a student
- Generates individual exams
- Verifies that the student can see all exercises associated with their exam
This would provide an extra layer of confidence in the fix and serve as documentation of the resolved issue.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- src/test/playwright/e2e/exam/ExamAssessment.spec.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
src/test/playwright/e2e/exam/ExamAssessment.spec.ts (1)
349-349
: LGTM! This change addresses the race condition issue.The addition of
await examAPIRequests.registerStudentForExam(exam, studentOne);
at this point in theprepareExam
function effectively resolves the race condition mentioned in the PR objectives. By registering the student after creating the exam and exercise, but before generating individual exams, we ensure that the student will have access to all relevant exercises when the exam is generated.This change aligns perfectly with the PR's goal of modifying the order of operations in the test setup to prevent scenarios where registered students were unable to see exercises associated with their exams.
Checklist
General
Motivation and Context
The exam assessment e2e tests broke due to the logic changes in the student exam creation in #9123, as there is now a race condition, when the student exams are generated automatically. The playwright tests registered users before creating exercises, which lead to the student exams showing up without any exercises to the user.
Description
Simply registering the students after creating the exercises, solves the issue.
Steps for Testing
Prerequisites:
Run the playwright e2e test and make sure no tests fail.
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Test Coverage
Screenshots
Before the students did not have any exercises in their exams:

After the fix:

Summary by CodeRabbit
New Features
Bug Fixes
dayjs
for accurate time management.