Development: Add tests for IrisCommonSubSettingsUpdateComponent
#11749
Conversation
github-actions
bot
added
client
WalkthroughThis change expands test coverage for the IrisCommonSubSettingsUpdateComponent by introducing new test cases for error handling, variant selection logic, event toggling, category management, and parent-child component interactions using asynchronous testing utilities. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (2)
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts (2)
222-234: Consider simplifying async handling in category error testThe test correctly verifies that a failing
findAllCategoriesOfCoursecall triggersonErrorand that the service is invoked once. Given thatthrowError(() => error)is synchronous andloadCategories()likely just subscribes immediately, thefakeAsync/tick()wrapper may not be necessary and could be replaced with a plain synchronous test body (orwaitForAsyncif the implementation later becomes truly async). This would reduce complexity and avoid potential fakeAsync flakiness noted elsewhere in the repo. Based on learnings, …
287-310: ngOnChanges event disabled status test is effective;SimpleChangesetup could be more idiomaticThis test effectively verifies that:
ngOnChangescallsupdateEventDisabledStatuswhenparentSubSettingschanges, and- the
eventInParentDisabledStatusMapreflects the parent’senabledflag transitions.One optional improvement: in the second
SimpleChange, you pass the sameparentSubSettingsobject as bothpreviousValueandcurrentValueafter mutating it. For closer fidelity to Angular’s usual change records, you could pass distinct snapshot objects (or at least separate values for enabled=false vs true). This wouldn’t change behavior here but would make the test more representative of real Input changes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: SimonEntholzer
Repo: ls1intum/Artemis PR: 8929
File: src/test/java/de/tum/in/www1/artemis/authentication/UserLocalVcIntegrationTest.java:29-33
Timestamp: 2024-07-16T19:27:15.402Z
Learning: When addressing missing test coverage, ensure that new tests specifically validate the functionality introduced in the PR, such as token-based authentication.
Learnt from: alexjoham
Repo: ls1intum/Artemis PR: 9455
File: src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java:401-401
Timestamp: 2024-10-15T11:33:17.915Z
Learning: In the Artemis project, when new fields are added to classes like `PyrisChatStatusUpdateDTO`, corresponding tests may be implemented in separate integration test classes such as `IrisChatTokenTrackingIntegrationTest`.
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9443
File: src/test/javascript/spec/component/hestia/git-diff-report/git-diff-modal.component.spec.ts:55-60
Timestamp: 2024-10-10T11:42:23.069Z
Learning: In `git-diff-report-modal.component.spec.ts`, using `fakeAsync` and `tick` does not work for handling asynchronous operations in the tests; alternative methods are needed.
📚 Learning: 2024-10-10T11:42:23.069Z
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9443
File: src/test/javascript/spec/component/hestia/git-diff-report/git-diff-modal.component.spec.ts:55-60
Timestamp: 2024-10-10T11:42:23.069Z
Learning: In `git-diff-report-modal.component.spec.ts`, using `fakeAsync` and `tick` does not work for handling asynchronous operations in the tests; alternative methods are needed.
Applied to files:
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts
📚 Learning: 2025-09-01T10:20:40.706Z
Learnt from: Michael-Breu-UIbk
Repo: ls1intum/Artemis PR: 10989
File: src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts:132-149
Timestamp: 2025-09-01T10:20:40.706Z
Learning: In the Artemis codebase, Angular component test files for ProgrammingExerciseDetailComponent follow a pattern where the component is imported but not explicitly declared in TestBed.configureTestingModule(), yet TestBed.createComponent() still works successfully. This pattern is consistently used across test files like programming-exercise-detail.component.spec.ts and programming-exercise-detail.component.with-sharing.spec.ts.
Applied to files:
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts
📚 Learning: 2024-10-29T12:28:57.305Z
Learnt from: SimonEntholzer
Repo: ls1intum/Artemis PR: 9478
File: src/main/webapp/app/shared/user-settings/ssh-settings/details/ssh-user-settings-key-details.component.ts:104-111
Timestamp: 2024-10-29T12:28:57.305Z
Learning: In `ssh-user-settings-key-details.component.ts`, changing the error handling code by adding optional chaining (`?.`) and replacing `.indexOf()` with `.includes()` may alter semantics and should be avoided.
Applied to files:
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts
📚 Learning: 2025-08-21T17:30:20.530Z
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/drag-and-drop-question/drag-and-drop-question.component.spec.ts:34-34
Timestamp: 2025-08-21T17:30:20.530Z
Learning: FitTextDirective in src/main/webapp/app/quiz/shared/fit-text/fit-text.directive.ts is a standalone directive marked with standalone: true, so it should be imported in TestBed imports array, not declarations array.
Applied to files:
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts
📚 Learning: 2024-10-20T22:00:52.335Z
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9505
File: src/test/javascript/spec/integration/code-editor/code-editor-container.integration.spec.ts:179-181
Timestamp: 2024-10-20T22:00:52.335Z
Learning: In `src/test/javascript/spec/integration/code-editor/code-editor-container.integration.spec.ts`, `ResizeObserver` is mocked within the `beforeEach` block.
Applied to files:
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts
📚 Learning: 2025-04-01T17:19:55.677Z
Learnt from: tobias-lippert
Repo: ls1intum/Artemis PR: 10610
File: src/test/javascript/spec/service/guided-tour.service.spec.ts:193-193
Timestamp: 2025-04-01T17:19:55.677Z
Learning: In the guide tour service tests, the `router.navigateByUrl` mock should remain synchronous (returning boolean) rather than returning a Promise to maintain compatibility with existing test logic that depends on synchronous behavior.
Applied to files:
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts
📚 Learning: 2025-08-08T22:22:20.043Z
Learnt from: SamuelRoettgermann
Repo: ls1intum/Artemis PR: 11111
File: src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.ts:70-79
Timestamp: 2025-08-08T22:22:20.043Z
Learning: In the Artemis Angular components (such as `exam-rooms.component.ts`), error handling for data loading failures can be implemented by setting the data signal to `undefined`. The template then automatically displays appropriate error messages through conditional rendering with if/else blocks and translation keys, without requiring explicit error status management in the component logic.
Applied to files:
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts
📚 Learning: 2025-08-30T20:04:51.435Z
Learnt from: SamuelRoettgermann
Repo: ls1intum/Artemis PR: 11111
File: src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.ts:127-136
Timestamp: 2025-08-30T20:04:51.435Z
Learning: In the Artemis Angular components (such as `exam-rooms.component.ts`), when using translation methods like `showErrorNotification`, the base translation path is automatically prepended to the provided key. For example, if the base path is `artemisApp.examRooms.adminOverview` and you provide `examRoomOverview.loadError`, the full translation key resolves to `artemisApp.examRooms.adminOverview.examRoomOverview.loadError`.
Applied to files:
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts
🧬 Code graph analysis (1)
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts (1)
src/test/javascript/spec/helpers/mocks/iris/mock-settings.ts (1)
mockVariants(65-78)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-tests
- GitHub Check: client-style
- GitHub Check: server-tests
- GitHub Check: server-style
- GitHub Check: Analyse
🔇 Additional comments (6)
src/main/webapp/app/iris/manage/settings/iris-settings-update/iris-common-sub-settings-update/iris-common-sub-settings-update.component.spec.ts (6)
1-1: Imports align with new test coverageThe added imports (
fakeAsync,tick,IrisEventType,throwError,globalUtils,HttpErrorResponse) are all used by the new tests and keep the spec self‑contained. No issues here.Also applies to: 3-3, 12-12, 15-16
214-220: Guard test for missingsubSettings.typelooks solidThis test nicely exercises the guard that prevents variant loading when the feature type is missing, and correctly asserts that
getVariantsForFeatureis never called. Setup is minimal and focused; no changes needed.
236-245: Variant name helper test covers both happy path and fallbackThis test cleanly validates
getSelectedVariantName()for the normal case (matching variant) and the fallback behavior when the selected variant ID is unknown. The expectations match the intended semantics; no adjustments needed.
247-256: Parent variant name helper test mirrors child behavior correctlyThe parent‑side test for
getSelectedVariantNameParent()mirrors the child variant test and ensures both the resolved name and the non‑existent fallback are handled. This gives good confidence in the helper’s behavior.
258-272: Proactive events toggle behavior is well‑coveredThe test thoroughly checks that:
- toggling an event adds it to
disabledProactiveEvents,- toggling again removes it, and
- calling the handler with
subSettingsundefined is a no‑op.This provides solid regression coverage around the event toggling logic and its robustness to missing settings.
274-285: Custom instructions update test is clear and robustThe test correctly asserts that
onCustomInstructionsChangeupdatescustomInstructionswhensubSettingsexists and safely does nothing whensubSettingsis undefined. This matches the desired defensive behavior.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||||||||
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
github-project-automation
bot
moved this from Work In Progress
to Merged
in Artemis Development
krusche
changed the title
Iris: Add tests for IrisCommonSubSettingsUpdateComponentDevelopment: Add tests for IrisCommonSubSettingsUpdateComponent
Checklist
Motivation and Context
This commit adds a comprehensive set of unit tests for the
IrisCommonSubSettingsUpdateComponentto improve statement and branch coverage.Description
The new tests cover the following scenarios:
getSelectedVariantName,getSelectedVariantNameParent).ngOnChangeslifecycle hook behavior, ensuring UI state updates correctly when parent settings change.subSettingsobject is undefined.Steps for Testing
Verify that all tests pass, including the newly added tests
Code Review
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.