Programming exercises: Add a warning for duplicated test cases in the problem statement

[coolchock]

Checklist

General

• I tested all changes and their related features with all corresponding user types on a test server.
• Language: I followed the guidelines for inclusive, diversity-sensitive, and appreciative language.
• I chose a title conforming to the naming conventions for pull requests.

Client

• Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data.
• I followed the coding and design guidelines.
• I translated all newly inserted strings into English and German.

Motivation and Context

There was a problem with duplicated test cases in the problem statement. #7736
The backend part was already implemented, but in the issue it was mentioned, that a warning in the problem statement should also be shown, that test cases are duplicated.

Steps for Testing

Prerequisites:

• 1 Instructor / 1 Tutor
• 1 Programming Exercise

1. Log in to Artemis
2. Navigate to Course Administration
3. Create a programming exercise / open an existing programming exercise.
4. Copy a test case and paste it somewhere else in the problem statement
5. Make sure that a warning appears in each line where a duplicated test occurs
6. Make sure that all duplicated test cases are listed in the "test case issues" tooltip

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked

Review Progress

Performance Review

• I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance
• I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance

Code Review

• Code Review 1
• Code Review 2

Manual Tests

• Test 1
• Test 2

Summary by CodeRabbit

• New Features

  • Added display for duplicated test cases in programming exercise instructions, including a count of occurrences.

• Bug Fixes

  • Enhanced analysis of problem statements to identify and report duplicated test cases.

• Documentation

  • Updated analysis model to include duplicatedTestCases field for better tracking and reporting.

• Tests

  • Added new tests to ensure duplicated test cases are correctly identified and handled in the instruction analysis.

[coderabbitai]

Important

Auto Review Skipped

Review was skipped due to path filters

Walkthrough

The recent update introduces functionality to detect and handle duplicated test cases in programming exercise instructions. A new condition checks for these duplicates, which are then displayed with their counts. The analysis component, service, and model are updated to support this feature, including a new array and enum type. The editing component now maps these findings to warnings, ensuring that duplicated test cases are flagged during the exercise creation process.

Changes

File Path	Change Summary
.../instructions-editor/analysis/...instruction-analysis.component.html .../instructions-editor/analysis/...instruction-analysis.component.ts .../instructions-editor/analysis/...instruction-analysis.model.ts .../instructions-editor/analysis/...instruction-analysis.service.ts	Modified the logic and methods to handle and display repeated test cases and their count, added new array and enum type, and adjusted translation lookup.
.../instructions-editor/...editable-instruction.component.ts	Updated to handle repeatedTestCases in methods.
.../spec/component/programming-exercise/...instruction-analysis.component.spec.ts .../spec/service/...instruction-analysis.service.spec.ts	Updated to handle and test the repeatedTestCases array.

Poem

🐇 "In the code garden, where bugs may roam,
We've found duplicates, not far from home.
With a hop and a fix, no more they'll clone,
For a burrow well-coded is a rabbit's throne." 🌱

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?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository from git and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 0025383 and 2e557b2.

Files ignored due to filter (3)

• src/main/webapp/i18n/de/programmingExercise.json
• src/main/webapp/i18n/en/programmingExercise.json
• src/test/javascript/spec/helpers/sample/problemStatement.json

Files selected for processing (7)

• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.component.html (2 hunks)
• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.component.ts (3 hunks)
• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.model.ts (1 hunks)
• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.service.ts (6 hunks)
• src/main/webapp/app/exercises/programming/manage/instructions-editor/programming-exercise-editable-instruction.component.ts (1 hunks)
• src/test/javascript/spec/component/programming-exercise/programming-exercise-instruction-analysis.component.spec.ts (6 hunks)
• src/test/javascript/spec/service/programming-exercise-instruction-analysis.service.spec.ts (3 hunks)

Files skipped from review due to trivial changes (1)

• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.model.ts

Additional comments: 22

src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.component.html (2)

• 2-2: The condition in the @if block now checks for the length of duplicatedTestCases in addition to invalidTestCases and missingTestCases. This is a logical extension to include the new feature of detecting duplicated test cases.

• 38-47: A new section has been added to display duplicated test cases. It is important to ensure that the translation key artemisApp.programmingExercise.testCaseAnalysis.duplicatedTestCases is properly set up in the translation files to display the correct message to the user.

src/test/javascript/spec/service/programming-exercise-instruction-analysis.service.spec.ts (3)

• 2-2: The import statement for problemStatementDuplicatedTestCases has been added. It's good practice to ensure that only necessary imports are included to avoid bloating the test files.

• 18-22: The analyzeProblemStatement method has been correctly updated to include duplicatedTestCases in its return object. This aligns with the new feature implementation.

