Development: Remove angular mock test modules
#10274
Conversation
WalkthroughThis pull request removes multiple mock module imports from various Angular test configurations. In several test files, imports of modules used for tooltips, dropdowns, collapsible components, and pagination have been eliminated or replaced. Additionally, some tests have seen adjustments in import ordering and provider configuration, and a new import has been added in one case. Several standalone mock module files have been deleted entirely from the helpers directory. Changes
Suggested labels
📜 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: 1
🧹 Nitpick comments (1)
src/test/javascript/spec/component/competencies/competency-accordion.component.spec.ts (1)
13-60: Consider adding more test coverage.The current test suite only covers progress calculations. Consider adding tests for:
- Component initialization
- Error handling scenarios
- Edge cases (e.g., empty metrics, undefined competency)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
src/test/javascript/spec/component/admin/system-notification-management/system-notification-management.component.spec.ts(1 hunks)src/test/javascript/spec/component/code-editor/code-editor-header.component.spec.ts(2 hunks)src/test/javascript/spec/component/competencies/competency-accordion.component.spec.ts(1 hunks)src/test/javascript/spec/component/competencies/generate-competencies/competency-recommendation-detail.component.spec.ts(1 hunks)src/test/javascript/spec/component/competencies/generate-competencies/course-description-form.component.spec.ts(1 hunks)src/test/javascript/spec/component/competencies/import/competency-search.component.spec.ts(1 hunks)src/test/javascript/spec/component/course/course-overview.component.spec.ts(3 hunks)src/test/javascript/spec/component/exam/manage/exam-import.component.spec.ts(1 hunks)src/test/javascript/spec/component/exam/participate/exercises/modeling-exam-submission.component.spec.ts(1 hunks)src/test/javascript/spec/component/exam/participate/exercises/quiz-exam-submission.component.spec.ts(1 hunks)src/test/javascript/spec/component/git-diff-report/git-diff-report.component.spec.ts(1 hunks)src/test/javascript/spec/component/iris/settings/iris-settings-update-component.spec.ts(2 hunks)src/test/javascript/spec/component/overview/course-conversations/course-conversations.component.spec.ts(4 hunks)src/test/javascript/spec/component/overview/course-conversations/course-wide-search.component.spec.ts(2 hunks)src/test/javascript/spec/component/overview/course-conversations/dialogs/channels-overview-dialog/channels-overview-dialog.component.spec.ts(2 hunks)src/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-detail-dialog/tabs/conversation-members/conversation-member-row/conversation-member-row.component.spec.ts(2 hunks)src/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-detail-dialog/tabs/conversation-members/conversation-members.component.spec.ts(2 hunks)src/test/javascript/spec/component/overview/course-conversations/layout/conversation-thread-sidebar/conversation-thread-sidebar.component.spec.ts(1 hunks)src/test/javascript/spec/component/programming-exercise/build-plan-editor.component.spec.ts(1 hunks)src/test/javascript/spec/component/shared/navbar.component.spec.ts(1 hunks)src/test/javascript/spec/component/shared/result.component.spec.ts(2 hunks)src/test/javascript/spec/component/standardized-competencies/admin-import-standardized-competencies.spec.ts(0 hunks)src/test/javascript/spec/component/standardized-competencies/detail/knowledge-area-edit.component.spec.ts(1 hunks)src/test/javascript/spec/component/standardized-competencies/detail/standardized-competency-edit.spec.ts(1 hunks)src/test/javascript/spec/component/tutorial-groups/shared/tutorial-group-detail.component.spec.ts(2 hunks)src/test/javascript/spec/component/tutorial-groups/shared/tutorial-group-sessions-table.component.spec.ts(1 hunks)src/test/javascript/spec/component/tutorial-groups/shared/tutorial-groups-table.component.spec.ts(2 hunks)src/test/javascript/spec/helpers/mocks/directive/ngbAlertsMocks.module.ts(0 hunks)src/test/javascript/spec/helpers/mocks/directive/ngbCollapseMocks.module.ts(0 hunks)src/test/javascript/spec/helpers/mocks/directive/ngbDropdownMocks.module.ts(0 hunks)src/test/javascript/spec/helpers/mocks/directive/ngbPaginationMocks.module.ts(0 hunks)src/test/javascript/spec/helpers/mocks/directive/ngbTooltipMocks.module.ts(0 hunks)
💤 Files with no reviewable changes (6)
- src/test/javascript/spec/component/standardized-competencies/admin-import-standardized-competencies.spec.ts
- src/test/javascript/spec/helpers/mocks/directive/ngbCollapseMocks.module.ts
- src/test/javascript/spec/helpers/mocks/directive/ngbTooltipMocks.module.ts
- src/test/javascript/spec/helpers/mocks/directive/ngbPaginationMocks.module.ts
- src/test/javascript/spec/helpers/mocks/directive/ngbAlertsMocks.module.ts
- src/test/javascript/spec/helpers/mocks/directive/ngbDropdownMocks.module.ts
✅ Files skipped from review due to trivial changes (6)
- src/test/javascript/spec/component/standardized-competencies/detail/knowledge-area-edit.component.spec.ts
- src/test/javascript/spec/component/exam/participate/exercises/modeling-exam-submission.component.spec.ts
- src/test/javascript/spec/component/exam/manage/exam-import.component.spec.ts
- src/test/javascript/spec/component/tutorial-groups/shared/tutorial-groups-table.component.spec.ts
- src/test/javascript/spec/component/code-editor/code-editor-header.component.spec.ts
- src/test/javascript/spec/component/overview/course-conversations/course-conversations.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
`src/test/javascript/spec/**/*.ts`: jest: true; mock: NgMock...
src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-detail-dialog/tabs/conversation-members/conversation-members.component.spec.tssrc/test/javascript/spec/component/competencies/import/competency-search.component.spec.tssrc/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-detail-dialog/tabs/conversation-members/conversation-member-row/conversation-member-row.component.spec.tssrc/test/javascript/spec/component/competencies/generate-competencies/course-description-form.component.spec.tssrc/test/javascript/spec/component/competencies/generate-competencies/competency-recommendation-detail.component.spec.tssrc/test/javascript/spec/component/git-diff-report/git-diff-report.component.spec.tssrc/test/javascript/spec/component/tutorial-groups/shared/tutorial-group-sessions-table.component.spec.tssrc/test/javascript/spec/component/standardized-competencies/detail/standardized-competency-edit.spec.tssrc/test/javascript/spec/component/overview/course-conversations/layout/conversation-thread-sidebar/conversation-thread-sidebar.component.spec.tssrc/test/javascript/spec/component/tutorial-groups/shared/tutorial-group-detail.component.spec.tssrc/test/javascript/spec/component/programming-exercise/build-plan-editor.component.spec.tssrc/test/javascript/spec/component/competencies/competency-accordion.component.spec.tssrc/test/javascript/spec/component/exam/participate/exercises/quiz-exam-submission.component.spec.tssrc/test/javascript/spec/component/overview/course-conversations/course-wide-search.component.spec.tssrc/test/javascript/spec/component/shared/navbar.component.spec.tssrc/test/javascript/spec/component/overview/course-conversations/dialogs/channels-overview-dialog/channels-overview-dialog.component.spec.tssrc/test/javascript/spec/component/course/course-overview.component.spec.tssrc/test/javascript/spec/component/shared/result.component.spec.tssrc/test/javascript/spec/component/admin/system-notification-management/system-notification-management.component.spec.tssrc/test/javascript/spec/component/iris/settings/iris-settings-update-component.spec.ts
🪛 Biome (1.9.4)
src/test/javascript/spec/component/iris/settings/iris-settings-update-component.spec.ts
[error] 50-50: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (21)
src/test/javascript/spec/component/overview/course-conversations/layout/conversation-thread-sidebar/conversation-thread-sidebar.component.spec.ts (1)
19-19: LGTM! Proper removal of unused mock module.The empty imports array is correct as the component doesn't require tooltip functionality in its tests. The test suite properly uses ng-mocks for component and directive mocking.
src/test/javascript/spec/component/competencies/import/competency-search.component.spec.ts (1)
16-16: LGTM! Proper module configuration.The imports are correctly configured with only the essential modules (ArtemisTestModule, ArtemisFormsModule) needed for testing the component's form functionality.
src/test/javascript/spec/component/competencies/competency-accordion.component.spec.ts (1)
18-18: LGTM! Proper removal of unused mock module.The empty imports array is correct as the component doesn't require tooltip functionality in its tests.
src/test/javascript/spec/component/competencies/generate-competencies/course-description-form.component.spec.ts (1)
18-18: LGTM! Proper module configuration.The imports are correctly configured with only the essential modules (ArtemisTestModule, ReactiveFormsModule) needed for testing the component's form functionality. The test suite makes excellent use of ng-mocks for component and directive mocking.
src/test/javascript/spec/component/admin/system-notification-management/system-notification-management.component.spec.ts (1)
33-33: LGTM! Removal of NgbPaginationMocksModule is safe.The test suite remains functional as pagination testing only requires event dispatching, which doesn't need the mock module.
src/test/javascript/spec/component/competencies/generate-competencies/competency-recommendation-detail.component.spec.ts (1)
21-21: LGTM! Removal of NgbTooltipMocksModule and NgbCollapseMocksModule is safe.The test suite remains functional as it focuses on component logic rather than tooltip or collapse behavior.
src/test/javascript/spec/component/standardized-competencies/detail/standardized-competency-edit.spec.ts (1)
43-43: LGTM! Removal of NgbTooltipMocksModule is safe.The test suite remains functional as it focuses on form logic and CRUD operations rather than tooltip behavior.
src/test/javascript/spec/component/programming-exercise/build-plan-editor.component.spec.ts (1)
35-35: LGTM! Removal of NgbTooltipMocksModule is safe.The test suite remains functional as it focuses on editor logic and build plan operations rather than tooltip behavior.
src/test/javascript/spec/component/overview/course-conversations/dialogs/channels-overview-dialog/channels-overview-dialog.component.spec.ts (1)
50-50: LGTM! Safe to remove NgbCollapseMocksModule.The removal of
NgbCollapseMocksModuleis safe as the test suite usesng-mocksfor component and service mocking, and the test coverage remains comprehensive.src/test/javascript/spec/component/tutorial-groups/shared/tutorial-group-detail.component.spec.ts (1)
62-63: LGTM! Safe to remove NgbTooltipMocksModule.The removal of
NgbTooltipMocksModuleis safe as the test suite usesng-mocksfor component and service mocking. The remainingRouterModule.forRoot([])import is sufficient for the test requirements.Also applies to: 115-116
src/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-detail-dialog/tabs/conversation-members/conversation-members.component.spec.ts (1)
41-41: LGTM! Safe to remove NgbPaginationMocksModule.The removal of
NgbPaginationMocksModuleis safe as:
- The test suite uses
ng-mocksfor component mocking.- Pagination functionality is covered through the
ItemCountComponentmock.- The remaining imports (
FormsModule, ReactiveFormsModule) are sufficient for form testing.src/test/javascript/spec/component/tutorial-groups/shared/tutorial-group-sessions-table.component.spec.ts (1)
118-118: LGTM! Safe to remove NgbCollapseMocksModule.The removal of
NgbCollapseMocksModuleis safe as the test suite usesng-mocksfor component and service mocking. The remainingArtemisTestModuleimport is sufficient for the test requirements.src/test/javascript/spec/component/git-diff-report/git-diff-report.component.spec.ts (1)
17-17: LGTM! Safe removal of NgbTooltipMocksModule.The removal of the mock module is appropriate as the test suite doesn't verify any tooltip-related functionality, and ArtemisTestModule provides sufficient testing infrastructure.
src/test/javascript/spec/component/overview/course-conversations/course-wide-search.component.spec.ts (1)
60-60: LGTM! Safe removal of mock module.The imports are correctly reduced to only the essential modules (FormsModule, ReactiveFormsModule) needed for testing the component's form functionality.
src/test/javascript/spec/component/exam/participate/exercises/quiz-exam-submission.component.spec.ts (1)
52-52: LGTM! Safe removal of imports array.The empty imports array is appropriate as the tests focus on business logic, with UI components properly mocked using MockComponent and MockDirective.
src/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-detail-dialog/tabs/conversation-members/conversation-member-row/conversation-member-row.component.spec.ts (1)
63-63: LGTM! Appropriate module configuration.The imports array correctly retains NgbTooltipModule while removing unnecessary mock modules, as the component requires tooltip functionality but not dropdown behavior.
src/test/javascript/spec/component/shared/result.component.spec.ts (2)
4-4: LGTM!The addition of
MissingResultInformationimport is necessary as it's used in the test cases for verifying missing result information messages.
88-88: LGTM!The removal of
NgbTooltipMocksModulefrom imports is appropriate as the tooltip functionality is still covered throughMockDirective(TranslateDirective)in the declarations array.src/test/javascript/spec/component/course/course-overview.component.spec.ts (2)
3-4: LGTM!The import order changes are minor refactoring improvements that don't affect functionality.
166-166: LGTM!The replacement of
NgbDropdownMocksModulewithMockDirective(NgbDropdown)is a good improvement as it:
- Reduces dependency on mock modules
- Uses ng-mocks' MockDirective which is more maintainable
- Maintains test coverage
Also applies to: 202-202
src/test/javascript/spec/component/shared/navbar.component.spec.ts (1)
95-95: LGTM!The removal of
NgbTooltipMocksModule,NgbCollapseMocksModule, andNgbDropdownMocksModulefrom imports is appropriate as the component's functionality is still properly tested through other mock declarations and providers.
src/test/javascript/spec/component/iris/settings/iris-settings-update-component.spec.ts
Outdated
Show resolved
Hide resolved
krusche
dismissed stale reviews from coolchock, xHadie, cremertim, and coderabbitai[bot]
via
a4a4cad
Checklist
General
Client
Motivation and Context
After #10184 and #10267, the shared modules and the mock modules used in tests remain.
This PR deletes the latter ones.
Description
Remove all mock modules and their usages in test files
Steps for Testing
Code Review only.
Make sure all client tests pass.
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Test Coverage
unchanged
Summary by CodeRabbit
Summary by CodeRabbit
Tests
Chores
Configuration