Remove "context saved" expectations from CoreDataStack tests (#18485)

wordpress-mobile · May 3, 2022 · 90cc17e · 90cc17e
2 parents 53482e9 + c6873f2
commit 90cc17e
Show file tree

Hide file tree

Showing 8 changed files with 25 additions and 93 deletions.
diff --git a/WordPress/WordPressTest/AccountServiceTests.swift b/WordPress/WordPressTest/AccountServiceTests.swift
@@ -10,7 +10,6 @@ class AccountServiceTests: XCTestCase {
         super.setUp()
 
         contextManager = TestContextManager()
-        contextManager.requiresTestExpectation = false
         accountService = AccountService(managedObjectContext: contextManager.mainContext)
     }
 

diff --git a/WordPress/WordPressTest/AtomicAuthenticationServiceTests.swift b/WordPress/WordPressTest/AtomicAuthenticationServiceTests.swift
@@ -11,7 +11,6 @@ class AtomicAuthenticationServiceTests: XCTestCase {
         super.setUp()
 
         contextManager = TestContextManager()
-        contextManager.requiresTestExpectation = false
 
         let api = WordPressComRestApi(oAuthToken: "")
         let remote = AtomicAuthenticationServiceRemote(wordPressComRestApi: api)

diff --git a/WordPress/WordPressTest/BlogJetpackTest.m b/WordPress/WordPressTest/BlogJetpackTest.m
@@ -75,19 +75,17 @@ - (void)testJetpackUsername {
 }
 
 - (void)testJetpackSetupDoesntReplaceDotcomAccount {
-    XCTestExpectation *saveExpectation = [self expectationWithDescription:@"Context save expectation"];
-    self.testContextManager.testExpectation = saveExpectation;
+    XCTestExpectation *saveExpectation = [self expectationForNotification:NSManagedObjectContextDidSaveNotification object:self.testContextManager.mainContext handler:nil];
 
-    AccountService *accountService = [[AccountService alloc] initWithManagedObjectContext:[ContextManager sharedInstance].mainContext];
+    AccountService *accountService = [[AccountService alloc] initWithManagedObjectContext:self.testContextManager.mainContext];
     WPAccount *wpComAccount = [accountService createOrUpdateAccountWithUsername:@"user" authToken:@"token"];
-    [self waitForExpectationsWithTimeout:2.0 handler:nil];
+    [self waitForExpectations:@[saveExpectation] timeout:2.0];
     WPAccount * defaultAccount = [accountService defaultWordPressComAccount];
     XCTAssertEqualObjects(wpComAccount, defaultAccount);
 
-    saveExpectation = [self expectationWithDescription:@"Context save expectation"];
-    self.testContextManager.testExpectation = saveExpectation;
+    saveExpectation = [self expectationForNotification:NSManagedObjectContextDidSaveNotification object:self.testContextManager.mainContext handler:nil];
     [accountService createOrUpdateAccountWithUsername:@"test1" authToken:@"token1"];
-    [self waitForExpectationsWithTimeout:2.0 handler:nil];
+    [self waitForExpectations:@[saveExpectation] timeout:2.0];
     defaultAccount = [accountService defaultWordPressComAccount];
     XCTAssertEqualObjects(wpComAccount, defaultAccount);
 }
@@ -101,8 +99,7 @@ - (void)testWPCCShouldntDuplicateBlogs {
                                                 statusCode:200 headers:@{@"Content-Type":@"application/json"}];
     }];
 
-    XCTestExpectation *saveExpectation = [self expectationWithDescription:@"Context save expectation"];
-    self.testContextManager.testExpectation = saveExpectation;
+    XCTestExpectation *saveExpectation = [self expectationForNotification:NSManagedObjectContextDidSaveNotification object:self.testContextManager.mainContext handler:nil];
 
     AccountService *accountService = [[AccountService alloc] initWithManagedObjectContext:self.testContextManager.mainContext];
     BlogService *blogService = [[BlogService alloc] initWithManagedObjectContext:self.testContextManager.mainContext];
