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.

Sign up for GitHub

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

Jump to bottom

fix: Finish transaction for external view controllers #3440

Merged
merged 26 commits into from
Nov 29, 2023
Merged

fix: Finish transaction for external view controllers #3440

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
cf9f8da
fix: External view controller transactions not finished
brustolin Nov 22, 2023
24839d4
Update ExternalUIViewController.swift
brustolin Nov 22, 2023
a9cc9a8
Merge branch 'main' into fiz/external-lib-swizzling
brustolin Nov 22, 2023
c441887
Update CHANGELOG.md
brustolin Nov 22, 2023
2c5f1f5
Merge branch 'fiz/external-lib-swizzling' of github.com:getsentry/sen…
brustolin Nov 22, 2023
a236c95
Update CHANGELOG.md
brustolin Nov 22, 2023
a60eff5
Update SentryUIViewControllerSwizzlingTests.swift
brustolin Nov 22, 2023
58708cf
Update project.pbxproj
brustolin Nov 22, 2023
ce79a00
Update project.pbxproj
brustolin Nov 22, 2023
cd449ad
Update no-changes-in-high-risk-files.sh
brustolin Nov 22, 2023
bd6b7ce
Merge branch 'main' into fiz/external-lib-swizzling
brustolin Nov 22, 2023
bfbf299
refs
brustolin Nov 23, 2023
9ef9f2d
Format code
getsentry-bot Nov 23, 2023
c0c330e
lint
brustolin Nov 23, 2023
cff8050
Merge branch 'fiz/external-lib-swizzling' of github.com:getsentry/sen…
brustolin Nov 23, 2023
ed603af
Apply suggestions from code review
brustolin Nov 24, 2023
00668a1
Format code
getsentry-bot Nov 24, 2023
49bfca6
Update Sources/Sentry/SentryUIViewControllerSwizzling.m
brustolin Nov 24, 2023
129c342
Update no-changes-in-high-risk-files.sh
brustolin Nov 27, 2023
0141922
search lib
brustolin Nov 27, 2023
10fc126
ref: Extract checking image name inApp (#3452)
philipphofmann Nov 29, 2023
8e7f7a0
Extra test framework
brustolin Nov 29, 2023
ea2c5ea
Format code
getsentry-bot Nov 29, 2023
b17cbd7
Update project.pbxproj
brustolin Nov 29, 2023
ecb0372
Update project.pbxproj
brustolin Nov 29, 2023
527e6fc
Update Tests/SentryTests/Integrations/Performance/UIViewController/Se…
brustolin Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Finish transaction for external view controllers (#3440)

## 8.16.0

### Features
Expand Down
12 changes: 12 additions & 0 deletions Samples/iOS-Swift/iOS-External/ExternalViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Foundation
import Sentry
import UIKit

class ExternalViewController: UIViewController {

override func viewDidAppear(_ animated: Bool) {
super.viewDidAppear(animated)
SentrySDK.reportFullyDisplayed()
}

}
10 changes: 10 additions & 0 deletions Samples/iOS-Swift/iOS-External/iOS_External.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#import <Foundation/Foundation.h>

//! Project version number for iOS_External.
FOUNDATION_EXPORT double iOS_ExternalVersionNumber;

//! Project version string for iOS_External.
FOUNDATION_EXPORT const unsigned char iOS_ExternalVersionString[];

// In this header, you should import all the public headers of your framework using statements like
// #import <iOS_External/PublicHeader.h>
272 changes: 272 additions & 0 deletions Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions Samples/iOS-Swift/iOS-Swift/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
options.attachViewHierarchy = true
options.environment = "test-app"
options.enableTimeToFullDisplayTracing = true

options.add(inAppInclude: "iOS_External")
brustolin marked this conversation as resolved.
Show resolved Hide resolved

let isBenchmarking = ProcessInfo.processInfo.arguments.contains("--io.sentry.test.benchmarking")

Expand Down
25 changes: 25 additions & 0 deletions Samples/iOS-Swift/iOS-Swift/Base.lproj/Main.storyboard
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,15 @@
<action selector="uiClickTransaction:" destination="BYZ-38-t0r" eventType="touchUpInside" id="UJy-CY-NIQ"/>
</connections>
</button>
<button opaque="NO" contentMode="scaleToFill" contentHorizontalAlignment="center" contentVerticalAlignment="center" buttonType="system" lineBreakMode="middleTruncation" translatesAutoresizingMaskIntoConstraints="NO" id="tvP-G7-z60">
<rect key="frame" x="0.0" y="308" width="259" height="28"/>
<fontDescription key="fontDescription" type="system" pointSize="13"/>
<inset key="imageEdgeInsets" minX="0.0" minY="0.0" maxX="2.2250738585072014e-308" maxY="0.0"/>
<state key="normal" title="External View Controller"/>
<connections>
<segue destination="oCp-Df-BWS" kind="show" id="iFk-us-OK4"/>
</connections>
</button>
</subviews>
</stackView>
</subviews>
Expand Down Expand Up @@ -389,6 +398,22 @@
</objects>
<point key="canvasLocation" x="1020" y="1685"/>
</scene>
<!--External View Controller-->
<scene sceneID="FBQ-Cn-whQ">
<objects>
<viewController id="oCp-Df-BWS" customClass="ExternalViewController" customModule="iOS_External" sceneMemberID="viewController">
<view key="view" contentMode="scaleToFill" id="xra-iw-aq3">
<rect key="frame" x="0.0" y="0.0" width="320" height="568"/>
<autoresizingMask key="autoresizingMask" widthSizable="YES" heightSizable="YES"/>
<viewLayoutGuide key="safeArea" id="Hqn-KI-o6p"/>
<color key="backgroundColor" systemColor="systemBackgroundColor"/>
</view>
<navigationItem key="navigationItem" id="AWo-oj-ODl"/>
</viewController>
<placeholder placeholderIdentifier="IBFirstResponder" id="CiN-t1-bFc" userLabel="First Responder" customClass="UIResponder" sceneMemberID="firstResponder"/>
</objects>
<point key="canvasLocation" x="649" y="2399"/>
</scene>
<!--Navigation Controller-->
<scene sceneID="0gQ-IO-Fn5">
<objects>
Expand Down
16 changes: 10 additions & 6 deletions Sentry.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,7 @@
D808FB88281AB33C009A2A33 /* SentryUIEventTrackerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D808FB86281AB31D009A2A33 /* SentryUIEventTrackerTests.swift */; };
D808FB8B281BCE96009A2A33 /* TestSentrySwizzleWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = D808FB89281BCE46009A2A33 /* TestSentrySwizzleWrapper.swift */; };
D808FB92281BF6EC009A2A33 /* SentryUIEventTrackingIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D808FB90281BF6E9009A2A33 /* SentryUIEventTrackingIntegrationTests.swift */; };
D80C990B2B0DFE410052F311 /* ExternalUIViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = D80C990A2B0DFE410052F311 /* ExternalUIViewController.swift */; };
D8137D54272B53070082656C /* TestSentrySpan.m in Sources */ = {isa = PBXBuildFile; fileRef = D8137D53272B53070082656C /* TestSentrySpan.m */; };
D8199DBE29376EDE0074249E /* SentryInternal.h in Headers */ = {isa = PBXBuildFile; fileRef = D8199DB829376ECC0074249E /* SentryInternal.h */; };
D8199DBF29376EE20074249E /* SentryInternal.m in Sources */ = {isa = PBXBuildFile; fileRef = D8199DB929376ECC0074249E /* SentryInternal.m */; };
Expand Down Expand Up @@ -1690,6 +1691,7 @@
D808FB86281AB31D009A2A33 /* SentryUIEventTrackerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryUIEventTrackerTests.swift; sourceTree = "<group>"; };
D808FB89281BCE46009A2A33 /* TestSentrySwizzleWrapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestSentrySwizzleWrapper.swift; sourceTree = "<group>"; };
D808FB90281BF6E9009A2A33 /* SentryUIEventTrackingIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryUIEventTrackingIntegrationTests.swift; sourceTree = "<group>"; };
D80C990A2B0DFE410052F311 /* ExternalUIViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ExternalUIViewController.swift; sourceTree = "<group>"; };
D8105B8D297FD16800299F03 /* SentryPerformanceTracker+Testing.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "SentryPerformanceTracker+Testing.h"; sourceTree = "<group>"; };
D8137D52272B53070082656C /* TestSentrySpan.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = TestSentrySpan.h; sourceTree = "<group>"; };
D8137D53272B53070082656C /* TestSentrySpan.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = TestSentrySpan.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3220,6 +3222,7 @@
844EDC7829415AB300C86F34 /* TestSentrySystemWrapper.swift */,
844EDCE72947DCD700C86F34 /* TestSentryNSTimerFactory.swift */,
84B7FA3B29B2866200AD93B1 /* SentryTestUtils-ObjC-BridgingHeader.h */,
D80C990A2B0DFE410052F311 /* ExternalUIViewController.swift */,
62C25C852B075F4900C68CBD /* TestOptions.swift */,
);
path = SentryTestUtils;
Expand Down Expand Up @@ -4523,6 +4526,7 @@
84AC61DB29F7654A009EEF61 /* TestDispatchSourceWrapper.swift in Sources */,
8431F01729B2851500D8DC56 /* TestSentrySystemWrapper.swift in Sources */,
84281C632A579D0700EE88F2 /* SentryProfilerMocks.mm in Sources */,
D80C990B2B0DFE410052F311 /* ExternalUIViewController.swift in Sources */,
84B7FA4129B28CD200AD93B1 /* TestSentryDispatchQueueWrapper.swift in Sources */,
84B7FA3E29B28ADD00AD93B1 /* TestClient.swift in Sources */,
);
Expand Down Expand Up @@ -5261,7 +5265,7 @@
DEVELOPMENT_TEAM = "";
EXECUTABLE_PREFIX = lib;
GCC_C_LANGUAGE_STANDARD = gnu99;
MACH_O_TYPE = staticlib;
MACH_O_TYPE = mh_dylib;
brustolin marked this conversation as resolved.
Show resolved Hide resolved
MTL_ENABLE_DEBUG_INFO = YES;
MTL_FAST_MATH = YES;
PRODUCT_BUNDLE_IDENTIFIER = io.sentry.SentryTestUtils;
Expand Down Expand Up @@ -5415,7 +5419,7 @@
DEVELOPMENT_TEAM = "";
EXECUTABLE_PREFIX = lib;
GCC_C_LANGUAGE_STANDARD = gnu99;
MACH_O_TYPE = staticlib;
MACH_O_TYPE = mh_dylib;
MTL_ENABLE_DEBUG_INFO = YES;
MTL_FAST_MATH = YES;
PRODUCT_BUNDLE_IDENTIFIER = io.sentry.SentryTestUtils;
Expand Down Expand Up @@ -5446,7 +5450,7 @@
DEVELOPMENT_TEAM = "";
EXECUTABLE_PREFIX = lib;
GCC_C_LANGUAGE_STANDARD = gnu99;
MACH_O_TYPE = staticlib;
MACH_O_TYPE = mh_dylib;
MTL_FAST_MATH = YES;
PRODUCT_BUNDLE_IDENTIFIER = io.sentry.SentryTestUtils;
PRODUCT_NAME = "$(TARGET_NAME)";
Expand Down Expand Up @@ -5476,7 +5480,7 @@
DEVELOPMENT_TEAM = "";
EXECUTABLE_PREFIX = lib;
GCC_C_LANGUAGE_STANDARD = gnu99;
MACH_O_TYPE = staticlib;
MACH_O_TYPE = mh_dylib;
MTL_FAST_MATH = YES;
PRODUCT_BUNDLE_IDENTIFIER = io.sentry.SentryTestUtils;
PRODUCT_NAME = "$(TARGET_NAME)";
Expand Down Expand Up @@ -5506,7 +5510,7 @@
DEVELOPMENT_TEAM = "";
EXECUTABLE_PREFIX = lib;
GCC_C_LANGUAGE_STANDARD = gnu99;
MACH_O_TYPE = staticlib;
MACH_O_TYPE = mh_dylib;
MTL_FAST_MATH = YES;
PRODUCT_BUNDLE_IDENTIFIER = io.sentry.SentryTestUtils;
PRODUCT_NAME = "$(TARGET_NAME)";
Expand Down Expand Up @@ -5768,7 +5772,7 @@
DEVELOPMENT_TEAM = "";
EXECUTABLE_PREFIX = lib;
GCC_C_LANGUAGE_STANDARD = gnu99;
MACH_O_TYPE = staticlib;
MACH_O_TYPE = mh_dylib;
MTL_FAST_MATH = YES;
PRODUCT_BUNDLE_IDENTIFIER = io.sentry.SentryTestUtils;
PRODUCT_NAME = "$(TARGET_NAME)";
Expand Down
10 changes: 10 additions & 0 deletions SentryTestUtils/ExternalUIViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import Foundation

