From e1a9087750d969a6ec5124fa8fce1184b04181bc Mon Sep 17 00:00:00 2001 From: Jens Ayton Date: Mon, 14 Dec 2020 16:32:11 +0100 Subject: [PATCH 1/4] Test that withSpan() calls end() as expected --- Tests/TracingTests/TestTracer.swift | 6 ++++-- Tests/TracingTests/TracerTests.swift | 8 ++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Tests/TracingTests/TestTracer.swift b/Tests/TracingTests/TestTracer.swift index 9417c2ff..34749296 100644 --- a/Tests/TracingTests/TestTracer.swift +++ b/Tests/TracingTests/TestTracer.swift @@ -19,6 +19,7 @@ import Tracing final class TestTracer: Tracer { private(set) var spans = [TestSpan]() + var onEndSpan: (Span) -> Void = { _ in } func startSpan( _ operationName: String, @@ -30,8 +31,9 @@ final class TestTracer: Tracer { operationName: operationName, startTime: time, baggage: baggage, - kind: kind - ) { _ in } + kind: kind, + onEnd: onEndSpan + ) self.spans.append(span) return span } diff --git a/Tests/TracingTests/TracerTests.swift b/Tests/TracingTests/TracerTests.swift index d75d949d..e0caf456 100644 --- a/Tests/TracingTests/TracerTests.swift +++ b/Tests/TracingTests/TracerTests.swift @@ -63,11 +63,15 @@ final class TracerTests: XCTestCase { InstrumentationSystem.bootstrapInternal(NoOpTracer()) } + var spanEnded = false + tracer.onEndSpan = { _ in spanEnded = true } + let value = tracer.withSpan("hello", baggage: .topLevel) { _ in "yes" } XCTAssertEqual(value, "yes") + // XCTAssertEqual(spanEnded, true) } func testWithSpan_throws() { @@ -77,11 +81,15 @@ final class TracerTests: XCTestCase { InstrumentationSystem.bootstrapInternal(NoOpTracer()) } + var spanEnded = false + tracer.onEndSpan = { _ in spanEnded = true } + do { _ = try tracer.withSpan("hello", baggage: .topLevel) { _ in throw ExampleSpanError() } } catch { + XCTAssertEqual(spanEnded, true) XCTAssertEqual(error as? ExampleSpanError, ExampleSpanError()) return } From 44dbf9ba8501ac6f68557f31e9aa2ba566e92569 Mon Sep 17 00:00:00 2001 From: Jens Ayton Date: Mon, 14 Dec 2020 16:40:49 +0100 Subject: [PATCH 2/4] Call span.end() in the happy path in withSpan --- Sources/Tracing/Tracer.swift | 4 ++-- Tests/TracingTests/TracerTests.swift | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/Tracing/Tracer.swift b/Sources/Tracing/Tracer.swift index d6979936..21adec8d 100644 --- a/Sources/Tracing/Tracer.swift +++ b/Sources/Tracing/Tracer.swift @@ -98,11 +98,11 @@ extension Tracer { _ function: (Span) throws -> T ) rethrows -> T { let span = self.startSpan(operationName, context: context, ofKind: kind) + defer { span.end() } do { return try function(span) } catch { span.recordError(error) - span.end() throw error // rethrow } } @@ -125,11 +125,11 @@ extension Tracer { _ function: (Span) throws -> T ) rethrows -> T { let span = self.startSpan(operationName, baggage: baggage, ofKind: kind) + defer { span.end() } do { return try function(span) } catch { span.recordError(error) - span.end() throw error // rethrow } } diff --git a/Tests/TracingTests/TracerTests.swift b/Tests/TracingTests/TracerTests.swift index e0caf456..15c44f94 100644 --- a/Tests/TracingTests/TracerTests.swift +++ b/Tests/TracingTests/TracerTests.swift @@ -71,7 +71,7 @@ final class TracerTests: XCTestCase { } XCTAssertEqual(value, "yes") - // XCTAssertEqual(spanEnded, true) + XCTAssertEqual(spanEnded, true) } func testWithSpan_throws() { From 58318a0f2987441d5e5a5ce208f54d8430df777b Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 15 Dec 2020 06:39:36 +0900 Subject: [PATCH 3/4] Apply suggestions from code review --- Tests/TracingTests/TracerTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/TracingTests/TracerTests.swift b/Tests/TracingTests/TracerTests.swift index 15c44f94..1e6f215c 100644 --- a/Tests/TracingTests/TracerTests.swift +++ b/Tests/TracingTests/TracerTests.swift @@ -71,7 +71,7 @@ final class TracerTests: XCTestCase { } XCTAssertEqual(value, "yes") - XCTAssertEqual(spanEnded, true) + XCTAssertTrue(spanEnded) } func testWithSpan_throws() { @@ -89,7 +89,7 @@ final class TracerTests: XCTestCase { throw ExampleSpanError() } } catch { - XCTAssertEqual(spanEnded, true) + XCTAssertTrue(spanEnded, true) XCTAssertEqual(error as? ExampleSpanError, ExampleSpanError()) return } From 01f7d7bf5f181d934137cfe826c76bda300ed840 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 15 Dec 2020 11:12:52 +0900 Subject: [PATCH 4/4] Update TracerTests.swift --- Tests/TracingTests/TracerTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/TracingTests/TracerTests.swift b/Tests/TracingTests/TracerTests.swift index 1e6f215c..54dd7792 100644 --- a/Tests/TracingTests/TracerTests.swift +++ b/Tests/TracingTests/TracerTests.swift @@ -89,7 +89,7 @@ final class TracerTests: XCTestCase { throw ExampleSpanError() } } catch { - XCTAssertTrue(spanEnded, true) + XCTAssertTrue(spanEnded) XCTAssertEqual(error as? ExampleSpanError, ExampleSpanError()) return }