Skip to content

Commit 404a199

Browse files
committed
review-feedback: Remove redundant and possibly flakey tests
#7584 (comment) test_dataTaskWithURL_doesNotCallThrough_dataTaskWithRequest test_dataTaskWithRequest_doesNotCallThrough_dataTaskWithURL were asserting that one impl does not rely on the other internally. There's no reason to believe that they do - the test was probably checking for something that would not happen. Additionally it was relying on method_setImplementation which could cause random failures in CI if not cleaned up properly => remove
1 parent c1ab58f commit 404a199

1 file changed

Lines changed: 3 additions & 79 deletions

File tree

Tests/SentryTests/Integrations/Performance/Network/SentryNSURLSessionTaskSearchTests.swift

Lines changed: 3 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -17,97 +17,21 @@ class SentryNSURLSessionTaskSearchTests: XCTestCase {
1717
//
1818
// The swizzling code relies on this by swizzling [NSURLSession class] directly
1919
// rather than doing runtime discovery. These tests verify that assumption
20-
// still holds — if Apple ever moves these methods to a subclass, these tests
20+
// still holds — if Apple ever moves these methods, these tests
2121
// will fail and we'll know to update the swizzling approach.
2222

23-
func test_URLSession_isNotClassCluster_dataTaskWithRequest() {
23+
func test_URLSessionDataTaskWithRequest_ByIosVersion() {
2424
let selector = #selector(URLSession.dataTask(with:completionHandler:)
2525
as (URLSession) -> (URLRequest, @escaping @Sendable (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask)
2626
assertNSURLSessionImplementsDirectly(selector: selector, selectorName: "dataTaskWithRequest:completionHandler:")
2727
}
2828

29-
func test_URLSession_isNotClassCluster_dataTaskWithURL() {
29+
func test_URLSessionDataTaskWithURL_ByIosVersion() {
3030
let selector = #selector(URLSession.dataTask(with:completionHandler:)
3131
as (URLSession) -> (URL, @escaping @Sendable (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask)
3232
assertNSURLSessionImplementsDirectly(selector: selector, selectorName: "dataTaskWithURL:completionHandler:")
3333
}
3434

35-
// MARK: - dataTaskWithURL: / dataTaskWithRequest: independence
36-
//
37-
// We swizzle both dataTaskWithRequest:completionHandler: and
38-
// dataTaskWithURL:completionHandler: because they are independent
39-
// implementations — dataTaskWithURL: does NOT dispatch to
40-
// dataTaskWithRequest: via objc_msgSend.
41-
//
42-
// If this test ever fails, Apple has changed the internal dispatch so
43-
// one calls through to the other. In that case, remove the redundant
44-
// swizzle and add a deduplication guard to avoid double-capture.
45-
46-
func test_dataTaskWithURL_doesNotCallThrough_dataTaskWithRequest() {
47-
assertNoCallThrough(
48-
from: NSSelectorFromString("dataTaskWithURL:completionHandler:"),
49-
to: NSSelectorFromString("dataTaskWithRequest:completionHandler:"),
50-
call: { session in
51-
let url = URL(string: "https://example.com")!
52-
let task = session.dataTask(with: url) { _, _, _ in }
53-
task.cancel()
54-
}
55-
)
56-
}
57-
58-
func test_dataTaskWithRequest_doesNotCallThrough_dataTaskWithURL() {
59-
assertNoCallThrough(
60-
from: NSSelectorFromString("dataTaskWithRequest:completionHandler:"),
61-
to: NSSelectorFromString("dataTaskWithURL:completionHandler:"),
62-
call: { session in
63-
let request = URLRequest(url: URL(string: "https://example.com")!)
64-
let task = session.dataTask(with: request) { _, _, _ in }
65-
task.cancel()
66-
}
67-
)
68-
}
69-
70-
/// Temporarily replaces the IMP of `targetSelector` with one that increments
71-
/// a counter, then invokes `call` (which should trigger `sourceSelector`).
72-
/// Asserts the counter stays at 0 — meaning `sourceSelector` does not
73-
/// internally dispatch to `targetSelector` via objc_msgSend.
74-
private func assertNoCallThrough(
75-
from sourceSelector: Selector,
76-
to targetSelector: Selector,
77-
call: (URLSession) -> Void
78-
) {
79-
guard let method = class_getInstanceMethod(URLSession.self, targetSelector) else {
80-
XCTFail("URLSession should implement \(targetSelector)")
81-
return
82-
}
83-
84-
let originalIMP = method_getImplementation(method)
85-
defer { method_setImplementation(method, originalIMP) }
86-
87-
var hitCount = 0
88-
89-
let replacementBlock: @convention(block) (NSObject, AnyObject, Any?) -> AnyObject = { obj, arg, handler in
90-
hitCount += 1
91-
typealias Fn = @convention(c) (NSObject, Selector, AnyObject, Any?) -> AnyObject
92-
let original = unsafeBitCast(originalIMP, to: Fn.self)
93-
return original(obj, targetSelector, arg, handler)
94-
}
95-
96-
method_setImplementation(method, imp_implementationWithBlock(replacementBlock))
97-
98-
let session = URLSession(configuration: .ephemeral)
99-
defer { session.invalidateAndCancel() }
100-
101-
call(session)
102-
103-
XCTAssertEqual(
104-
hitCount, 0,
105-
"\(sourceSelector) called through to \(targetSelector). "
106-
+ "These methods are no longer independent — remove the redundant swizzle "
107-
+ "in SentrySwizzleWrapperHelper and add a deduplication guard."
108-
)
109-
}
110-
11135
// MARK: - Helper
11236

11337
/// Walks the class hierarchy for sessions created with default and ephemeral

0 commit comments

Comments
 (0)