Programming exercises: Fix test case parsing of nested test suite elements
#9490
Conversation
github-actions
bot
added
tests
server
WalkthroughThe pull request introduces significant modifications to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.java (2 hunks)
- src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.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_importssrc/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java (1)
Pattern
src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
📓 Learnings (3)
📓 Common learnings
Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:42.972Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:48.768Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.java (2)
Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:42.972Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:48.768Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java (2)
Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:42.972Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:48.768Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.
🔇 Additional comments (2)
src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.java (2)
36-37: LGTMThe added comment improves code clarity by explaining that a top-level
<testsuites>element is parsed like a<testsuite>. The recursive processing enhances the parser's ability to handle different XML structures.
60-61: LGTMThe addition of the
testSuitesfield to theTestSuiterecord allows effective parsing of nested test suites. The use of annotations ensures proper deserialization of the XML elements.
src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.java
Show resolved
Hide resolved
magaupp
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
WalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.java (2 hunks)
- src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.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_importssrc/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java (1)
Pattern
src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
📓 Learnings (3)
📓 Common learnings
Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:42.972Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:48.768Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.java (2)
Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:42.972Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:48.768Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java (2)
Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:42.972Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:48.768Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.
🔇 Additional comments (4)
src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.java (4)
36-37: LGTM: Simplified processing of test suitesThe changes to directly process the
testSuiteobject streamline the method and enhance readability.
54-56: LGTM: Added processing of nested test suitesThe addition of the loop to process nested test suites ensures that all nested test cases are properly handled.
60-61: LGTM: UpdatedTestSuiterecord to include nested test suitesThe inclusion of the
testSuitesfield in theTestSuiterecord, along with the appropriate Jackson annotations, allows for correct parsing of nested<testsuite>elements.
65-65: LGTM: InitializedtestSuitesto an empty list if nullBy using
Objects.requireNonNullElse,testSuitesis set to an empty list when null, preventing potentialNullPointerExceptionduring iteration.
src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java (1)
Pattern
src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
📓 Learnings (2)
📓 Common learnings
Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:42.972Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:48.768Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java (2)
Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:42.972Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.Learnt from: Strohgelaender PR: ls1intum/Artemis#8677 File: src/main/java/de/tum/in/www1/artemis/service/connectors/localci/buildagent/TestResultXmlParser.java:0-0 Timestamp: 2024-10-08T15:35:48.768Z Learning: The `TestSuite` constructor in `TestResultXmlParser.java` was improved by using `Objects.requireNonNullElse` to handle null values concisely.
src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java
Show resolved
Hide resolved
sachmii
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
HawKhiem
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
krusche
changed the title
Programming exercises: Fix JUnit XML parsing of nested testsuite elementsProgramming exercises: Fix test case parsing of nested test suite elements
Checklist
General
Server
Changes affecting Programming Exercises
Motivation and Context
The JUnit XML format allows
<testcase>elements to contain other<testcase>elements. The current parser is not able to handle this.The Haskell template, for example, produces such files and fails on LocalCI.
Description
The handling of
<testcases>and<testcase>elements can be generalized, such that both elements are handled the same.This reduces special cases and simplifies the code as well.
Fixes #9159.
Steps for Testing
Prerequisites:
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
Summary by CodeRabbit
New Features
Bug Fixes
Tests