Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failing tests throw NSInternalInconsistencyException when using AppCode #740

Closed
mark-anders opened this issue May 5, 2020 · 4 comments · Fixed by #741
Closed

Failing tests throw NSInternalInconsistencyException when using AppCode #740

mark-anders opened this issue May 5, 2020 · 4 comments · Fixed by #741
Assignees
Labels

Comments

@mark-anders
Copy link

I've been using Nimble successfully for years with Xcode and JetBrains' AppCode. However, with recent versions of AppCode that support Swift 5 (which use Xcode 11.4), any test failure throws an NSInternalInconsistencyException right after reporting the error that occurred.

I filed a bug with JetBrains and they narrowed it down to the use of swizzling in the +load method XCTestObservationCenter+Register. You can see that bug here:

https://youtrack.jetbrains.com/issue/OC-19997.

The solution was to replace the swizzling code with the following, which I tried and it worked.

  [[XCTestObservationCenter sharedTestObservationCenter] addTestObserver:[CurrentTestCaseTracker sharedInstance]];

The comment for +load indicates that the swizzling was done due an issue with Xcode 7.3. However, Nimble 9.0.0 now requires 11.4 and it could be that the issue no longer exists. In my fork, I ran all tests for all platforms and everything passed, so it may not be required any longer.

I have a fix and could submit a pull request.

@ikesyo
Copy link
Member

ikesyo commented May 6, 2020

Thanks for filing the issue. I confirmed the followings:

  • -[XCTestObservationCenter _addLegacyTestObserver:] is no longer called on Xcode 10.1 or later (it's okay to apply the fix on v8.x as well)
  • An XCTestLog instance is already added to XCTestObservationCenter when calling sharedTestObservationCenter initially at +load.

That means that we should be able to register CurrentTestCaseTracker safely by +load method or using a C function with __attribute__((constructor)). I'll use the later way.

__attribute__((constructor))
static void registerCurrentTestCaseTracker() {
    [[XCTestObservationCenter sharedTestObservationCenter] addTestObserver:[CurrentTestCaseTracker sharedInstance]];
}

ref:

/// Uses objc method swizzling to register `CurrentTestCaseTracker` as a test observer. This is necessary
/// because Xcode 7.3 introduced timing issues where if a custom `XCTestObservation` is registered too early
/// it suppresses all console output (generated by `XCTestLog`), breaking any tools that depend on this output.
/// This approach waits to register our custom test observer until XCTest adds its first "legacy" observer,
/// falling back to registering after the first normal observer if this private method ever changes.
+ (void)load {
if (class_getInstanceMethod([self class], @selector(_addLegacyTestObserver:))) {
// Swizzle -_addLegacyTestObserver:
swizzleSelectors([self class], @selector(_addLegacyTestObserver:), @selector(NMB_original__addLegacyTestObserver:));
} else {
// Swizzle -addTestObserver:, only if -_addLegacyTestObserver: is not implemented
swizzleSelectors([self class], @selector(addTestObserver:), @selector(NMB_original_addTestObserver:));
}
}
#pragma mark - Replacement Methods
/// Registers `CurrentTestCaseTracker` as a test observer after `XCTestLog` has been added.
- (void)NMB_original__addLegacyTestObserver:(id)observer {
[self NMB_original__addLegacyTestObserver:observer];
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
[self addTestObserver:[CurrentTestCaseTracker sharedInstance]];
});
}
/// Registers `CurrentTestCaseTracker` as a test observer after `XCTestLog` has been added.
/// This method is only used if `-_addLegacyTestObserver:` is not impelemented. (added in Xcode 7.3)
- (void)NMB_original_addTestObserver:(id<XCTestObservation>)observer {
[self NMB_original_addTestObserver:observer];
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
[self NMB_original_addTestObserver:[CurrentTestCaseTracker sharedInstance]];
});
}

@ikesyo
Copy link
Member

ikesyo commented May 6, 2020

@mark-anders Could you try the branch avoid-method-swizzling of #741?

@ikesyo
Copy link
Member

ikesyo commented May 14, 2020

FYI v8.0.8 has been released.

@mark-anders
Copy link
Author

Hi @ikesyo, sorry for the delay. I was traveling and wasn't able to test previously. Thank you so much for the quick fix. It works great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants