From 8ea5e088809b6ea3fe0335d3fdd59f8c77301574 Mon Sep 17 00:00:00 2001 From: Ryan Nystrom Date: Sat, 12 Nov 2016 09:48:23 -0800 Subject: [PATCH] Add edge-case unit tests Summary: - Removed dead code in batch data and updater - Tested updating with empty index sets - Tested updating with not-found section controller - Tested reloading when collection view or data source are not set - Tested `-[IGListAdapter sectionForObject:]` - Tested index path return for not-found section controller - Tested pointer comparison with `NSObject+IGListDiffable` category Fixes #190, #189 - [x] All tests pass. Demo project builds and runs. - [x] I added tests, an experiment, or detailed why my change isn't tested. - [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/CONTRIBUTING.md) Closes https://github.com/Instagram/IGListKit/pull/191 Differential Revision: D4172781 Pulled By: rnystrom fbshipit-source-id: c25cc917e9a1ebc5fc94fa022e01f99c80c2466c --- Source/IGListAdapterUpdater.m | 21 ----- Source/IGListBatchUpdateData.mm | 15 ---- Source/Internal/IGListAdapterInternal.h | 4 +- Tests/IGListAdapterTests.m | 104 +++++++++++++++++++++++- Tests/IGListDiffTests.m | 5 ++ 5 files changed, 111 insertions(+), 38 deletions(-) diff --git a/Source/IGListAdapterUpdater.m b/Source/IGListAdapterUpdater.m index c1e28276..06316d23 100644 --- a/Source/IGListAdapterUpdater.m +++ b/Source/IGListAdapterUpdater.m @@ -221,17 +221,6 @@ static NSArray *objectsWithDuplicateIdentifiersRemoved(NSArray *)indexPaths removingSections:(NSIndexSet *)sections { - NSMutableSet *filteredIndexPaths = [indexPaths mutableCopy]; - for (NSIndexPath *indexPath in indexPaths) { - const NSUInteger section = indexPath.section; - if ([sections containsIndex:section]) { - [filteredIndexPaths removeObject:indexPath]; - } - } - return filteredIndexPaths; -} - void convertReloadToDeleteInsert(NSMutableIndexSet *reloads, NSMutableIndexSet *deletes, NSMutableIndexSet *inserts, @@ -300,16 +289,6 @@ void convertReloadToDeleteInsert(NSMutableIndexSet *reloads, self.batchUpdateOrReloadInProgress = NO; } -- (NSArray *)trimmedIndexPaths:(NSArray *)indexPaths inSections:(NSIndexSet *)sections { - NSMutableArray *paths = [indexPaths mutableCopy]; - for (NSInteger i = 0; i < paths.count; i++) { - if ([sections containsIndex:[paths[i] section]]) { - [paths removeObjectAtIndex:i]; - } - } - return paths; -} - - (void)cleanupState { self.queuedUpdateIsAnimated = YES; diff --git a/Source/IGListBatchUpdateData.mm b/Source/IGListBatchUpdateData.mm index f2c5fe32..bd402015 100644 --- a/Source/IGListBatchUpdateData.mm +++ b/Source/IGListBatchUpdateData.mm @@ -40,21 +40,6 @@ static void convertMoveToDeleteAndInsert(NSMutableSet *moves, @implementation IGListBatchUpdateData -// Converts all moves that have section operations into a section delete + insert. -+ (void)cleanIndexSetWithMap:(const std::unordered_map &)map - moves:(NSMutableSet *)moves - sections:(NSMutableIndexSet *)sections - deletes:(NSMutableIndexSet *)deletes - inserts:(NSMutableIndexSet *)inserts { - for(const auto &i : map) { - const NSUInteger index = i.first; - if ([sections containsIndex:index]) { - [sections removeIndex:index]; - convertMoveToDeleteAndInsert(moves, i.second, deletes, inserts); - } - } -} - // Converts all section moves that have index path operations into a section delete + insert. + (void)cleanIndexPathsWithMap:(const std::unordered_map &)map moves:(NSMutableSet *)moves diff --git a/Source/Internal/IGListAdapterInternal.h b/Source/Internal/IGListAdapterInternal.h index fc1def34..52129258 100644 --- a/Source/Internal/IGListAdapterInternal.h +++ b/Source/Internal/IGListAdapterInternal.h @@ -32,7 +32,8 @@ IGListCollectionContext __weak UICollectionView *_collectionView; } -@property (nonatomic, strong, readonly) id updatingDelegate; +@property (nonatomic, strong) id updatingDelegate; + @property (nonatomic, strong, readonly) IGListSectionMap *sectionMap; @property (nonatomic, strong, readonly) IGListDisplayHandler *displayHandler; @property (nonatomic, strong, readonly) IGListWorkingRangeHandler *workingRangeHandler; @@ -61,6 +62,7 @@ IGListCollectionContext - (NSArray *)indexPathsFromSectionController:(IGListSectionController *)sectionController indexes:(NSIndexSet *)indexes adjustForUpdateBlock:(BOOL)adjustForUpdateBlock; +- (NSIndexPath *)indexPathForSectionController:(IGListSectionController *)controller index:(NSInteger)index; @end diff --git a/Tests/IGListAdapterTests.m b/Tests/IGListAdapterTests.m index a013a8a9..7ae1b17d 100644 --- a/Tests/IGListAdapterTests.m +++ b/Tests/IGListAdapterTests.m @@ -13,7 +13,6 @@ #import #import -#import #import "IGListAdapterInternal.h" #import "IGListTestAdapterDataSource.h" @@ -626,4 +625,107 @@ XCTAssertEqual(CGPointEqualToPoint(point, p), YES); \ IGAssertEqualPoint([self.collectionView contentOffset], 0, 10); } +- (void)test_whenQueryingIndexPath_withOOBSectionController_thatNilReturned { + self.dataSource.objects = @[@0, @1, @2]; + [self.adapter reloadDataWithCompletion:nil]; + + id randomSectionController = [IGListSectionController new]; + XCTAssertNil([self.adapter indexPathForSectionController:randomSectionController index:0]); +} + +- (void)test_whenQueryingSectionForObject_thatSectionReturned { + self.dataSource.objects = @[@0, @1, @2]; + [self.adapter reloadDataWithCompletion:nil]; + XCTAssertEqual([self.adapter sectionForObject:@0], 0); + XCTAssertEqual([self.adapter sectionForObject:@1], 1); + XCTAssertEqual([self.adapter sectionForObject:@2], 2); + XCTAssertEqual([self.adapter sectionForObject:@3], NSNotFound); +} + +- (void)test_whenReloadingData_withNoDataSource_thatCompletionCalledWithNO { + self.dataSource.objects = @[@1]; + IGListAdapter *adapter = [[IGListAdapter alloc] initWithUpdater:[IGListReloadDataUpdater new] + viewController:nil + workingRangeSize:0]; + adapter.collectionView = self.collectionView; + + __block BOOL executed = NO; + [adapter reloadDataWithCompletion:^(BOOL finished) { + executed = YES; + XCTAssertFalse(finished); + }]; + XCTAssertTrue(executed); +} + +- (void)test_whenReloadingData_withNoCollectionView_thatCompletionCalledWithNO { + self.dataSource.objects = @[@1]; + IGListAdapter *adapter = [[IGListAdapter alloc] initWithUpdater:[IGListReloadDataUpdater new] + viewController:nil + workingRangeSize:0]; + adapter.dataSource = self.dataSource; + + __block BOOL executed = NO; + [adapter reloadDataWithCompletion:^(BOOL finished) { + executed = YES; + XCTAssertFalse(finished); + }]; + XCTAssertTrue(executed); +} + +- (void)test_whenSectionControllerReloading_withEmptyIndexes_thatNoUpdatesHappen { + self.dataSource.objects = @[@0, @1, @2]; + [self.adapter reloadDataWithCompletion:nil]; + + id mockDelegate = [OCMockObject mockForProtocol:@protocol(IGListUpdatingDelegate)]; + [[mockDelegate reject] reloadItemsInCollectionView:[OCMArg any] indexPaths:[OCMArg any]]; + self.adapter.updatingDelegate = mockDelegate; + + id sectionController = [self.adapter sectionControllerForObject:@1]; + [self.adapter reloadInSectionController:sectionController atIndexes:[NSIndexSet new]]; + + [mockDelegate verify]; +} + +- (void)test_whenSectionControllerDeleting_withEmptyIndexes_thatNoUpdatesHappen { + self.dataSource.objects = @[@0, @1, @2]; + [self.adapter reloadDataWithCompletion:nil]; + + id mockDelegate = [OCMockObject mockForProtocol:@protocol(IGListUpdatingDelegate)]; + [[mockDelegate reject] deleteItemsFromCollectionView:[OCMArg any] indexPaths:[OCMArg any]]; + self.adapter.updatingDelegate = mockDelegate; + + id sectionController = [self.adapter sectionControllerForObject:@1]; + [self.adapter deleteInSectionController:sectionController atIndexes:[NSIndexSet new]]; + + [mockDelegate verify]; +} + +- (void)test_whenSectionControllerInserting_withEmptyIndexes_thatNoUpdatesHappen { + self.dataSource.objects = @[@0, @1, @2]; + [self.adapter reloadDataWithCompletion:nil]; + + id mockDelegate = [OCMockObject mockForProtocol:@protocol(IGListUpdatingDelegate)]; + [[mockDelegate reject] insertItemsIntoCollectionView:[OCMArg any] indexPaths:[OCMArg any]]; + self.adapter.updatingDelegate = mockDelegate; + + id sectionController = [self.adapter sectionControllerForObject:@1]; + [self.adapter insertInSectionController:sectionController atIndexes:[NSIndexSet new]]; + + [mockDelegate verify]; +} + +- (void)test_whenReloading_withSectionControllerNotFound_thatNoUpdatesHappen { + self.dataSource.objects = @[@0, @1, @2]; + [self.adapter reloadDataWithCompletion:nil]; + + id mockDelegate = [OCMockObject mockForProtocol:@protocol(IGListUpdatingDelegate)]; + [[mockDelegate reject] reloadCollectionView:[OCMArg any] sections:[OCMArg any]]; + self.adapter.updatingDelegate = mockDelegate; + + id sectionController = [IGListSectionController new]; + [self.adapter reloadSectionController:sectionController]; + + [mockDelegate verify]; +} + @end diff --git a/Tests/IGListDiffTests.m b/Tests/IGListDiffTests.m index 1e7763bf..eb60c06f 100644 --- a/Tests/IGListDiffTests.m +++ b/Tests/IGListDiffTests.m @@ -428,4 +428,9 @@ static NSArray *sorted(NSArray *arr) { XCTAssertEqualObjects(sorted(result.inserts), expectedInserts); } +- (void)test_whenComparingDiffableObjects_withDefaultCategory_thatPointersAreAlwaysEqual { + NSObject *object = [NSObject new]; + XCTAssertTrue([object isEqualToDiffableObject:object]); +} + @end