@@ -130,7 +127,7 @@ - (void)testWPCCShouldntDuplicateBlogs {
                                   };
 
     // Wait on the merge to be completed
-    [self waitForExpectationsWithTimeout:2.0 handler:nil];
+    [self waitForExpectations:@[saveExpectation] timeout:2.0];
 
     // test.blog + wp.com + jetpack
     XCTAssertEqual(1, [accountService numberOfAccounts]);
@@ -174,8 +171,7 @@ - (void)testSyncBlogsMigratesJetpackSSL
                                                 statusCode:200 headers:@{@"Content-Type":@"application/json"}];
     }];
 
-    XCTestExpectation *saveExpectation = [self expectationWithDescription:@"Context save expectation"];
-    self.testContextManager.testExpectation = saveExpectation;
+    XCTestExpectation *saveExpectation = [self expectationForNotification:NSManagedObjectContextDidSaveNotification object:self.testContextManager.mainContext handler:nil];
 
     AccountService *accountService = [[AccountService alloc] initWithManagedObjectContext:self.testContextManager.mainContext];
     BlogService *blogService = [[BlogService alloc] initWithManagedObjectContext:self.testContextManager.mainContext];
@@ -192,7 +188,7 @@ - (void)testSyncBlogsMigratesJetpackSSL
     jetpackBlog.url = @"https://jetpack.example.com/";
 
     // Wait on the merge to be completed
-    [self waitForExpectationsWithTimeout:2.0 handler:nil];
+    [self waitForExpectations:@[saveExpectation] timeout:2.0];
 
     XCTAssertEqual(1, [accountService numberOfAccounts]);
     // test.blog + wp.com + jetpack (legacy)

diff --git a/WordPress/WordPressTest/ContextManagerMock.h b/WordPress/WordPressTest/ContextManagerMock.h
@@ -9,9 +9,7 @@ NS_ASSUME_NONNULL_BEGIN
 @property (nonatomic, readwrite, strong) NSManagedObjectModel           *managedObjectModel;
 @property (nonatomic, readwrite, strong) NSPersistentStoreCoordinator   *persistentStoreCoordinator;
 @property (nonatomic, readonly,  strong) NSPersistentStoreCoordinator   *standardPSC;
-@property (nonatomic, readwrite, assign) BOOL                           requiresTestExpectation;
 @property (nonatomic, readonly,  strong) NSURL                          *storeURL;