• 43-57: A new test case has been added to ensure that the analysis service correctly identifies duplicated test cases. This is a necessary addition to validate the new functionality.

src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.component.ts (3)

• 23-23: The duplicatedTestCases array has been added to the component's properties to store any duplicated test cases found during analysis. This is a necessary change to support the new feature.

• 37-37: The analyzeProblemStatement method now correctly updates the duplicatedTestCases property of the component, ensuring that the new feature's state is managed appropriately.

• 66-69: The comment above the analyzeTasks method has been updated to include the detection of duplicated test cases. This is a good practice to keep the documentation in sync with the code changes.

src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.service.ts (6)

• 13-13: The new constant DUPLICATED_TEST_CASE_TRANSLATION has been added for the translation of duplicated test case issues. It's important to ensure that this constant is used consistently throughout the service.

• 34-37: The analyzeProblemStatement method has been updated to include duplicatedTestCases in the return object, which is necessary for the new feature to function correctly.

• 60-60: The analyzeTestCases method now includes logic to detect duplicated test cases, which is a crucial part of the new feature. It's important to ensure that the logic correctly identifies duplicates without false positives.

• 72-87: The logic to accumulate and analyze duplicated test cases has been added. This includes creating a map of test case occurrences and filtering out those with multiple line numbers to identify duplicates. It's important to ensure that this logic is robust and handles edge cases correctly.

• 123-124: The getTranslationByIssueType method has been updated to handle the new DUPLICATED_TEST_CASES issue type. This ensures that the correct translation is used when reporting duplicated test cases.

• 137-137: The extractRegexFromTasks method has been adjusted to handle the changes in the analyzeTestCases method. This ensures that the regex extraction logic is in line with the new feature requirements.

src/test/javascript/spec/component/programming-exercise/programming-exercise-instruction-analysis.component.spec.ts (6)

• 77-77: The duplicatedTestCases array has been added to the test setup. This is necessary to test the component's handling of duplicated test cases.

• 79-84: The analyzeProblemStatementStub mock has been updated to return duplicatedTestCases. This ensures that the component's behavior with duplicated test cases can be tested.

• 98-98: The test now includes an assertion to check the duplicatedTestCases property, ensuring that the component correctly updates its state based on the analysis service's output.

• 132-132: The duplicatedTestCases array has been added to the test setup for a new test case. This test case should verify the component's rendering behavior when duplicated test cases are present.

• 146-162: The test case has been updated to include assertions for duplicatedTestCases. This ensures that the component's rendering behavior is correct when duplicated test cases are detected.

• 179-179: The test includes an assertion to check the duplicatedTestCases property after rendering, ensuring that the component's internal state reflects the analysis service's output.

src/main/webapp/app/exercises/programming/manage/instructions-editor/programming-exercise-editable-instruction.component.ts (2)

• 228-231: The mapAnalysisToWarnings method has been updated to handle a new parameter duplicatedTestCases. Ensure that the method correctly processes the new parameter and that the annotations are set appropriately.

• 234-245: The mapIssuesToAnnotations method now includes logic for handling duplicatedTestCases. Verify that the annotations for duplicated test cases are created correctly and that the warning message is appropriate and informative.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 2e557b2 and 86ffb59.

Files ignored due to filter (3)

• src/main/webapp/i18n/de/programmingExercise.json
• src/main/webapp/i18n/en/programmingExercise.json
• src/test/javascript/spec/helpers/sample/problemStatement.json

Files selected for processing (7)

• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.component.html (2 hunks)
• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.component.ts (3 hunks)
• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.model.ts (1 hunks)
• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.service.ts (6 hunks)
• src/main/webapp/app/exercises/programming/manage/instructions-editor/programming-exercise-editable-instruction.component.ts (1 hunks)
• src/test/javascript/spec/component/programming-exercise/programming-exercise-instruction-analysis.component.spec.ts (6 hunks)
• src/test/javascript/spec/service/programming-exercise-instruction-analysis.service.spec.ts (3 hunks)

Additional comments: 23

src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.model.ts (2)

• 6-6: The addition of the repeatedTestCases property to the ProblemStatementAnalysis type aligns with the PR's objective to handle duplicated test cases. This change is consistent and correctly implemented.

• 12-12: The new enum value REPEATED_TEST_CASES added to ProblemStatementIssue is consistent with the changes and the PR's objectives. It is correctly implemented.

src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.component.html (2)

• 2-2: The new condition added to check for the presence of repeatedTestCases is consistent with the feature's requirements. It ensures that the warning for duplicated test cases will be displayed when necessary.

