Development: Add e2e tests for instructors assessing exams in the second round
#9863
Conversation
github-actions
bot
added
tests
client
muradium
changed the title
Development: Add test for instructor assessing exam on second round
muradium
changed the title
Development: Add test for instructor assessing exam on second roundDevelopment: Add test for instructor assessing exam on second round
muradium
changed the title
Development: Add test for instructor assessing exam on second roundDevelopment: E2E test for instructor assessing exam on second round
WalkthroughThe changes involve modifications to the conditional rendering logic in the assessment dashboard components, enhancing control over UI elements based on Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (9)
src/test/playwright/support/pageobjects/assessment/AbstractExerciseAssessmentPage.ts (2)
Line range hint
16-27: Good refactoring, consider making selectors more maintainableThe extraction of feedback filling logic into a separate method improves code reusability and maintainability. However, the implementation could be enhanced further.
Consider:
- Moving the CSS selectors to constants or a separate configuration file:
private static readonly SELECTORS = { UNREFERENCED_FEEDBACK: '.unreferenced-feedback-detail', POINTS_INPUT: '#feedback-points', FEEDBACK_TEXTAREA: '#feedback-textarea' } as const;
- Adding basic validation for the points parameter:
async fillFeedback(points: number, feedback?: string) { + if (points < 0 || !Number.isFinite(points)) { + throw new Error('Points must be a non-negative finite number'); + } const unreferencedFeedback = this.page...
68-72: Approve complaint handling logic with architectural suggestionsThe differentiated handling of TEXT exercise complaints is well-implemented, properly distinguishing between acceptance and rejection paths.
Consider these architectural improvements:
- Move API endpoints to a dedicated configuration:
private static readonly ENDPOINTS = { TEXT: { ACCEPT: `${BASE_API}/participations/*/submissions/*/text-assessment-after-complaint`, REJECT: `${BASE_API}/complaints/*/response` }, PROGRAMMING: { ASSESSMENT: `${BASE_API}/programming-submissions/*/assessment-after-complaint` }, // ... other endpoints } as const;
- Consider using a strategy pattern to handle different exercise types instead of the growing switch statement. This would make it easier to add new exercise types without modifying existing code.
src/main/webapp/app/course/dashboards/assessment-dashboard/assessment-dashboard.component.html (1)
Line range hint
225-229: Consider adding data-testid attributes to related elements.While the
data-testid="toggle-second-correction"is correctly added for the second correction button, consider adding similar attributes to related elements that are part of the E2E test flow, such as:
- The exam title header
- The second correction hint message
- The assessment dashboard information component
This would make the E2E tests more robust and easier to maintain.
src/test/playwright/e2e/exam/ExamAssessment.spec.ts (3)
144-151: Consider adding more assertions to the second round assessment testWhile the test covers the basic flow, consider adding assertions for:
- Verification that the first round score is preserved
- Validation of the assessment feedback visibility to the student
- Checking the assessment round number in the response
380-390: Add JSDoc documentation for the new parametersConsider adding JSDoc documentation to explain the purpose and behavior of the new parameters:
async function startAssessing( courseID: number, examID: number, timeout: number, examManagement: ExamManagementPage, courseAssessment: CourseAssessmentDashboardPage, exerciseAssessment: ExerciseAssessmentDashboardPage, + /** Whether to enable second correction round */ toggleSecondRound: boolean = false, + /** Whether this is the first time the user is assessing */ isFirstTimeAssessing: boolean = true, )
Line range hint
429-433: Add a comment explaining the complaint handling logicThe condition combining exercise type and rejection status would benefit from a comment explaining why modeling exercises or rejected complaints require different message handling.
+ // Modeling exercises always show submission message + // Rejected complaints show submission message + // Accepted complaints for non-modeling exercises show update message if (exerciseType == ExerciseType.MODELING || reject) { await examAssessment.checkComplaintMessage('Response to complaint has been submitted'); } else { await examAssessment.checkComplaintMessage('The assessment was updated successfully.'); }src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.html (3)
38-38: Consider simplifying nested conditionsThe multiple nested conditions for the second correction button could be simplified using Angular's new control flow syntax.
Consider refactoring to:
-@if (exercise.isAtLeastInstructor && exam?.numberOfCorrectionRoundsInExam && exam?.numberOfCorrectionRoundsInExam! > 1 && isExamMode && !isTestRun) { +@if (canEnableSecondCorrection()) {And move the complex condition to a component method:
canEnableSecondCorrection(): boolean { return this.exercise.isAtLeastInstructor && this.exam?.numberOfCorrectionRoundsInExam > 1 && this.isExamMode && !this.isTestRun; }
Line range hint
261-466: Consider extracting assessment table into a separate componentThe assessment table section is quite large and handles multiple responsibilities. Consider extracting it into a dedicated component to improve maintainability and reusability.
Consider creating a new component like
assessment-submission-table.componentto handle the table-specific logic and template.
Line range hint
467-725: Reduce duplication in complaints and feedback tablesThe complaints and more feedback request sections share very similar table structures and logic. Consider unifying them into a reusable table component.
Create a generic table component that can handle both complaints and feedback requests:
interface TableConfig { type: 'complaint' | 'feedback'; data: any[]; sortConfig: any; onSort: (event: any) => void; } @Component({ selector: 'jhi-assessment-request-table', template: '...' // Extract common table structure }) export class AssessmentRequestTableComponent { @Input() config!: TableConfig; }This would significantly reduce code duplication and improve maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
src/main/webapp/app/course/dashboards/assessment-dashboard/assessment-dashboard.component.html(1 hunks)src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.html(1 hunks)src/test/playwright/e2e/exam/ExamAssessment.spec.ts(7 hunks)src/test/playwright/support/pageobjects/assessment/AbstractExerciseAssessmentPage.ts(2 hunks)src/test/playwright/support/pageobjects/assessment/ExerciseAssessmentDashboardPage.ts(2 hunks)
🧰 Additional context used
🔇 Additional comments (10)
src/test/playwright/support/pageobjects/assessment/ExerciseAssessmentDashboardPage.ts (2)
16-17: Well-implemented assessment round selection logic!
The updated method signature with an optional parameter and the use of nth() selector is a robust way to handle multiple assessment rounds while maintaining backward compatibility.
42-44: Verify the data-testid attribute exists in the component.
The implementation follows best practices by using a data-testid selector. However, let's verify that this test ID is properly set up in the component.
✅ Verification successful
The data-testid "toggle-second-correction" is properly implemented in the components
The test ID is correctly defined in both the exercise assessment dashboard and course assessment dashboard components, confirming proper implementation for the E2E test selector.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the toggle-second-correction test ID exists in the component template
# Search for the data-testid in component templates
rg -l 'data-testid="toggle-second-correction"'
Length of output: 242
src/test/playwright/support/pageobjects/assessment/AbstractExerciseAssessmentPage.ts (1)
Line range hint 1-89: Verify test coverage for complaint handling scenarios
The implementation supports the E2E test requirements for second round assessment, but let's ensure proper test coverage exists.
✅ Verification successful
Test coverage for complaint handling is comprehensive across exercise types
The verification shows that complaint handling is well-tested:
- Text exercises have acceptance test coverage in
TextExerciseAssessment.spec.ts - File upload exercises have acceptance test coverage in
FileUploadExerciseAssessment.spec.ts - Modeling exercises have rejection test coverage in
ModelingExerciseAssessment.spec.ts - Programming exercises have commented test code in
ProgrammingExerciseAssessment.spec.ts - Exam-specific complaint handling is covered in
ExamAssessment.spec.ts
All exercise-specific page objects properly extend and implement the complaint handling methods from the abstract class.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for comprehensive test coverage of complaint handling scenarios
# Look for test files that use this page object
echo "Searching for test files using AbstractExerciseAssessmentPage..."
rg -l "AbstractExerciseAssessmentPage" --type ts
# Check for complaint handling test scenarios
echo "Checking complaint handling test coverage..."
rg -A 5 "acceptComplaint|rejectComplaint" --type ts
Length of output: 20523
src/main/webapp/app/course/dashboards/assessment-dashboard/assessment-dashboard.component.html (1)
Line range hint 5-5: LGTM! Conditional rendering follows the new Angular syntax.
The template correctly uses the new @if and @for directives instead of the older *ngIf and *ngFor syntax, adhering to the coding guidelines.
Also applies to: 8-8, 17-17, 33-33, 38-38, 68-68, 92-92, 126-126, 225-229, 266-266, 269-269, 272-272, 275-275
src/test/playwright/e2e/exam/ExamAssessment.spec.ts (3)
79-79: LGTM: Simplified complaint handling logic
The removal of conditional complaint handling based on assessment success flags makes the code more straightforward and maintainable.
Also applies to: 117-117, 141-141
123-125: LGTM: Added support for multiple correction rounds
The test setup now properly configures the exam with multiple correction rounds, which is essential for testing the second round assessment functionality.
297-297: LGTM: Well-structured exam preparation with correction rounds support
The changes to prepareExam are well-implemented with:
- Backward compatibility through default parameter value
- Clear configuration of correction rounds in exam setup
Also applies to: 327-327
src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.html (3)
41-41: LGTM: Test ID addition enhances E2E testability
The addition of data-testid="toggle-second-correction" to the second correction button aligns with the PR objectives and follows good testing practices.
Line range hint 1-1: LGTM: Proper usage of new Angular control flow syntax
The template correctly uses the new @if and @for syntax as required by the coding guidelines, replacing the older *ngIf and *ngFor directives.
Also applies to: 4-11
Line range hint 38-46: Verify E2E test coverage for second correction workflow
The template supports the E2E test scenario described in the PR objectives, but let's verify that all necessary test hooks are in place.
✅ Verification successful
Based on the analysis of the codebase through the executed scripts, I can now provide a final response:
E2E test hooks are properly configured for second correction workflow
The added data-testid="toggle-second-correction" is correctly placed and consistent with other test IDs in the codebase. The component is properly integrated into both the exercise assessment dashboard and course assessment dashboard templates, ensuring testability across different contexts.
- The toggle button's test ID is accessible in both standalone exercise view and course-wide assessment view
- The component's structure follows the established testing patterns in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for all required test IDs in the second correction workflow
# Search for other related test IDs that might be needed for the complete workflow
rg "data-testid.*second.*correction" --type html
rg "data-testid.*assessment" --type html
Length of output: 445
Script:
#!/bin/bash
# Let's check for the component implementation and test files related to second correction
rg "secondCorrection" -g "*.{ts,spec.ts,html}" -A 3
# Also check for any existing E2E tests
fd -e cy.ts | xargs rg "second.*correction" -A 3
Length of output: 125328
Script:
#!/bin/bash
# Let's check for E2E test files specifically related to second correction
fd -e cy.ts | xargs cat | grep -A 5 "second.*correction"
# Also check for test IDs in test files
fd -e spec.ts | xargs rg "data-testid.*second.*correction"
Length of output: 119
muradium
dismissed stale reviews from az108 and coderabbitai[bot]
via
f8a3fbf
krusche
changed the title
Development: E2E test for instructor assessing exam on second roundDevelopment: Add e2e tests for instructors assessing exams in the second round
Checklist
General
Client
Motivation and Context
We want to test the scenario where instructor enables a second round of exam assessment and assesses the exam submission one more time.
Description
Adds E2E test in Playwright about instructor enabling second round of assessment and making changes to the assessment tutor has done on the first round. To make the test lightweight, it's added as part of the series of text assessment tests, where the first round of assessment is followed by a student complaining, the complaint being rejected and finally, the final assessment done during the second round.
Steps for Testing
Steps for running the tests:
npm install && npm run playwright:setupnpx playwright test e2e/exam/ExamAssessment.spec.ts -g "Text exercise assessment"npm run playwright:opento open the Playwright UI, search for the "Text exercise assessment" and run itWarning: Don't solely run the "Instructor makes a second round of assessment" test as it's part of the serial group of tests and will fail if previous tests are not executed.
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Review Progress
Code Review
Manual Tests
Summary by CodeRabbit