Test/calendar provider #1766
Test/calendar provider #1766
Conversation
Codecov Report❌ Patch coverage is ❌ Your project check has failed because the head coverage (7%) is below the target coverage (70%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #1766 +/- ##
=========================================
- Coverage 60% 7% -52%
=========================================
Files 2 38 +36
Lines 81 1755 +1674
=========================================
+ Hits 48 121 +73
- Misses 33 1634 +1601 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive unit tests for the CalendarNotifier provider with support for dependency injection to enable testing. The tests validate cache management, internationalization, error handling, and state synchronization.
Key changes:
- Implements a FakeCalendarFetcher mock for testing without real I/O operations
- Adds dependency injection capability to CalendarNotifier via optional constructor parameter
- Enhances CachedAsyncNotifier with ref.mounted guards to prevent state updates on unmounted providers
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/uni_app/test/unit/providers/calendar_provider_test.dart | New test file with 7 test cases covering event loading, cache expiration, error handling, and locale support |
| packages/uni_app/lib/model/providers/riverpod/calendar_provider.dart | Adds optional fetcher parameter to constructor to support dependency injection for testing |
| packages/uni_app/lib/model/providers/riverpod/cached_async_notifier.dart | Adds ref.mounted checks to prevent state updates after provider disposal |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
|
|
||
| test('Must reload from remote if cache is expired', () async { | ||
| final oldDate = DateTime.now().subtract(const Duration(days: 31)); |
There was a problem hiding this comment.
The test uses 31 days for cache expiration, but the CalendarNotifier has a cacheDuration of 30 days (line 20 in calendar_provider.dart). This mismatch means the test is using an incorrect value. The test should use 30 days to match the actual cache duration, or use a value slightly larger than 30 days to ensure the cache is expired.
| final oldDate = DateTime.now().subtract(const Duration(days: 31)); | |
| final oldDate = DateTime.now().subtract(const Duration(days: 30)); |
| expect(data?.getEvents(AppLocale.en).length, 0); | ||
| expect(data?.getEvents(AppLocale.pt).length, 0); |
There was a problem hiding this comment.
Redundant test assertions. Lines 87-88 check if the lists are empty using isEmpty, and lines 89-90 check if the length is 0. These assertions are testing the same condition in different ways. Consider removing lines 89-90 as they add no additional test coverage.
| expect(data?.getEvents(AppLocale.en).length, 0); | |
| expect(data?.getEvents(AppLocale.pt).length, 0); |
| ); | ||
| }); | ||
|
|
||
| test('Data must update when the language changes', () async { |
There was a problem hiding this comment.
The test name suggests it validates behavior when the language changes, but it doesn't actually test a language change scenario. The test loads data once, invalidates the provider, and loads again, but both calls would fetch both PT and EN events simultaneously (as seen in loadFromStorage). This test actually validates that provider invalidation works, not that data updates when language changes. Consider renaming the test to better reflect what it actually tests, such as 'Must reload all events when provider is invalidated'.
| test('Data must update when the language changes', () async { | |
| test('Must reload all events when provider is invalidated', () async { |
| expect(state.error.toString(), contains('Fetcher failed')); | ||
| }); | ||
|
|
||
| test('Must reload from remote if cache is expired', () async { |
There was a problem hiding this comment.
The comment mentions "31 days" as the cache period, but the actual cacheDuration is 30 days as defined in CalendarNotifier. Update the comment to accurately reflect the 30-day cache duration.
| fakeFetcher.mockGroups['en'] = [ | ||
| CalendarEvent( | ||
| name: 'Início de aulas, outros', | ||
| startDate: DateTime.parse('2025-09-15'), | ||
| endDate: DateTime.parse('2025-09-15'), | ||
| ), |
There was a problem hiding this comment.
The event name is in Portuguese ('Início de aulas, outros') but it's being added to the English events list (fakeFetcher.mockGroups['en']). This appears to be a copy-paste error. The event should either have an English name like 'Start of classes, others' to match the language, or should be added to the Portuguese events list instead.
Description
This task involves the implementation of robust unit tests for the CalendarNotifier. The primary goal was to validate synchronization logic, cache management, ensuring the provider behaves correctly across local and remote data scenarios.
Implementation Details
FakeCalendarFetcherto simulate API/JSON responses and error scenarios without relying on real I/O.CachedAsyncNotifier.AsyncErrorstate when the fetcher fails, ensuring the application handles exceptions gracefully.