From 5790ee86a41009afe64bd5166e620b6d14805c65 Mon Sep 17 00:00:00 2001 From: Marie Denis Date: Fri, 15 Nov 2024 15:46:58 +0100 Subject: [PATCH 1/2] RUM-7285 Add api-surface step to Lint stage --- .gitlab-ci.yml | 11 +++++++++++ Makefile | 14 ++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 9205d662c5..e893079dcf 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -91,6 +91,17 @@ Lint: - make clean repo-setup ENV=ci - make lint license-check - make rum-models-verify sr-models-verify + - make api-surface ENV=ci + - if ! diff api-surface-swift api-surface-swift-generated; then + echo "❌ Swift API surface mismatch"; + echo "👉 Run \`make api-surface\` locally to update \`api-surface-swift\` and commit the changes."; + exit 1; + fi + - if ! diff api-surface-objc api-surface-objc-generated; then + echo "❌ Objective-C API surface mismatch"; + echo "👉 Run \`make api-surface\` locally to update \`api-surface-objc\` and commit the changes."; + exit 1; + fi Unit Tests (iOS): stage: test diff --git a/Makefile b/Makefile index 3e33ec37b9..d26eabdf56 100644 --- a/Makefile +++ b/Makefile @@ -356,6 +356,16 @@ sr-snapshot-tests-open: @$(ECHO_TITLE) "make sr-snapshot-tests-open" ./tools/sr-snapshot-test.sh --open-project +# Define default paths for API output files +SWIFT_OUTPUT_PATH ?= api-surface-swift +OBJC_OUTPUT_PATH ?= api-surface-objc + +# Use different paths when running in CI +ifeq ($(ENV),ci) + SWIFT_OUTPUT_PATH := api-surface-swift-generated + OBJC_OUTPUT_PATH := api-surface-objc-generated +endif + # Generate api-surface files for Datadog and DatadogObjc. api-surface: @echo "Generating api-surface-swift" @@ -369,7 +379,7 @@ api-surface: --library-name DatadogCrashReporting \ --library-name DatadogWebViewTracking \ --library-name DatadogSessionReplay \ - > ../../api-surface-swift && \ + > ../../$(SWIFT_OUTPUT_PATH) && \ cd - @echo "Generating api-surface-objc" @@ -377,7 +387,7 @@ api-surface: swift run api-surface spm \ --path ../../ \ --library-name DatadogObjc \ - > ../../api-surface-objc && \ + > ../../$(OBJC_OUTPUT_PATH) && \ cd - # Generate Datadog monitors terraform definition for E2E tests: From 4aa62a91d504f1c3f84125415391ddfd1dad0daa Mon Sep 17 00:00:00 2001 From: Marie Denis Date: Thu, 5 Dec 2024 14:10:10 +0100 Subject: [PATCH 2/2] RUM-7285 Address CR + split commands + add tests --- .gitlab-ci.yml | 12 +- .../Sources/SessionReplayConfiguration.swift | 4 +- Makefile | 114 +++++++------- api-surface-swift | 16 +- .../Sources/APISurfaceCore/Commands.swift | 105 +++++++++++-- .../Sources/api-surface/main.swift | 9 +- .../IntegrationTests.swift | 140 ++++++++++++++---- 7 files changed, 287 insertions(+), 113 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e893079dcf..ed7bb799a6 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -91,17 +91,7 @@ Lint: - make clean repo-setup ENV=ci - make lint license-check - make rum-models-verify sr-models-verify - - make api-surface ENV=ci - - if ! diff api-surface-swift api-surface-swift-generated; then - echo "❌ Swift API surface mismatch"; - echo "👉 Run \`make api-surface\` locally to update \`api-surface-swift\` and commit the changes."; - exit 1; - fi - - if ! diff api-surface-objc api-surface-objc-generated; then - echo "❌ Objective-C API surface mismatch"; - echo "👉 Run \`make api-surface\` locally to update \`api-surface-objc\` and commit the changes."; - exit 1; - fi + - make api-surface-verify Unit Tests (iOS): stage: test diff --git a/DatadogSessionReplay/Sources/SessionReplayConfiguration.swift b/DatadogSessionReplay/Sources/SessionReplayConfiguration.swift index 5bd43d9d08..6dd22329cc 100644 --- a/DatadogSessionReplay/Sources/SessionReplayConfiguration.swift +++ b/DatadogSessionReplay/Sources/SessionReplayConfiguration.swift @@ -85,7 +85,8 @@ extension SessionReplay { /// - startRecordingImmediately: If the recording should start automatically. When `true`, the recording starts automatically; when `false` it doesn't, and the recording will need to be started manually. Default: `true`. /// - customEndpoint: Custom server url for sending replay data. Default: `nil`. /// - featureFlags: Experimental feature flags. - public init( // swiftlint:disable:this function_default_parameter_at_end + // swiftlint:disable function_default_parameter_at_end + public init( replaySampleRate: Float = 100, textAndInputPrivacyLevel: TextAndInputPrivacyLevel, imagePrivacyLevel: ImagePrivacyLevel, @@ -102,6 +103,7 @@ extension SessionReplay { self.customEndpoint = customEndpoint self.featureFlags = featureFlags } + // swiftlint:enable function_default_parameter_at_end /// Creates Session Replay configuration. /// - Parameters: diff --git a/Makefile b/Makefile index d26eabdf56..30a746f81f 100644 --- a/Makefile +++ b/Makefile @@ -58,18 +58,18 @@ export DD_SDK_DATADOG_XCCONFIG_CI # Installs tools and dependencies with homebrew. # Do not call 'brew update' and instead let Bitrise use its own brew bottle mirror. dependencies: - @echo "⚙️ Installing dependencies..." - @bundle install - @brew list swiftlint &>/dev/null || brew install swiftlint - @brew upgrade carthage - @carthage bootstrap --platform iOS,tvOS --use-xcframeworks - @echo $$DD_SDK_BASE_XCCONFIG > xcconfigs/Base.local.xcconfig; - @brew list gh &>/dev/null || brew install gh + @echo "⚙️ Installing dependencies..." + @bundle install + @brew list swiftlint &>/dev/null || brew install swiftlint + @brew upgrade carthage + @carthage bootstrap --platform iOS,tvOS --use-xcframeworks + @echo $$DD_SDK_BASE_XCCONFIG > xcconfigs/Base.local.xcconfig; + @brew list gh &>/dev/null || brew install gh ifeq (${ci}, true) - @echo $$DD_SDK_BASE_XCCONFIG_CI >> xcconfigs/Base.local.xcconfig; - @echo $$DD_SDK_DATADOG_XCCONFIG_CI > xcconfigs/Datadog.local.xcconfig; + @echo $$DD_SDK_BASE_XCCONFIG_CI >> xcconfigs/Base.local.xcconfig; + @echo $$DD_SDK_DATADOG_XCCONFIG_CI > xcconfigs/Datadog.local.xcconfig; ifndef DD_DISABLE_TEST_INSTRUMENTING - @echo $$DD_SDK_TESTING_XCCONFIG_CI > xcconfigs/DatadogSDKTesting.local.xcconfig; + @echo $$DD_SDK_TESTING_XCCONFIG_CI > xcconfigs/DatadogSDKTesting.local.xcconfig; endif endif @@ -356,50 +356,64 @@ sr-snapshot-tests-open: @$(ECHO_TITLE) "make sr-snapshot-tests-open" ./tools/sr-snapshot-test.sh --open-project -# Define default paths for API output files -SWIFT_OUTPUT_PATH ?= api-surface-swift -OBJC_OUTPUT_PATH ?= api-surface-objc - -# Use different paths when running in CI -ifeq ($(ENV),ci) - SWIFT_OUTPUT_PATH := api-surface-swift-generated - OBJC_OUTPUT_PATH := api-surface-objc-generated -endif - -# Generate api-surface files for Datadog and DatadogObjc. +# Generate api-surface files for Datadog and DatadogObjc api-surface: - @echo "Generating api-surface-swift" - @cd tools/api-surface && \ - swift run api-surface spm \ - --path ../../ \ - --library-name DatadogCore \ - --library-name DatadogLogs \ - --library-name DatadogTrace \ - --library-name DatadogRUM \ - --library-name DatadogCrashReporting \ - --library-name DatadogWebViewTracking \ - --library-name DatadogSessionReplay \ - > ../../$(SWIFT_OUTPUT_PATH) && \ - cd - - - @echo "Generating api-surface-objc" - @cd tools/api-surface && \ - swift run api-surface spm \ - --path ../../ \ - --library-name DatadogObjc \ - > ../../$(OBJC_OUTPUT_PATH) && \ - cd - + @$(ECHO_TITLE) "make api-surface" + @echo "Generating api-surface-swift" + @cd tools/api-surface && \ + swift run api-surface generate \ + --path ../../ \ + --library-name DatadogCore \ + --library-name DatadogLogs \ + --library-name DatadogTrace \ + --library-name DatadogRUM \ + --library-name DatadogCrashReporting \ + --library-name DatadogWebViewTracking \ + --library-name DatadogSessionReplay \ + --output-file ../../api-surface-swift + + @echo "Generating api-surface-objc" + @cd tools/api-surface && \ + swift run api-surface generate \ + --path ../../ \ + --library-name DatadogObjc \ + --output-file ../../api-surface-objc + +# Verify API surface files for Datadog and DatadogObjc +api-surface-verify: + @$(ECHO_TITLE) "make api-surface-verify" + @echo "Verifying api-surface-swift" + @cd tools/api-surface && \ + swift run api-surface verify \ + --path ../../ \ + --library-name DatadogCore \ + --library-name DatadogLogs \ + --library-name DatadogTrace \ + --library-name DatadogRUM \ + --library-name DatadogCrashReporting \ + --library-name DatadogWebViewTracking \ + --library-name DatadogSessionReplay \ + --output-file /tmp/api-surface-swift-generated \ + ../../api-surface-swift + + @echo "Verifying api-surface-objc" + @cd tools/api-surface && \ + swift run api-surface verify \ + --path ../../ \ + --library-name DatadogObjc \ + --output-file /tmp/api-surface-objc-generated \ + ../../api-surface-objc # Generate Datadog monitors terraform definition for E2E tests: e2e-monitors-generate: - @echo "Deleting previous 'main.tf as it will be soon generated." - @rm -f tools/nightly-e2e-tests/monitors-gen/main.tf - @echo "Deleting previous Terraform state and backup as we don't need to track it." - @rm -f tools/nightly-e2e-tests/monitors-gen/terraform.tfstate - @rm -f tools/nightly-e2e-tests/monitors-gen/terraform.tfstate.backup - @echo "⚙️ Generating 'main.tf':" - @./tools/nightly-e2e-tests/nightly_e2e.py generate-tf --tests-dir ../../Datadog/E2ETests - @echo "⚠️ Remember to delete all iOS monitors manually from Mobile-Integration org before running 'terraform apply'." + @echo "Deleting previous 'main.tf as it will be soon generated." + @rm -f tools/nightly-e2e-tests/monitors-gen/main.tf + @echo "Deleting previous Terraform state and backup as we don't need to track it." + @rm -f tools/nightly-e2e-tests/monitors-gen/terraform.tfstate + @rm -f tools/nightly-e2e-tests/monitors-gen/terraform.tfstate.backup + @echo "⚙️ Generating 'main.tf':" + @./tools/nightly-e2e-tests/nightly_e2e.py generate-tf --tests-dir ../../Datadog/E2ETests + @echo "⚠️ Remember to delete all iOS monitors manually from Mobile-Integration org before running 'terraform apply'." # Creates dogfooding PR in shopist-ios dogfood-shopist: diff --git a/api-surface-swift b/api-surface-swift index 7f2d008e04..e3313af1b4 100644 --- a/api-surface-swift +++ b/api-surface-swift @@ -53,7 +53,6 @@ public enum Datadog public static func stopInstance(named instanceName: String = CoreRegistry.defaultInstanceName) public static func initialize(with configuration: Configuration,trackingConsent: TrackingConsent,instanceName: String = CoreRegistry.defaultInstanceName) -> DatadogCoreProtocol - # ---------------------------------- # API surface for DatadogLogs: # ---------------------------------- @@ -182,7 +181,6 @@ public enum Logs public protocol LogEventMapper func map(event: LogEvent, callback: @escaping (LogEvent) -> Void) - # ---------------------------------- # API surface for DatadogTrace: # ---------------------------------- @@ -337,7 +335,6 @@ public enum SpanTags public class Tracer public static func shared(in core: DatadogCoreProtocol = CoreRegistry.default) -> OTTracer - # ---------------------------------- # API surface for DatadogRUM: # ---------------------------------- @@ -1790,7 +1787,6 @@ public enum PerformanceMetric case flutterRasterTime case jsFrameTimeSeconds - # ---------------------------------- # API surface for DatadogCrashReporting: # ---------------------------------- @@ -1799,7 +1795,6 @@ public final class CrashReporting public static func enable(in core: DatadogCoreProtocol = CoreRegistry.default) public static func enable() - # ---------------------------------- # API surface for DatadogWebViewTracking: # ---------------------------------- @@ -1815,7 +1810,6 @@ public enum WebViewTracking public func send(body: Any, slotId: String? = nil) public static func messageEmitter(in core: DatadogCoreProtocol,logsSampleRate: Float = 100) -> AbstractMessageEmitter - # ---------------------------------- # API surface for DatadogSessionReplay: # ---------------------------------- @@ -2222,9 +2216,17 @@ public enum SessionReplay public var touchPrivacyLevel: TouchPrivacyLevel public var startRecordingImmediately: Bool public var customEndpoint: URL? - public init( // swiftlint:disable:this function_default_parameter_at_endreplaySampleRate: Float = 100,textAndInputPrivacyLevel: TextAndInputPrivacyLevel,imagePrivacyLevel: ImagePrivacyLevel,touchPrivacyLevel: TouchPrivacyLevel,startRecordingImmediately: Bool = true,customEndpoint: URL? = nil) + public var featureFlags: FeatureFlags + public init(replaySampleRate: Float = 100,textAndInputPrivacyLevel: TextAndInputPrivacyLevel,imagePrivacyLevel: ImagePrivacyLevel,touchPrivacyLevel: TouchPrivacyLevel,startRecordingImmediately: Bool = true,customEndpoint: URL? = nil,featureFlags: FeatureFlags = .defaults) public init(replaySampleRate: Float = 100,defaultPrivacyLevel: SessionReplayPrivacyLevel = .mask,startRecordingImmediately: Bool = true,customEndpoint: URL? = nil) public mutating func setAdditionalNodeRecorders(_ additionalNodeRecorders: [SessionReplayNodeRecorder]) +[?] extension SessionReplay.Configuration + public typealias FeatureFlags = [FeatureFlag: Bool] + public enum FeatureFlag + case swiftui +[?] extension SessionReplay.Configuration.FeatureFlags + public static var defaults: Self + public subscript(flag: Key) -> Bool public enum objc_TextAndInputPrivacyLevelOverride: Int case none case maskSensitiveInputs diff --git a/tools/api-surface/Sources/APISurfaceCore/Commands.swift b/tools/api-surface/Sources/APISurfaceCore/Commands.swift index a447741acd..eb7042c501 100644 --- a/tools/api-surface/Sources/APISurfaceCore/Commands.swift +++ b/tools/api-surface/Sources/APISurfaceCore/Commands.swift @@ -7,12 +7,10 @@ import Foundation import ArgumentParser -public var printFunction: (String) -> Void = { print($0) } - -public struct SPMLibrarySurfaceCommand: ParsableCommand { +public struct GenerateCommand: ParsableCommand { public static let configuration = CommandConfiguration( - commandName: "spm", - abstract: "Prints API surface for given SPM library or list of libraries." + commandName: "generate", + abstract: "Generate API surface files for given SPM library or list of libraries." ) @Option(help: "Specify a library name (use this option multiple times to provide list of libraries).") @@ -21,23 +19,106 @@ public struct SPMLibrarySurfaceCommand: ParsableCommand { @Option(help: "The path to the folder containing `Package.swift`.") var path: String + @Option(help: "The file to which the generated API surface should be written.") + var outputFile: String + + public init() {} + + public func run() throws { + try generateAPISurface( + libraryName: libraryName, + path: path, + outputFile: outputFile + ) + } +} + +public struct VerifyCommand: ParsableCommand { + public static let configuration = CommandConfiguration( + commandName: "verify", + abstract: "Verify that a generated API surface matches the reference file." + ) + + @Option(help: "Specify a library name (use this option multiple times to provide a list of libraries).") + var libraryName: [String] + + @Option(help: "The path to the folder containing `Package.swift`.") + var path: String + + @Option(help: "The temporary file to which the generated API surface should be written.") + var outputFile: String + + @Argument(help: "Path to the reference API surface file to compare against.") + var referencePath: String + public init() {} public func run() throws { - var printSeparator = false - for libraryName in libraryName { - let surface = try APISurface(spmLibraryName: libraryName, inPath: path) + try generateAPISurface( + libraryName: libraryName, + path: path, + outputFile: outputFile + ) + + // Compare the generated files with the reference files + let diff = try compareFiles(reference: referencePath, generated: outputFile) + + if !diff.isEmpty { + throw ValidationError(""" + ❌ API surface mismatch detected! + Run `make api-surface` locally to update reference files and commit the changes. + """) + } + + print("✅ API surface files are up-to-date.") + } + + private func compareFiles(reference: String, generated: String) throws -> String { + let referenceContent = try String(contentsOfFile: reference) + let generatedContent = try String(contentsOfFile: generated) + + return referenceContent == generatedContent ? "" : "Difference in \(reference)" + } +} + +private func generateAPISurface( + libraryName: [String], + path: String, + outputFile: String +) throws { + var output = "" + var printSeparator = false + + for library in libraryName { + do { + let surface = try APISurface(spmLibraryName: library, inPath: path) if printSeparator { - printFunction("\n") + output.append("\n") } - printFunction(""" + + output.append(""" # ---------------------------------- - # API surface for \(libraryName): + # API surface for \(library): # ---------------------------------- """) - printFunction(try surface.print()) + + output.append("\n") + output.append(try surface.print() + "\n") + printSeparator = true + } catch { + print("❌ Error generating API surface for library \(library): \(error)") + throw error } } + + // Write the output to the specified file + do { + try output.write(toFile: outputFile, atomically: true, encoding: .utf8) + print("✅ API surface written to \(outputFile)") + } catch { + print("❌ Error writing API surface to \(outputFile): \(error)") + throw error + } } diff --git a/tools/api-surface/Sources/api-surface/main.swift b/tools/api-surface/Sources/api-surface/main.swift index d090d92156..bf1f818e0a 100644 --- a/tools/api-surface/Sources/api-surface/main.swift +++ b/tools/api-surface/Sources/api-surface/main.swift @@ -9,11 +9,12 @@ import APISurfaceCore private struct RootCommand: ParsableCommand { static let configuration = CommandConfiguration( - commandName: "api-surface", - abstract: "Prints API surface for given module.", + abstract: "A tool to manage API surface files.", subcommands: [ - SPMLibrarySurfaceCommand.self - ] + GenerateCommand.self, + VerifyCommand.self + ], + defaultSubcommand: GenerateCommand.self ) } diff --git a/tools/api-surface/Tests/APISurfaceCoreTests/IntegrationTests.swift b/tools/api-surface/Tests/APISurfaceCoreTests/IntegrationTests.swift index 89e6ea60c9..774434927a 100644 --- a/tools/api-surface/Tests/APISurfaceCoreTests/IntegrationTests.swift +++ b/tools/api-surface/Tests/APISurfaceCoreTests/IntegrationTests.swift @@ -6,71 +6,155 @@ import XCTest import APISurfaceCore +import ArgumentParser class IntegrationTests: XCTestCase { func testCreatingSurfaceForFixtureLibraries() throws { - var outputs: [String] = [] - APISurfaceCore.printFunction = { outputs.append($0) } - // Given - let command = try SPMLibrarySurfaceCommand.parse([ + let temporaryFile = "/tmp/api-surface-test-output" + let command = try GenerateCommand.parse([ "--library-name", "Fixture1", "--library-name", "Fixture2", - "--path", fixturesPackageFolder().path + "--path", fixturesPackageFolder().path, + "--output-file", temporaryFile ]) // When try command.run() // Then + let output = try String(contentsOfFile: temporaryFile) XCTAssertEqual( - outputs.joined(separator: "\n"), + output, """ # ---------------------------------- # API surface for Fixture1: # ---------------------------------- public class Car - public enum Manufacturer: String - case manufacturer1 - case manufacturer2 - case manufacturer3 - public init(manufacturer: Manufacturer) - public func startEngine() -> Bool - public func stopEngine() -> Bool + public enum Manufacturer: String + case manufacturer1 + case manufacturer2 + case manufacturer3 + public init(manufacturer: Manufacturer) + public func startEngine() -> Bool + public func stopEngine() -> Bool public extension Car - var price: Int - + var price: Int # ---------------------------------- # API surface for Fixture2: # ---------------------------------- public class Car - public enum Manufacturer: String - case manufacturer1 - case manufacturer2 - case manufacturer3 - public init(manufacturer: Manufacturer) - public func startEngine() -> Bool - public func stopEngine() -> Bool + public enum Manufacturer: String + case manufacturer1 + case manufacturer2 + case manufacturer3 + public init(manufacturer: Manufacturer) + public func startEngine() -> Bool + public func stopEngine() -> Bool public extension Car - var price: Int - → extension String - public func foo() + var price: Int + [?] extension String + public func foo() public extension Int - func bar() + func bar() + """ ) } + + func testVerifySurfaceForFixtureLibraries() throws { + // Write expected output to a temporary file + let referenceFile = "/tmp/api-surface-test-reference" + let expectedOutput = """ + # ---------------------------------- + # API surface for Fixture1: + # ---------------------------------- + + public class Car + public enum Manufacturer: String + case manufacturer1 + case manufacturer2 + case manufacturer3 + public init(manufacturer: Manufacturer) + public func startEngine() -> Bool + public func stopEngine() -> Bool + public extension Car + var price: Int + + """ + try expectedOutput.write(toFile: referenceFile, atomically: true, encoding: .utf8) + + // Generate API surface to temporary file + let generatedFile = "/tmp/api-surface-test-generated" + let generateCommand = try GenerateCommand.parse([ + "--library-name", "Fixture1", + "--path", fixturesPackageFolder().path, + "--output-file", generatedFile + ]) + try generateCommand.run() + + // Verify the generated file matches the reference + let verifyCommand = try VerifyCommand.parse([ + "--library-name", "Fixture1", + "--path", fixturesPackageFolder().path, + "--output-file", generatedFile, + referenceFile + ]) + try verifyCommand.run() + } + + func testVerifySurfaceMismatch() throws { + // Write incorrect reference output to a temporary file + let referenceFile = "/tmp/api-surface-test-reference" + let incorrectOutput = """ + # ---------------------------------- + # API surface for Fixture1: + # ---------------------------------- + + public class Bike + public init() + """ + try incorrectOutput.write(toFile: referenceFile, atomically: true, encoding: .utf8) + + // Generate API surface to temporary file + let generatedFile = "/tmp/api-surface-test-generated" + let generateCommand = try GenerateCommand.parse([ + "--library-name", "Fixture1", + "--path", fixturesPackageFolder().path, + "--output-file", generatedFile + ]) + try generateCommand.run() + + // Verify that it detects a mismatch + let verifyCommand = try VerifyCommand.parse([ + "--library-name", "Fixture1", + "--path", fixturesPackageFolder().path, + "--output-file", generatedFile, + referenceFile + ]) + + XCTAssertThrowsError(try verifyCommand.run()) { error in + if let validationError = error as? ValidationError { + XCTAssertEqual( + validationError.description, + "❌ API surface mismatch detected!\nRun `make api-surface` locally to update reference files and commit the changes." + ) + } else { + XCTFail("Unexpected error type: \(error)") + } + } + } } /// Resolves the url to `Fixtures` folder. private func fixturesPackageFolder() -> URL { var currentFolder = URL(fileURLWithPath: #file).deletingLastPathComponent() - while currentFolder.pathComponents.count > 0 { - if FileManager.default.fileExists(atPath: currentFolder.appendingPathComponent("Package.swift").path) { + let packageFileName = currentFolder.appendingPathComponent("Package.swift") + if FileManager.default.fileExists(atPath: packageFileName.path) { return currentFolder.appendingPathComponent("Fixtures") } else { currentFolder.deleteLastPathComponent()