From 8847ebeef28e2e0adb3bede5f987e72cdf2b66b4 Mon Sep 17 00:00:00 2001 From: shenlong <139912620+shenlong-tanwen@users.noreply.github.com> Date: Sun, 10 Dec 2023 02:31:23 +0000 Subject: [PATCH] fix(mobile): mobile album sort not persisting (#5584) * chore(deps): use mocktail instead of mockito * refactor: move stubs to fixtures/ * fix: fetch assetsortmode based on storeindex * test: validate AlbumSortByOptions provider --------- Co-authored-by: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> --- .gitignore | 1 + .../album_sort_by_options.provider.dart | 2 +- mobile/pubspec.lock | 10 +- mobile/pubspec.yaml | 2 +- .../album_sort_by_options_provider_test.dart | 427 ++++++++++++------ mobile/test/{ => fixtures}/album.stub.dart | 0 mobile/test/{ => fixtures}/asset.stub.dart | 0 mobile/test/{ => fixtures}/user.stub.dart | 0 .../mocks/app_settings_provider.mock.dart | 9 + mobile/test/sync_service_test.dart | 2 +- mobile/test/test_utils.dart | 71 +++ 11 files changed, 372 insertions(+), 152 deletions(-) rename mobile/test/{ => fixtures}/album.stub.dart (100%) rename mobile/test/{ => fixtures}/asset.stub.dart (100%) rename mobile/test/{ => fixtures}/user.stub.dart (100%) create mode 100644 mobile/test/mocks/app_settings_provider.mock.dart create mode 100644 mobile/test/test_utils.dart diff --git a/.gitignore b/.gitignore index a3d042a09b..628a077fa2 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ coverage mobile/gradle.properties mobile/openapi/pubspec.lock mobile/*.jks +mobile/libisar.dylib diff --git a/mobile/lib/modules/album/providers/album_sort_by_options.provider.dart b/mobile/lib/modules/album/providers/album_sort_by_options.provider.dart index 2309e19aa7..6d1622a481 100644 --- a/mobile/lib/modules/album/providers/album_sort_by_options.provider.dart +++ b/mobile/lib/modules/album/providers/album_sort_by_options.provider.dart @@ -99,7 +99,7 @@ class AlbumSortByOptions extends _$AlbumSortByOptions { .watch(appSettingsServiceProvider) .getSetting(AppSettingsEnum.selectedAlbumSortOrder); return AlbumSortMode.values.firstWhere( - (e) => e.index == sortOpt, + (e) => e.storeIndex == sortOpt, orElse: () => AlbumSortMode.title, ); } diff --git a/mobile/pubspec.lock b/mobile/pubspec.lock index 7cb4188d90..26a135d8d6 100644 --- a/mobile/pubspec.lock +++ b/mobile/pubspec.lock @@ -964,14 +964,14 @@ packages: url: "https://pub.dev" source: hosted version: "1.0.4" - mockito: - dependency: "direct dev" + mocktail: + dependency: "direct main" description: - name: mockito - sha256: "7d5b53bcd556c1bc7ffbe4e4d5a19c3e112b7e925e9e172dd7c6ad0630812616" + name: mocktail + sha256: bac151b31e4ed78bd59ab89aa4c0928f297b1180186d5daf03734519e5f596c1 url: "https://pub.dev" source: hosted - version: "5.4.2" + version: "1.0.1" nested: dependency: transitive description: diff --git a/mobile/pubspec.yaml b/mobile/pubspec.yaml index 7355807185..6f7f9218f6 100644 --- a/mobile/pubspec.yaml +++ b/mobile/pubspec.yaml @@ -58,6 +58,7 @@ dependencies: wakelock_plus: ^1.1.1 flutter_local_notifications: ^15.1.0+1 timezone: ^0.9.2 + mocktail: ^1.0.1 openapi: path: openapi @@ -84,7 +85,6 @@ dev_dependencies: flutter_launcher_icons: "^0.9.2" flutter_native_splash: ^2.2.16 isar_generator: *isar_version - mockito: ^5.3.2 integration_test: sdk: flutter custom_lint: ^0.5.6 diff --git a/mobile/test/album_sort_by_options_provider_test.dart b/mobile/test/album_sort_by_options_provider_test.dart index 18efc5b423..30b235166b 100644 --- a/mobile/test/album_sort_by_options_provider_test.dart +++ b/mobile/test/album_sort_by_options_provider_test.dart @@ -1,182 +1,321 @@ import 'package:collection/collection.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/modules/album/providers/album_sort_by_options.provider.dart'; +import 'package:immich_mobile/modules/settings/services/app_settings.service.dart'; import 'package:immich_mobile/shared/models/album.dart'; import 'package:immich_mobile/shared/models/asset.dart'; -import 'package:immich_mobile/shared/models/user.dart'; import 'package:isar/isar.dart'; +import 'package:mocktail/mocktail.dart'; -import 'album.stub.dart'; -import 'asset.stub.dart'; +import 'fixtures/album.stub.dart'; +import 'fixtures/asset.stub.dart'; +import 'mocks/app_settings_provider.mock.dart'; +import 'test_utils.dart'; void main() { - late final Isar db; + /// Verify the sort modes + group("AlbumSortMode", () { + late final Isar db; - setUpAll(() async { - await Isar.initializeIsarCore(download: true); - db = await Isar.open( - [ - AssetSchema, - AlbumSchema, - UserSchema, - ], - maxSizeMiB: 256, - directory: ".", - ); - }); - - final albums = [ - AlbumStub.emptyAlbum, - AlbumStub.sharedWithUser, - AlbumStub.oneAsset, - AlbumStub.twoAsset, - ]; - - setUp(() { - db.writeTxnSync(() { - db.clearSync(); - // Save all assets - db.assets.putAllSync([AssetStub.image1, AssetStub.image2]); - db.albums.putAllSync(albums); - for (final album in albums) { - album.sharedUsers.saveSync(); - album.assets.saveSync(); - } - }); - expect(db.albums.countSync(), 4); - expect(db.assets.countSync(), 2); - }); - - group("Album sort - Created Time", () { - const created = AlbumSortMode.created; - test("Created time - ASC", () { - final sorted = created.sortFn(albums, false); - expect(sorted.isSortedBy((a) => a.createdAt), true); + setUpAll(() async { + db = await TestUtils.initIsar(); }); - test("Created time - DESC", () { - final sorted = created.sortFn(albums, true); - expect( - sorted.isSorted((b, a) => a.createdAt.compareTo(b.createdAt)), - true, - ); + final albums = [ + AlbumStub.emptyAlbum, + AlbumStub.sharedWithUser, + AlbumStub.oneAsset, + AlbumStub.twoAsset, + ]; + + setUp(() { + db.writeTxnSync(() { + db.clearSync(); + // Save all assets + db.assets.putAllSync([AssetStub.image1, AssetStub.image2]); + db.albums.putAllSync(albums); + for (final album in albums) { + album.sharedUsers.saveSync(); + album.assets.saveSync(); + } + }); + expect(db.albums.countSync(), 4); + expect(db.assets.countSync(), 2); + }); + + group("Album sort - Created Time", () { + const created = AlbumSortMode.created; + test("Created time - ASC", () { + final sorted = created.sortFn(albums, false); + expect(sorted.isSortedBy((a) => a.createdAt), true); + }); + + test("Created time - DESC", () { + final sorted = created.sortFn(albums, true); + expect( + sorted.isSorted((b, a) => a.createdAt.compareTo(b.createdAt)), + true, + ); + }); + }); + + group("Album sort - Asset count", () { + const assetCount = AlbumSortMode.assetCount; + test("Asset Count - ASC", () { + final sorted = assetCount.sortFn(albums, false); + expect( + sorted.isSorted((a, b) => a.assetCount.compareTo(b.assetCount)), + true, + ); + }); + + test("Asset Count - DESC", () { + final sorted = assetCount.sortFn(albums, true); + expect( + sorted.isSorted((b, a) => a.assetCount.compareTo(b.assetCount)), + true, + ); + }); + }); + + group("Album sort - Last modified", () { + const lastModified = AlbumSortMode.lastModified; + test("Last modified - ASC", () { + final sorted = lastModified.sortFn(albums, false); + expect( + sorted.isSorted((a, b) => a.modifiedAt.compareTo(b.modifiedAt)), + true, + ); + }); + + test("Last modified - DESC", () { + final sorted = lastModified.sortFn(albums, true); + expect( + sorted.isSorted((b, a) => a.modifiedAt.compareTo(b.modifiedAt)), + true, + ); + }); + }); + + group("Album sort - Created", () { + const created = AlbumSortMode.created; + test("Created - ASC", () { + final sorted = created.sortFn(albums, false); + expect( + sorted.isSorted((a, b) => a.createdAt.compareTo(b.createdAt)), + true, + ); + }); + + test("Created - DESC", () { + final sorted = created.sortFn(albums, true); + expect( + sorted.isSorted((b, a) => a.createdAt.compareTo(b.createdAt)), + true, + ); + }); + }); + + group("Album sort - Most Recent", () { + const mostRecent = AlbumSortMode.mostRecent; + + test("Most Recent - ASC", () { + final sorted = mostRecent.sortFn(albums, false); + expect( + sorted, + [ + AlbumStub.sharedWithUser, + AlbumStub.twoAsset, + AlbumStub.oneAsset, + AlbumStub.emptyAlbum, + ], + ); + }); + + test("Most Recent - DESC", () { + final sorted = mostRecent.sortFn(albums, true); + expect( + sorted, + [ + AlbumStub.emptyAlbum, + AlbumStub.oneAsset, + AlbumStub.twoAsset, + AlbumStub.sharedWithUser, + ], + ); + }); + }); + + group("Album sort - Most Oldest", () { + const mostOldest = AlbumSortMode.mostOldest; + + test("Most Oldest - ASC", () { + final sorted = mostOldest.sortFn(albums, false); + expect( + sorted, + [ + AlbumStub.twoAsset, + AlbumStub.emptyAlbum, + AlbumStub.oneAsset, + AlbumStub.sharedWithUser, + ], + ); + }); + + test("Most Oldest - DESC", () { + final sorted = mostOldest.sortFn(albums, true); + expect( + sorted, + [ + AlbumStub.sharedWithUser, + AlbumStub.oneAsset, + AlbumStub.emptyAlbum, + AlbumStub.twoAsset, + ], + ); + }); }); }); - group("Album sort - Asset count", () { - const assetCount = AlbumSortMode.assetCount; - test("Asset Count - ASC", () { - final sorted = assetCount.sortFn(albums, false); - expect( - sorted.isSorted((a, b) => a.assetCount.compareTo(b.assetCount)), - true, + /// Verify the sort mode provider + group('AlbumSortByOptions', () { + late AppSettingsService settingsMock; + late ProviderContainer container; + + setUp(() async { + settingsMock = AppSettingsServiceMock(); + container = TestUtils.createContainer( + overrides: [getAppSettingsServiceMock(settingsMock)], ); }); - test("Asset Count - DESC", () { - final sorted = assetCount.sortFn(albums, true); + test('Returns the default sort mode when none set', () { + // Returns the default value when nothing is set + when( + () => settingsMock.getSetting(AppSettingsEnum.selectedAlbumSortOrder), + ).thenReturn(0); + + expect(AlbumSortMode.created, container.read(albumSortByOptionsProvider)); + }); + + test('Returns the correct sort mode with index from Store', () { + // Returns the default value when nothing is set + when( + () => settingsMock.getSetting(AppSettingsEnum.selectedAlbumSortOrder), + ).thenReturn(3); + expect( - sorted.isSorted((b, a) => a.assetCount.compareTo(b.assetCount)), - true, + AlbumSortMode.lastModified, + container.read(albumSortByOptionsProvider), ); }); + + test('Properly saves the correct store index of sort mode', () { + container + .read(albumSortByOptionsProvider.notifier) + .changeSortMode(AlbumSortMode.mostOldest); + + verify( + () => settingsMock.setSetting( + AppSettingsEnum.selectedAlbumSortOrder, + AlbumSortMode.mostOldest.storeIndex, + ), + ); + }); + + test('Notifies listeners on state change', () { + when( + () => settingsMock.getSetting(AppSettingsEnum.selectedAlbumSortOrder), + ).thenReturn(0); + + final listener = ListenerMock(); + + container.listen( + albumSortByOptionsProvider, + listener, + fireImmediately: true, + ); + + // Created -> Most Oldest + container + .read(albumSortByOptionsProvider.notifier) + .changeSortMode(AlbumSortMode.mostOldest); + + // Most Oldest -> Title + container + .read(albumSortByOptionsProvider.notifier) + .changeSortMode(AlbumSortMode.title); + + verifyInOrder([ + () => listener.call(null, AlbumSortMode.created), + () => listener.call(AlbumSortMode.created, AlbumSortMode.mostOldest), + () => listener.call(AlbumSortMode.mostOldest, AlbumSortMode.title), + ]); + + verifyNoMoreInteractions(listener); + }); }); - group("Album sort - Last modified", () { - const lastModified = AlbumSortMode.lastModified; - test("Last modified - ASC", () { - final sorted = lastModified.sortFn(albums, false); - expect( - sorted.isSorted((a, b) => a.modifiedAt.compareTo(b.modifiedAt)), - true, + /// Verify the sort order provider + group('AlbumSortOrder', () { + late AppSettingsService settingsMock; + late ProviderContainer container; + + setUp(() async { + settingsMock = AppSettingsServiceMock(); + container = TestUtils.createContainer( + overrides: [getAppSettingsServiceMock(settingsMock)], ); }); - test("Last modified - DESC", () { - final sorted = lastModified.sortFn(albums, true); - expect( - sorted.isSorted((b, a) => a.modifiedAt.compareTo(b.modifiedAt)), - true, - ); - }); - }); + test('Returns the default sort order when none set - false', () { + when( + () => settingsMock.getSetting(AppSettingsEnum.selectedAlbumSortReverse), + ).thenReturn(false); - group("Album sort - Created", () { - const created = AlbumSortMode.created; - test("Created - ASC", () { - final sorted = created.sortFn(albums, false); - expect( - sorted.isSorted((a, b) => a.createdAt.compareTo(b.createdAt)), - true, + expect(false, container.read(albumSortOrderProvider)); + }); + + test('Properly saves the correct order', () { + container.read(albumSortOrderProvider.notifier).changeSortDirection(true); + + verify( + () => settingsMock.setSetting( + AppSettingsEnum.selectedAlbumSortReverse, + true, + ), ); }); - test("Created - DESC", () { - final sorted = created.sortFn(albums, true); - expect( - sorted.isSorted((b, a) => a.createdAt.compareTo(b.createdAt)), - true, + test('Notifies listeners on state change', () { + when( + () => settingsMock.getSetting(AppSettingsEnum.selectedAlbumSortReverse), + ).thenReturn(false); + + final listener = ListenerMock(); + + container.listen( + albumSortOrderProvider, + listener, + fireImmediately: true, ); - }); - }); - group("Album sort - Most Recent", () { - const mostRecent = AlbumSortMode.mostRecent; + // false -> true + container.read(albumSortOrderProvider.notifier).changeSortDirection(true); - test("Most Recent - ASC", () { - final sorted = mostRecent.sortFn(albums, false); - expect( - sorted, - [ - AlbumStub.sharedWithUser, - AlbumStub.twoAsset, - AlbumStub.oneAsset, - AlbumStub.emptyAlbum, - ], - ); - }); + // true -> false + container + .read(albumSortOrderProvider.notifier) + .changeSortDirection(false); - test("Most Recent - DESC", () { - final sorted = mostRecent.sortFn(albums, true); - expect( - sorted, - [ - AlbumStub.emptyAlbum, - AlbumStub.oneAsset, - AlbumStub.twoAsset, - AlbumStub.sharedWithUser, - ], - ); - }); - }); + verifyInOrder([ + () => listener.call(null, false), + () => listener.call(false, true), + () => listener.call(true, false), + ]); - group("Album sort - Most Oldest", () { - const mostOldest = AlbumSortMode.mostOldest; - - test("Most Oldest - ASC", () { - final sorted = mostOldest.sortFn(albums, false); - expect( - sorted, - [ - AlbumStub.twoAsset, - AlbumStub.emptyAlbum, - AlbumStub.oneAsset, - AlbumStub.sharedWithUser, - ], - ); - }); - - test("Most Oldest - DESC", () { - final sorted = mostOldest.sortFn(albums, true); - expect( - sorted, - [ - AlbumStub.sharedWithUser, - AlbumStub.oneAsset, - AlbumStub.emptyAlbum, - AlbumStub.twoAsset, - ], - ); + verifyNoMoreInteractions(listener); }); }); } diff --git a/mobile/test/album.stub.dart b/mobile/test/fixtures/album.stub.dart similarity index 100% rename from mobile/test/album.stub.dart rename to mobile/test/fixtures/album.stub.dart diff --git a/mobile/test/asset.stub.dart b/mobile/test/fixtures/asset.stub.dart similarity index 100% rename from mobile/test/asset.stub.dart rename to mobile/test/fixtures/asset.stub.dart diff --git a/mobile/test/user.stub.dart b/mobile/test/fixtures/user.stub.dart similarity index 100% rename from mobile/test/user.stub.dart rename to mobile/test/fixtures/user.stub.dart diff --git a/mobile/test/mocks/app_settings_provider.mock.dart b/mobile/test/mocks/app_settings_provider.mock.dart new file mode 100644 index 0000000000..fbdf67a411 --- /dev/null +++ b/mobile/test/mocks/app_settings_provider.mock.dart @@ -0,0 +1,9 @@ +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/modules/settings/providers/app_settings.provider.dart'; +import 'package:immich_mobile/modules/settings/services/app_settings.service.dart'; +import 'package:mocktail/mocktail.dart'; + +class AppSettingsServiceMock with Mock implements AppSettingsService {} + +Override getAppSettingsServiceMock(AppSettingsService service) => + appSettingsServiceProvider.overrideWith((ref) => service); diff --git a/mobile/test/sync_service_test.dart b/mobile/test/sync_service_test.dart index 9abb863522..1d00875541 100644 --- a/mobile/test/sync_service_test.dart +++ b/mobile/test/sync_service_test.dart @@ -11,7 +11,7 @@ import 'package:immich_mobile/shared/services/hash.service.dart'; import 'package:immich_mobile/shared/services/immich_logger.service.dart'; import 'package:immich_mobile/shared/services/sync.service.dart'; import 'package:isar/isar.dart'; -import 'package:mockito/mockito.dart'; +import 'package:mocktail/mocktail.dart'; void main() { Asset makeAsset({ diff --git a/mobile/test/test_utils.dart b/mobile/test/test_utils.dart new file mode 100644 index 0000000000..5052f59107 --- /dev/null +++ b/mobile/test/test_utils.dart @@ -0,0 +1,71 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/modules/backup/models/backup_album.model.dart'; +import 'package:immich_mobile/modules/backup/models/duplicated_asset.model.dart'; +import 'package:immich_mobile/shared/models/album.dart'; +import 'package:immich_mobile/shared/models/android_device_asset.dart'; +import 'package:immich_mobile/shared/models/asset.dart'; +import 'package:immich_mobile/shared/models/etag.dart'; +import 'package:immich_mobile/shared/models/exif_info.dart'; +import 'package:immich_mobile/shared/models/ios_device_asset.dart'; +import 'package:immich_mobile/shared/models/logger_message.model.dart'; +import 'package:immich_mobile/shared/models/store.dart'; +import 'package:immich_mobile/shared/models/user.dart'; +import 'package:isar/isar.dart'; +import 'package:mocktail/mocktail.dart'; + +// Listener Mock to test when a provider notifies its listeners +class ListenerMock extends Mock { + // ignore: avoid-declaring-call-method + void call(T? previous, T next); +} + +final class TestUtils { + const TestUtils._(); + + /// Downloads Isar binaries (if required) and initializes a new Isar db + static Future initIsar() async { + await Isar.initializeIsarCore(download: true); + final db = await Isar.open( + [ + StoreValueSchema, + ExifInfoSchema, + AssetSchema, + AlbumSchema, + UserSchema, + BackupAlbumSchema, + DuplicatedAssetSchema, + LoggerMessageSchema, + ETagSchema, + AndroidDeviceAssetSchema, + IOSDeviceAssetSchema, + ], + maxSizeMiB: 256, + directory: ".", + ); + // Clear and close db on test end + addTearDown(() async { + await db.writeTxn(() => db.clear()); + await db.close(); + }); + return db; + } + + /// Creates a new ProviderContainer to test Riverpod providers + static ProviderContainer createContainer({ + ProviderContainer? parent, + List overrides = const [], + List? observers, + }) { + final container = ProviderContainer( + parent: parent, + overrides: overrides, + observers: observers, + ); + + // Dispose on test end + addTearDown(container.dispose); + + return container; + } +}