#if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)
import UIKit

//This class is used to test swizzling of view controllers in external libs
brustolin marked this conversation as resolved.
Show resolved Hide resolved
public class ExternalUIViewController: UIViewController {
}

#endif
13 changes: 13 additions & 0 deletions Sources/Sentry/SentryBinaryImageCache.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,19 @@ - (NSInteger)indexOfImage:(uint64_t)address
return -1; // Address not found
}

- (nullable NSString *)pathForImage:(NSString *)binaryName
philipphofmann marked this conversation as resolved.
Show resolved Hide resolved
{
@synchronized(self) {
for (SentryBinaryImageInfo *info in _cache) {
philipphofmann marked this conversation as resolved.
Show resolved Hide resolved
if ([[info.name.lastPathComponent lowercaseString]
isEqualToString:binaryName.lowercaseString]) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

h: The inAppInclude is a prefix. We must change this to using hasPrefix.

Suggested change
isEqualToString:binaryName.lowercaseString]) {
hasPrefix:binaryName.lowercaseString]) {

The InAppLogic does the same see

sentry-cocoa/Sources/Sentry/SentryInAppLogic.m

Line 35 in b064665

if ([imageName.lastPathComponent.lowercaseString hasPrefix:inAppInclude.lowercaseString])

I think it's a good idea to refactor the code so both this code here and the InAppLogic use the same code so we only have to change it once.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't notice this behaviour of inAppLogic, but I have a problem with it.

If I add Awesome to my inAppLogic it will match the Awesome lib and also Core-Awesome Data-Awesome and Private-Awesome.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it won't. The logic uses hasPrefix on the lastPathComponent. If you add Awesome to your inAppLogic Core-Awesome, Data-Awesome, and Private-Awesome won't be inApp, while Awesome-Core would be. I think we should stick to this logic.

return info.name;
}
}
}
brustolin marked this conversation as resolved.
Show resolved Hide resolved
return nil;
}

