Programming exercises: Fix an issue for grading statistics
#9779
Conversation
github-actions
bot
added
server
…-testCaseMap-null-value' into bugfix/programming-exercises/fix-testCaseMap-null-value
WalkthroughThe Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 pmdsrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration. 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 (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.java (2)
1081-1099: LGTM! Consider enhancing the warning logs.The null checks effectively prevent NullPointerException. However, the warning logs could include more context about the feedback to aid debugging.
Consider enhancing the warning logs:
-log.warn("Feedback with ID {} has a test case with a null test name.", feedback.getId()); +log.warn("Feedback with ID {} has a test case with a null test name. Exercise: {}, Participation: {}", + feedback.getId(), feedback.getResult().getParticipation().getExercise().getId(), + feedback.getResult().getParticipation().getId()); -log.warn("Feedback with ID {} has a test case with a null isPositive value.", feedback.getId()); +log.warn("Feedback with ID {} has a test case with a null isPositive value. Exercise: {}, Participation: {}", + feedback.getId(), feedback.getResult().getParticipation().getExercise().getId(), + feedback.getResult().getParticipation().getId());
1081-1099: Consider extracting filter conditions for better readability.The filter conditions could be more readable if extracted into a separate method.
Consider this refactoring:
-result.getFeedbacks().stream() - .filter(feedback -> { - if (!FeedbackType.AUTOMATIC.equals(feedback.getType())) { - return false; - } - if (feedback.getTestCase() == null) { - return false; - } - if (feedback.getTestCase().getTestName() == null) { - log.warn("Feedback with ID {} has a test case with a null test name.", feedback.getId()); - return false; - } - if (feedback.isPositive() == null) { - log.warn("Feedback with ID {} has a test case with a null isPositive value.", feedback.getId()); - return false; - } - return true; - }) +result.getFeedbacks().stream() + .filter(this::isValidTestCaseFeedback) +/** + * Checks if the feedback is valid for test case statistics. + * A valid feedback must: + * - Be of type AUTOMATIC + * - Have a non-null test case with a non-null test name + * - Have a non-null isPositive value + * + * @param feedback The feedback to validate + * @return true if the feedback is valid, false otherwise + */ +private boolean isValidTestCaseFeedback(Feedback feedback) { + if (!FeedbackType.AUTOMATIC.equals(feedback.getType())) { + return false; + } + if (feedback.getTestCase() == null) { + return false; + } + if (feedback.getTestCase().getTestName() == null) { + log.warn("Feedback with ID {} has a test case with a null test name.", feedback.getId()); + return false; + } + if (feedback.isPositive() == null) { + log.warn("Feedback with ID {} has a test case with a null isPositive value.", feedback.getId()); + return false; + } + return true; +}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
krusche
changed the title
Programming exercises: Resolve null feedback issue in testcase map for grading statisticsProgramming exercises: Fix an issue for grading statistics
krusche
deleted the
bugfix/programming-exercises/fix-testCaseMap-null-value
branch
Checklist
General
Server
Motivation and Context
A NullPointerException appeared in the server logs while loading grading statistics.
Description
My assumption is that because feedback.isPositive can be null (in cases where the test case has not run for the submission yet, possibly due to hidden tests), the grading statistics cannot distinguish this scenario. In my opinion, this can be ignored in the statistics and should not be considered, as after the deadline, once all test cases have run, the feedback for students should have a definitive isPositive value.
Review Progress
Code Review
Summary by CodeRabbit
Bug Fixes
Improvements