Skip to content

Commit

Permalink
Merge pull request #741 from Quick/avoid-method-swizzling
Browse files Browse the repository at this point in the history
Stop using method swizzling for registering CurrentTestCaseTracker to XCTestObservationCenter
  • Loading branch information
ikesyo authored May 8, 2020
2 parents 5111520 + 0d07988 commit e55e958
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 85 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
runs-on: macOS-latest
strategy:
matrix:
xcode: [11.4]
xcode: [11.4.1]
platform: [macos, ios, tvos, macos_xcodespm, ios_xcodespm]
steps:
- uses: actions/checkout@v2
Expand All @@ -35,7 +35,7 @@ jobs:
runs-on: macOS-latest
strategy:
matrix:
xcode: [11.4]
xcode: [11.4.1]
steps:
- uses: actions/checkout@v2
- run: sudo xcode-select -s '/Applications/Xcode_${{ matrix.xcode }}.app'
Expand Down
77 changes: 3 additions & 74 deletions Sources/NimbleObjectiveC/XCTestObservationCenter+Register.m
Original file line number Diff line number Diff line change
@@ -1,83 +1,12 @@
#import <XCTest/XCTest.h>
#import <objc/runtime.h>

#if __has_include("Nimble-Swift.h")
#import "Nimble-Swift.h"
#else
#import <Nimble/Nimble-Swift.h>
#endif

#pragma mark - Method Swizzling

/// Swaps the implementations between two instance methods.
///
/// @param class The class containing `originalSelector`.
/// @param originalSelector Original method to replace.
/// @param replacementSelector Replacement method.
void swizzleSelectors(Class class, SEL originalSelector, SEL replacementSelector) {
Method originalMethod = class_getInstanceMethod(class, originalSelector);
Method replacementMethod = class_getInstanceMethod(class, replacementSelector);

BOOL didAddMethod =
class_addMethod(class,
originalSelector,
method_getImplementation(replacementMethod),
method_getTypeEncoding(replacementMethod));

if (didAddMethod) {
class_replaceMethod(class,
replacementSelector,
method_getImplementation(originalMethod),
method_getTypeEncoding(originalMethod));
} else {
method_exchangeImplementations(originalMethod, replacementMethod);
}
}

#pragma mark - Private

@interface XCTestObservationCenter (Private)
- (void)_addLegacyTestObserver:(id)observer;
@end

@implementation XCTestObservationCenter (Register)

/// 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:));
}
__attribute__((constructor))
static void registerCurrentTestCaseTracker(void) {
[[XCTestObservationCenter sharedTestObservationCenter] addTestObserver:[CurrentTestCaseTracker sharedInstance]];
}

#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]];
});
}

@end
20 changes: 11 additions & 9 deletions test
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ if which xcodebuild > /dev/null; then
echo -e "Gathering ${GREEN}xcodebuild sdk versions${CLEAR}..."
BUILD_DIR=`pwd`/build
LATEST_IOS_SDK_VERSION=`xcodebuild -showsdks | grep iphonesimulator | cut -d ' ' -f 4 | ruby -e 'puts STDIN.read.chomp.split("\n").last'`
LATEST_IOS_VERSION=`xcrun simctl list | grep ^iOS | ruby -e 'puts /\(([0-9.]+).*\)/.match(STDIN.read.chomp.split("\n").last).to_a[1]'`
LATEST_TVOS_SDK_VERSION=`xcodebuild -showsdks | grep appletvsimulator | cut -d ' ' -f 4 | ruby -e 'puts STDIN.read.chomp.split("\n").last'`
LATEST_MACOS_SDK_VERSION=`xcodebuild -showsdks | grep 'macosx' | cut -d ' ' -f 3 | ruby -e 'puts STDIN.read.chomp.split("\n").last'`
LATEST_TVOS_VERSION=`xcrun simctl list | grep ^tvOS | ruby -e 'puts /\(([0-9.]+).*\)/.match(STDIN.read.chomp.split("\n").last).to_a[1]'`
LATEST_MACOS_SDK_VERSION=`xcodebuild -showsdks | grep 'macosx' | cut -d ' ' -f 2 | ruby -e 'puts STDIN.read.chomp.split("\n").last'`
BUILD_IOS_SDK_VERSION=${NIMBLE_BUILD_IOS_SDK_VERSION:-$LATEST_IOS_SDK_VERSION}
RUNTIME_IOS_SDK_VERSION=${NIMBLE_RUNTIME_IOS_SDK_VERSION:-$LATEST_IOS_SDK_VERSION}
RUNTIME_IOS_VERSION=${NIMBLE_RUNTIME_IOS_VERSION:-$LATEST_IOS_VERSION}
BUILD_TVOS_SDK_VERSION=${NIMBLE_BUILD_TVOS_SDK_VERSION:-$LATEST_TVOS_SDK_VERSION}
RUNTIME_TVOS_SDK_VERSION=${NIMBLE_RUNTIME_TVOS_SDK_VERSION:-$LATEST_TVOS_SDK_VERSION}
RUNTIME_TVOS_VERSION=${NIMBLE_RUNTIME_TVOS_VERSION:-$LATEST_TVOS_VERSION}
BUILD_MACOS_SDK_VERSION=${NIMBLE_BUILD_MACOS_SDK_VERSION:-$LATEST_MACOS_SDK_VERSION}
fi