• 38-47: The HTML segment added to display the repeated test cases and their count is correctly implemented. It uses Angular's structural directives and pipes as expected in the Angular framework, and it adheres to the PR's objectives.

src/test/javascript/spec/service/programming-exercise-instruction-analysis.service.spec.ts (4)

• 2-2: The import statement for problemStatementRepeatedTestCases is added to support the new test cases for duplicated test cases. This is a necessary change for the test setup.

• 18-22: The test case 'should analyse problem statement without any issues correctly' now includes a check for repeatedTestCases. This is a necessary addition to ensure that the new functionality is covered by tests.

• 35-41: The test case 'should analyse problem statement with issues correctly' has been updated to include repeatedTestCases. This ensures that the service's behavior is correctly tested when there are no duplicated test cases.

• 43-57: The new test case 'should analyse problem statement with duplicated test cases' is correctly implemented to test the analysis of problem statements with duplicated test cases. It verifies that the service correctly identifies and returns the duplicated test cases.

src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.component.ts (3)

• 23-23: The addition of the repeatedTestCases array to store duplicated test cases is consistent with the feature's requirements and is correctly implemented.

• 37-44: The modifications to the ngOnInit method to handle repeatedTestCases are correct. The method now properly updates the component's state with the results from the analysis service, including the new duplicated test cases feature.

• 66-69: The comment block has been updated to include 'having duplicated test cases' as a possible error. This update is necessary for code clarity and maintainability.

src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.service.ts (5)

• 13-13: The addition of the REPEATED_TEST_CASE_TRANSLATION constant is necessary for the translation of the new repeated test cases issue type. This change is correctly implemented.

• 34-37: The analyzeProblemStatement method has been updated to return repeatedTestCases in addition to the existing return values. This change is necessary to support the new feature and is correctly implemented.

• 69-90: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [44-87]

The analyzeTestCases method has been significantly modified to include the detection of duplicated test cases. The logic for identifying and handling repeated test cases is correctly implemented and aligns with the PR's objectives.

• 122-123: The getTranslationForIssueType method has been updated to handle the new issue type REPEATED_TEST_CASES. This change is necessary for the translation functionality to work with the new issue type.

• 136-136: The removal of uniq from the cleanMatches function is necessary to allow duplicates to be detected later in the analysis process. This change is correctly implemented.

src/test/javascript/spec/component/programming-exercise/programming-exercise-instruction-analysis.component.spec.ts (5)

• 77-84: The addition of the repeatedTestCases array and its handling in the component's logic and tests is correctly implemented. This ensures that the new feature is properly tested.

• 98-98: The test case now correctly checks for the presence of repeatedTestCases in the component's state, ensuring that the new feature works as expected.

• 112-112: The test case correctly checks for the presence of repeatedTestCases after changes to the problem statement, ensuring that the component updates its state correctly.

• 132-132: The addition of the repeatedTestCases array within the Analysis service integration test block is necessary for testing the integration with the analysis service.

• 140-165: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [146-179]

The test case 'should render warnings on missing, invalid and repeated test cases' has been correctly updated to include the handling of repeatedTestCases. This ensures that the component's rendering logic is tested for the new feature.

src/main/webapp/app/exercises/programming/manage/instructions-editor/programming-exercise-editable-instruction.component.ts (2)

• 229-231: The mapAnalysisToWarnings method has been updated to handle the repeatedTestCases parameter. This change is necessary to map the new duplicated test cases analysis to warnings in the editor.

• 234-245: The mapIssuesToAnnotations method has been correctly modified to accommodate the repeatedTestCases parameter and add annotations for it. This ensures that the editor will display warnings for duplicated test cases.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 86ffb59 and 969dd62.

Files selected for processing (2)

• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.component.ts (3 hunks)
• src/test/javascript/spec/service/programming-exercise-instruction-analysis.service.spec.ts (3 hunks)

Files skipped from review as they are similar to previous changes (2)

• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.component.ts
• src/test/javascript/spec/service/programming-exercise-instruction-analysis.service.spec.ts

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 969dd62 and 3a27cba.

Files ignored due to filter (2)

• src/main/webapp/i18n/de/programmingExercise.json
• src/main/webapp/i18n/en/programmingExercise.json

Files selected for processing (1)

• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.service.ts (6 hunks)

Files skipped from review as they are similar to previous changes (1)

• src/main/webapp/app/exercises/programming/manage/instructions-editor/analysis/programming-exercise-instruction-analysis.service.ts

[konrad2002]

manually tested on legacy ts11, works as expected

[Strohgelaender]

code

[Gusti2010]

Manually tested on ts11_Legacy. Works as expected!

[xHadie]

Manually tested on TS2_Legacy
Works as described 👍🏼