XCTest exception when using keyValueObservingExpectationForObject:keyPath:handler:

Question

In my unit tests, I am using the -[XCTestCase keyValueObservingExpectationForObject:keyPath:handler:] method in order to ensure that my NSOperation finishes, here is the code from my XCDYouTubeKit project:

- (void) testStartingOnBackgroundThread
{
    XCDYouTubeVideoOperation *operation = [[XCDYouTubeVideoOperation alloc] initWithVideoIdentifier:nil languageIdentifier:nil];
    [self keyValueObservingExpectationForObject:operation keyPath:@"isFinished" handler:^BOOL(id observedObject, NSDictionary *change)
    {
        XCTAssertNil([observedObject video]);
        XCTAssertNotNil([observedObject error]);
        return YES;
    }];

    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
        XCTAssertFalse([NSThread isMainThread]);
        [operation start];
    });
    [self waitForExpectationsWithTimeout:5 handler:nil];
}

This test always passes when I run it locally on my Mac but sometimes it fails on Travis with this error:

failed: caught "NSRangeException", "Cannot remove an observer <_XCKVOExpectation 0x1001846c0> for the key path "isFinished" from <XCDYouTubeVideoOperation 0x1001b9510> because it is not registered as an observer."

Am I doing something wrong?

Answer 1

Your code is correct, you have found a bug in the XCTest framework. Here is an in depth explanation, you can skip to the end of this answer if you are just looking for a workaround.

When you call keyValueObservingExpectationForObject:keyPath:handler:, an _XCKVOExpectation object is created under the hood. It is responsible for observing the object/keyPath you passed. Once the KVO notification has fired, the _safelyUnregister method is called, this is where the observer is removed. Here is the (reverse engineered) implementation of the _safelyUnregister method.

@implementation _XCKVOExpectation

- (void) _safelyUnregister
{
    if (!self.hasUnregistered)
    {
        [self.observedObject removeObserver:self forKeyPath:self.keyPath];
        self.hasUnregistered = YES;
    }
}

@end

This method is called once again at the end of waitForExpectationsWithTimeout:handler: and when the _XCKVOExpectation object is deallocated. Note that the operation terminates on a background thread but the test is run on the main thread. So you have a race condition: if _safelyUnregister is called on the main thread before the hasUnregistered property is set to YES on the background thread, the observer is removed twice, causing the Cannot remove an observer exception.

So in order to workaround this issue, you have to protect the _safelyUnregister method with a lock. Here is a code snippet for you to compile in your test target that will take care of fixing this bug.

#import <objc/runtime.h>

__attribute__((constructor)) void WorkaroundXCKVOExpectationUnregistrationRaceCondition(void);
__attribute__((constructor)) void WorkaroundXCKVOExpectationUnregistrationRaceCondition(void)
{
    SEL _safelyUnregisterSEL = sel_getUid("_safelyUnregister");
    Method safelyUnregister = class_getInstanceMethod(objc_lookUpClass("_XCKVOExpectation"), _safelyUnregisterSEL);
    void (*_safelyUnregisterIMP)(id, SEL) = (__typeof__(_safelyUnregisterIMP))method_getImplementation(safelyUnregister);
    method_setImplementation(safelyUnregister, imp_implementationWithBlock(^(id self) {
        @synchronized(self)
        {
            _safelyUnregisterIMP(self, _safelyUnregisterSEL);
        }
    }));
}

EDIT

This bug has been fixed in Xcode 7 beta 4.