Expand All @@ -33,12 +35,12 @@ function print_env {
echo " iOS:"
echo " Latest iOS SDK: $LATEST_IOS_SDK_VERSION"
echo " Building with iOS SDK: `color_if_overridden $BUILD_IOS_SDK_VERSION $NIMBLE_BUILD_IOS_SDK_VERSION`"
echo " Running with iOS SDK: `color_if_overridden $RUNTIME_IOS_SDK_VERSION $NIMBLE_RUNTIME_IOS_SDK_VERSION`"
echo " Running with iOS: `color_if_overridden $RUNTIME_IOS_VERSION $NIMBLE_RUNTIME_IOS_VERSION`"
echo
echo " tvOS:"
echo " Latest tvOS SDK: $LATEST_TVOS_SDK_VERSION"
echo " Building with tvOS SDK: `color_if_overridden $BUILD_TVOS_SDK_VERSION $NIMBLE_BUILD_TVOS_SDK_VERSION`"
echo " Running with tvOS SDK: `color_if_overridden $RUNTIME_TVOS_SDK_VERSION $NIMBLE_RUNTIME_TVOS_SDK_VERSION`"
echo " Running with tvOS: `color_if_overridden $RUNTIME_TVOS_VERSION $NIMBLE_RUNTIME_TVOS_VERSION`"
echo
echo " macOS:"
echo " Latest macOS SDK: $LATEST_MACOS_SDK_VERSION"
Expand All @@ -57,17 +59,17 @@ function test_ios {
run set -o pipefail && xcodebuild -project Nimble.xcodeproj -scheme "Nimble-iOS" -configuration "Debug" -destination "generic/platform=iOS" OTHER_SWIFT_FLAGS='$(inherited) -suppress-warnings' build | xcpretty

run osascript -e 'tell app "Simulator" to quit'
run set -o pipefail && xcodebuild -project Nimble.xcodeproj -scheme "Nimble-iOS" -configuration "Debug" -sdk "iphonesimulator$BUILD_IOS_SDK_VERSION" -destination "name=iPad Pro (9.7-inch),OS=$RUNTIME_IOS_SDK_VERSION" OTHER_SWIFT_FLAGS='$(inherited) -suppress-warnings' build-for-testing test-without-building | xcpretty
run set -o pipefail && xcodebuild -project Nimble.xcodeproj -scheme "Nimble-iOS" -configuration "Debug" -sdk "iphonesimulator$BUILD_IOS_SDK_VERSION" -destination "name=iPad Pro (9.7-inch),OS=$RUNTIME_IOS_VERSION" OTHER_SWIFT_FLAGS='$(inherited) -suppress-warnings' build-for-testing test-without-building | xcpretty

run osascript -e 'tell app "Simulator" to quit'
run set -o pipefail && xcodebuild -project Nimble.xcodeproj -scheme "Nimble-iOS" -configuration "Debug" -sdk "iphonesimulator$BUILD_IOS_SDK_VERSION" -destination "name=iPhone 8,OS=$RUNTIME_IOS_SDK_VERSION" OTHER_SWIFT_FLAGS='$(inherited) -suppress-warnings' build-for-testing test-without-building | xcpretty
run set -o pipefail && xcodebuild -project Nimble.xcodeproj -scheme "Nimble-iOS" -configuration "Debug" -sdk "iphonesimulator$BUILD_IOS_SDK_VERSION" -destination "name=iPhone 8,OS=$RUNTIME_IOS_VERSION" OTHER_SWIFT_FLAGS='$(inherited) -suppress-warnings' build-for-testing test-without-building | xcpretty
}

function test_tvos {
run set -o pipefail && xcodebuild -project Nimble.xcodeproj -scheme "Nimble-tvOS" -configuration "Debug" -destination "generic/platform=tvOS" OTHER_SWIFT_FLAGS='$(inherited) -suppress-warnings' build | xcpretty

run osascript -e 'tell app "Simulator" to quit'
run set -o pipefail && xcodebuild -project Nimble.xcodeproj -scheme "Nimble-tvOS" -configuration "Debug" -sdk "appletvsimulator$BUILD_TVOS_SDK_VERSION" -destination "name=Apple TV,OS=$RUNTIME_TVOS_SDK_VERSION" OTHER_SWIFT_FLAGS='$(inherited) -suppress-warnings' build-for-testing test-without-building | xcpretty
run set -o pipefail && xcodebuild -project Nimble.xcodeproj -scheme "Nimble-tvOS" -configuration "Debug" -sdk "appletvsimulator$BUILD_TVOS_SDK_VERSION" -destination "name=Apple TV,OS=$RUNTIME_TVOS_VERSION" OTHER_SWIFT_FLAGS='$(inherited) -suppress-warnings' build-for-testing test-without-building | xcpretty
}

function test_macos {
Expand All @@ -84,7 +86,7 @@ function test_xcode_spm_ios {
run osascript -e 'tell app "Simulator" to quit'
mv Nimble.xcodeproj Nimble.xcodeproj.bak
trap 'mv Nimble.xcodeproj.bak Nimble.xcodeproj' EXIT
run set -o pipefail && xcodebuild -scheme "Nimble" -configuration "Debug" -sdk "iphonesimulator$BUILD_IOS_SDK_VERSION" -destination "name=iPhone 8,OS=$RUNTIME_IOS_SDK_VERSION" OTHER_SWIFT_FLAGS='$(inherited) -suppress-warnings' build-for-testing test-without-building | xcpretty
run set -o pipefail && xcodebuild -scheme "Nimble" -configuration "Debug" -sdk "iphonesimulator$BUILD_IOS_SDK_VERSION" -destination "name=iPhone 8,OS=$RUNTIME_IOS_VERSION" OTHER_SWIFT_FLAGS='$(inherited) -suppress-warnings' build-for-testing test-without-building | xcpretty
}

function test_podspec {
Expand Down

0 comments on commit e55e958

Please sign in to comment.