@end

static void
Expand Down
8 changes: 0 additions & 8 deletions Sources/Sentry/SentryInAppLogic.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@

NS_ASSUME_NONNULL_BEGIN

@interface
SentryInAppLogic ()

@property (nonatomic, copy, readonly) NSArray<NSString *> *inAppIncludes;
@property (nonatomic, copy, readonly) NSArray<NSString *> *inAppExcludes;

@end

@implementation SentryInAppLogic

- (instancetype)initWithInAppIncludes:(NSArray<NSString *> *)inAppIncludes
Expand Down
3 changes: 2 additions & 1 deletion Sources/Sentry/SentrySDK.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,11 @@ + (void)startWithOptions:(SentryOptions *)options
SENTRY_LOG_DEBUG(@"Dispatching init work required to run on main thread.");
[SentryThreadWrapper onMainThread:^{
SENTRY_LOG_DEBUG(@"SDK main thread init started...");
[SentrySDK installIntegrations];

[SentryCrashWrapper.sharedInstance startBinaryImageCache];
[SentryDependencyContainer.sharedInstance.binaryImageCache start];

[SentrySDK installIntegrations];
brustolin marked this conversation as resolved.
Show resolved Hide resolved
#if TARGET_OS_IOS && SENTRY_HAS_UIKIT
[SentryDependencyContainer.sharedInstance.uiDeviceWrapper start];
#endif // TARGET_OS_IOS && SENTRY_HAS_UIKIT
Expand Down
24 changes: 11 additions & 13 deletions Sources/Sentry/SentryUIViewControllerSwizzling.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

#if SENTRY_HAS_UIKIT

# import "SentryBinaryImageCache.h"
# import "SentryDefaultObjCRuntimeWrapper.h"
# import "SentryDefines.h"
# import "SentryDependencyContainer.h"
# import "SentryLog.h"
# import "SentryNSProcessInfoWrapper.h"
# import "SentrySubClassFinder.h"
Expand Down Expand Up @@ -69,6 +71,15 @@ - (instancetype)initWithOptions:(SentryOptions *)options

- (void)start
{
SentryBinaryImageCache *imageCache = SentryDependencyContainer.sharedInstance.binaryImageCache;
brustolin marked this conversation as resolved.
Show resolved Hide resolved

for (NSString *string in self.inAppLogic.inAppIncludes) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

h: Just double-checking why this works now: Previously, we only looked for UIViewControllers in the image of the app delegate class. Now we use inAppIncludes which per default includes the CFBundleExecutable which is basically the same image of the app delegate class. Could it happen that the UIApplication returned by findApp is not in the CFBundleExecutable? If yes, this would be a breaking change and we still should call swizzleAllSubViewControllersInApp after swizzling the inApp ones. There is no risk of double swizzling because if we already swizzled the UIViewController again swizzling it does nothing if I'm not mistaken.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brustolin, please answer the question above ⬆️.

NSString *pathToImage = [imageCache pathForImage:string];
if (pathToImage != nil) {
[self swizzleUIViewControllersOfImage:pathToImage];
}
brustolin marked this conversation as resolved.
Show resolved Hide resolved
}

id<SentryUIApplication> app = [self findApp];
if (app != nil) {

Expand All @@ -95,19 +106,6 @@ - (void)start
@"a rootViewController");
}
}