-@property (nonatomic, nullable,  readwrite, strong) XCTestExpectation   *testExpectation;
 @end
 
 /**

diff --git a/WordPress/WordPressTest/ContextManagerMock.m b/WordPress/WordPressTest/ContextManagerMock.m
@@ -13,8 +13,6 @@ @implementation ContextManagerMock
 @synthesize persistentStoreCoordinator = _persistentStoreCoordinator;
 @synthesize mainContext = _mainContext;
 @synthesize managedObjectModel = _managedObjectModel;
-@synthesize requiresTestExpectation = _requiresTestExpectation;
-@synthesize testExpectation = _testExpectation;
 
 - (instancetype)init
 {
@@ -23,7 +21,6 @@ - (instancetype)init
         // Override the shared ContextManager
         [ContextManager internalSharedInstance];
         [ContextManager overrideSharedInstance:self];
-        _requiresTestExpectation = YES;
     }
 
     return self;
@@ -79,40 +76,13 @@ - (NSManagedObjectContext *)mainContext
     return _mainContext;
 }
 
-- (void)saveContext:(NSManagedObjectContext *)context
-{
-    [self saveContext:context withCompletionBlock:^{
-        if (self.testExpectation) {
-            [self.testExpectation fulfill];
-            self.testExpectation = nil;
-        } else if (self.requiresTestExpectation) {
-            NSLog(@"No test expectation present for context save");
-        }
-    }];
-}
-
 - (void)saveContextAndWait:(NSManagedObjectContext *)context
 {
     [super saveContextAndWait:context];
-    if (self.testExpectation) {
-        [self.testExpectation fulfill];
-        self.testExpectation = nil;
-    } else if (self.requiresTestExpectation) {
-        NSLog(@"No test expectation present for context save");
-    }
-}
-
-- (void)saveContext:(NSManagedObjectContext *)context withCompletionBlock:(void (^)(void))completionBlock
-{
-    [super saveContext:context withCompletionBlock:^{
-        if (self.testExpectation) {
-            [self.testExpectation fulfill];
-            self.testExpectation = nil;
-        } else if (self.requiresTestExpectation) {
-            NSLog(@"No test expectation present for context save");
-        }
-        completionBlock();
-    }];
+    // FIXME: Remove this method to use superclass one instead
+    // This log magically resolves a deadlock in
+    // `ZDashboardCardTests.testShouldNotShowQuickStartIfDefaultSectionIsSiteMenu`
+    NSLog(@"Context save completed");
 }
 
 - (NSURL *)storeURL

diff --git a/WordPress/WordPressTest/NotificationSyncMediatorTests.swift b/WordPress/WordPressTest/NotificationSyncMediatorTests.swift
@@ -62,8 +62,7 @@ class NotificationSyncMediatorTests: XCTestCase {
         XCTAssert(manager.mainContext.countObjects(ofType: Notification.self) == 0)
 
         // CoreData Expectations
-        manager.testExpectation = expectation(description: "Context save expectation")
-
+        let contextSaved = expectation(forNotification: .NSManagedObjectContextDidSave, object: manager.mainContext)
 
         // Mediator Expectations
         let expect = expectation(description: "Sync")
@@ -74,7 +73,7 @@ class NotificationSyncMediatorTests: XCTestCase {
             expect.fulfill()
         }
 
-        waitForExpectations(timeout: timeout, handler: nil)
+        wait(for: [contextSaved, expect], timeout: timeout)
     }
 
 
@@ -129,7 +128,7 @@ class NotificationSyncMediatorTests: XCTestCase {
         XCTAssert(manager.mainContext.countObjects(ofType: Notification.self) == 0)
 
         // CoreData Expectations
-        manager.testExpectation = expectation(description: "Context save expectation")
+        let contextSaved = expectation(forNotification: .NSManagedObjectContextDidSave, object: manager.mainContext)
 
         // Mediator Expectations
         let expect = expectation(description: "Sync")
@@ -141,7 +140,7 @@ class NotificationSyncMediatorTests: XCTestCase {
             expect.fulfill()
         }
 
-        waitForExpectations(timeout: timeout, handler: nil)
+        wait(for: [contextSaved, expect], timeout: timeout)
     }
 
 
@@ -161,7 +160,7 @@ class NotificationSyncMediatorTests: XCTestCase {
         XCTAssertFalse(note.read)
 
         // CoreData Expectations
-        manager.testExpectation = expectation(description: "Context save expectation")
+        let contextSaved = expectation(forNotification: .NSManagedObjectContextDidSave, object: manager.mainContext)
 
         // Mediator Expectations
         let expect = expectation(description: "Mark as Read")
@@ -172,7 +171,7 @@ class NotificationSyncMediatorTests: XCTestCase {
             expect.fulfill()
         }
 
-        waitForExpectations(timeout: timeout, handler: nil)
+        wait(for: [contextSaved, expect], timeout: timeout)
     }
 
     /// Verifies that Mark Notifications as Read effectively toggles a Notifications' read flag
@@ -197,7 +196,7 @@ class NotificationSyncMediatorTests: XCTestCase {
         XCTAssertTrue(note2.read)
 
         // CoreData Expectations
-        manager.testExpectation = expectation(description: "Context save expectation")
+        let contextSaved = expectation(forNotification: .NSManagedObjectContextDidSave, object: manager.mainContext)
 
         // Mediator Expectations
         let expect = expectation(description: "Mark as Read")
@@ -210,7 +209,7 @@ class NotificationSyncMediatorTests: XCTestCase {
             expect.fulfill()
         }
 
-        waitForExpectations(timeout: timeout, handler: nil)
+        wait(for: [contextSaved, expect], timeout: timeout)
     }
 
     /// Verifies that Mark Notifications as Read modifies only the specified notifications' read status
@@ -235,7 +234,7 @@ class NotificationSyncMediatorTests: XCTestCase {
         XCTAssertTrue(note2.read)
 
         // CoreData Expectations
-        manager.testExpectation = expectation(description: "Context save expectation")
+        let contextSaved = expectation(forNotification: .NSManagedObjectContextDidSave, object: manager.mainContext)
 
         // Mediator Expectations
         let expect = expectation(description: "Mark as Read")
@@ -247,7 +246,7 @@ class NotificationSyncMediatorTests: XCTestCase {
             expect.fulfill()
         }
 
-        waitForExpectations(timeout: timeout, handler: nil)
+        wait(for: [contextSaved, expect], timeout: timeout)
     }
 
 

diff --git a/WordPress/WordPressTest/TestContextManager.h b/WordPress/WordPressTest/TestContextManager.h
@@ -18,9 +18,7 @@ NS_ASSUME_NONNULL_BEGIN
 @property (nonatomic, readwrite, strong) NSManagedObjectModel           *managedObjectModel;
 @property (nonatomic, readwrite, strong) NSPersistentStoreCoordinator   *persistentStoreCoordinator;
 @property (nonatomic, readonly,  strong) NSPersistentStoreCoordinator   *standardPSC;
-@property (nonatomic, readwrite, assign) BOOL                           requiresTestExpectation;
 @property (nonatomic, readonly,  strong) NSURL                          *storeURL;
-@property (nonatomic, nullable,  readwrite, strong) XCTestExpectation   *testExpectation;
 @property (nonatomic, strong, nullable) id<ManagerMock, CoreDataStack>  stack;
 
 

diff --git a/WordPress/WordPressTest/TestContextManager.m b/WordPress/WordPressTest/TestContextManager.m
@@ -14,7 +14,6 @@ - (instancetype)init
     if (self) {
         // Override the shared ContextManager
         _stack = [[ContextManagerMock alloc] init];
-        _requiresTestExpectation = YES;
     }
 
     return self;
@@ -55,45 +54,19 @@ - (void)setMainContext:(NSManagedObjectContext *)mainContext
     [_stack setMainContext:mainContext];
 }
 
--(void)setTestExpectation:(XCTestExpectation *)testExpectation
-{
-    [_stack setTestExpectation:testExpectation];
-}
-
 - (void)saveContext:(NSManagedObjectContext *)context
 {
-    [self saveContext:context withCompletionBlock:^{
-        if (self.stack.testExpectation) {
-            [self.stack.testExpectation fulfill];
-            self.stack.testExpectation = nil;
-        } else if (self.stack.requiresTestExpectation) {
-            NSLog(@"No test expectation present for context save");
-        }
-    }];
+    [_stack saveContext:context];
 }
 
 - (void)saveContextAndWait:(NSManagedObjectContext *)context
 {
     [_stack saveContextAndWait:context];
-    if (self.stack.testExpectation) {
-        [self.stack.testExpectation fulfill];
-        self.stack.testExpectation = nil;
-    } else if (self.stack.requiresTestExpectation) {
-        NSLog(@"No test expectation present for context save");
-    }
 }
 
 - (void)saveContext:(NSManagedObjectContext *)context withCompletionBlock:(void (^)(void))completionBlock
 {
-    [_stack saveContext:context withCompletionBlock:^{
-        if (self.stack.testExpectation) {
-            [self.stack.testExpectation fulfill];
-            self.stack.testExpectation = nil;
-        } else if (self.stack.requiresTestExpectation) {
-            NSLog(@"No test expectation present for context save");
-        }
-        completionBlock();
-    }];
+    [_stack saveContext:context withCompletionBlock:completionBlock];
 }
 
 - (nonnull NSManagedObjectContext *const)newDerivedContext {