Clean up iOS Dependency - #3
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the Package.swift configuration to include broader platform support and simplifies the target structure. However, the complete removal of the test suite and the test file is problematic as it eliminates automated validation for C++ interoperability. The reviewer suggests retaining a simplified version of the SwiftCompatibilityTests that does not depend on external OpenTelemetry packages, and recommends explicitly defining the publicHeadersPath to ensure consistent header resolution.
I am having trouble creating individual review comments. Click here to see my feedback.
Package.swift (56-75)
Completely removing all tests leaves the package with no automated validation. We can retain the SwiftCompatibilityTests target by removing the opentelemetry-swift-core dependency and keeping a simplified test suite. This ensures that C++ interoperability and basic compilation/linking continue to be validated automatically in CI.
.testTarget(
name: "SwiftCompatibilityTests",
dependencies: [
"FirebaseTelemetryPersistence",
],
path: ".",
exclude: [
"CMakeLists.txt",
"README.md",
"LICENSE",
"src",
"include",
],
sources: ["tests/swift_compatibility_test.swift"],
swiftSettings: [
.interoperabilityMode(.Cxx)
]
),tests/swift_compatibility_test.swift (1-95)
Instead of deleting the compatibility tests entirely, we should keep a simplified version of this file that does not depend on opentelemetry-swift-core. This ensures we still have automated test coverage validating that the C++ types are correctly exposed and usable via Swift's C++ interop.
// Copyright 2026 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
import CxxStdlib
import Foundation
import Testing
@testable import FirebaseTelemetryPersistence
/// The native C++ span structure used for cross-language interoperability.
typealias CppSpan = firebase.telemetry.persistence.Span
/// The native C++ span TraceId.
typealias CppTraceId = firebase.telemetry.persistence.TraceId
struct SwiftCompatibilityTests {
@Test
func testInit() async throws {
let span = CppSpan(
CppTraceId(high: 123, low: 456),
789,
101,
1000,
2000,
std.string("testSpan"),
firebase.telemetry.persistence.AttributesList()
)
#expect(span.trace_id().high == 123)
#expect(span.trace_id().low == 456)
#expect(span.span_id() == 789)
#expect(span.parent_span_id() == 101)
#expect(span.start_time() == 1000)
#expect(span.end_time() == 2000)
#expect(span.name() == "testSpan")
}
}Package.swift (41)
Explicitly specifying publicHeadersPath: "include" is highly recommended when the target path is customized to the root directory (.). This avoids ambiguity and ensures that Swift Package Manager and Xcode correctly locate the public headers across different toolchain versions.
sources: ["src"],
publicHeadersPath: "include"
Swift Interop tests are no longer required as the interop is done through ObjC, so will not be taking this suggestion. Applied the |
opentelemetry-swift-coreopentelemetry-swift-corefirebase-ios-sdk