[self swizzleAllSubViewControllersInApp:app];
brustolin marked this conversation as resolved.
Show resolved Hide resolved
} else {
// If we can't find an UIApplication instance we may use the current process path as the
// image name. This mostly happens with SwiftUI projects.
NSString *processImage = self.processInfoWrapper.processPath;
philipphofmann marked this conversation as resolved.
Show resolved Hide resolved
if (processImage) {
[self swizzleUIViewControllersOfImage:processImage];
} else {
SENTRY_LOG_DEBUG(
@"UIViewControllerSwizzling: Did not found image name from current process. "
@"Skipping Swizzling of view controllers");
}
}

[self swizzleUIViewController];
Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/include/HybridPublic/SentryBinaryImageCache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ NS_ASSUME_NONNULL_BEGIN

- (nullable SentryBinaryImageInfo *)imageByAddress:(const uint64_t)address;

- (nullable NSString *)pathForImage:(NSString *)binaryName;

@end

NS_ASSUME_NONNULL_END
4 changes: 4 additions & 0 deletions Sources/Sentry/include/SentryInAppLogic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ NS_ASSUME_NONNULL_BEGIN
@interface SentryInAppLogic : NSObject
SENTRY_NO_INIT

@property (nonnull, readonly) NSArray<NSString *> *inAppIncludes;

@property (nonnull, readonly) NSArray<NSString *> *inAppExcludes;

/**
* Initializes @c SentryInAppLogic with @c inAppIncludes and @c inAppExcludes.
*
Expand Down
38 changes: 38 additions & 0 deletions ...ests/Integrations/Performance/UIViewController/SentryUIViewControllerSwizzlingTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,17 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase {

var options: Options {
let options = Options.noIntegrations()

let imageName = String(
cString: class_getImageName(SentryUIViewControllerSwizzlingTests.self)!,
encoding: .utf8)! as NSString
options.add(inAppInclude: imageName.lastPathComponent)

let externalImageName = String(
cString: class_getImageName(ExternalUIViewController.self)!,
encoding: .utf8)! as NSString
options.add(inAppInclude: externalImageName.lastPathComponent)

return options
}

Expand Down Expand Up @@ -56,6 +63,20 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase {
super.tearDown()
clearTestState()
}

func testExternalViewControllerImage() {
//Test to ensure ExternalUIViewController exists in an external lib
brustolin marked this conversation as resolved.
Show resolved Hide resolved
//just in case someone changes the settings of the `SentryTestUtils` lib
let imageName = String(
cString: class_getImageName(SentryUIViewControllerSwizzlingTests.self)!,
encoding: .utf8)! as NSString

let externalImageName = String(
cString: class_getImageName(ExternalUIViewController.self)!,
encoding: .utf8)! as NSString

XCTAssertNotEqual(externalImageName, imageName)
brustolin marked this conversation as resolved.
Show resolved Hide resolved
}

func testShouldSwizzle_TestViewController() {
let result = fixture.sut.shouldSwizzleViewController(TestViewController.self)
Expand Down Expand Up @@ -85,7 +106,16 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase {
fixture.sut.start()
let controller = TestViewController()
controller.loadView()
let span = SentrySDK.span

//To finish the transaction we need to finish `initialDisplay` span
//by calling `viewWillAppear` and reporting a new frame
controller.viewWillAppear(false)
Dynamic(SentryDependencyContainer.sharedInstance().framesTracker).reportNewFrame()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this to cause SentryTimeToDisplayTracker.framesTrackerHasNewFrame to be called, ending the span it's managing? If so, please mention it here, or whatever other side effect is desired.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not call SentryTimeToDisplayTracker.framesTrackerHasNewFrame it calls SentryTimeToDisplayTracker.reportNewFrame which cause SentryTimeToDisplayTracker to report a new frame to all its delegate. This desired side effect is described in the comment above.

Copy link
Member

@armcknight armcknight Nov 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the reason I asked about SentryTimeToDisplayTracker.framesTrackerHasNewFrame is because when I looked at SentryTimeToDisplayTracker.reportNewFrame and clicked through to the delegate call, the only option that comes up is SentryTimeToDisplayTracker.framesTrackerHasNewFrame:
image

So either we're both right or there's something else going on? Please let us know the mechanism.

Copy link
Contributor Author

@brustolin brustolin Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right, I misread the class name you wrote. reportNewFrame is a function of SentryFramesTracker that will call framesTrackerHasNewFrame from its delegates.

brustolin marked this conversation as resolved.
Show resolved Hide resolved

XCTAssertNotNil(SentrySDK.span)
controller.viewDidAppear(false)
XCTAssertTrue(span?.isFinished == true)
}

func testViewControllerWithLoadView_TransactionBoundToScope() {
Expand Down Expand Up @@ -123,6 +153,14 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase {
}
}

func testSwizzlingOfExternalLibs() {
brustolin marked this conversation as resolved.
Show resolved Hide resolved
let sut = fixture.sut
sut.start()
let controller = ExternalUIViewController()
controller.loadView()
XCTAssertNotNil(SentrySDK.span)
}

func testSwizzle_fromScene_invalidNotification_NoObject() {
let swizzler = fixture.testableSut

Expand Down
Loading