test: Enable KSCrash variant of SentryTestUtils - #8552
Draft
NinjaLikesCheez wants to merge 21 commits into
Draft
Conversation
…ntegration Adds KSCrash crash handler installation to SentryKSCrashIntegration. - Defines KSCrashInstalling/KSCrashInstallerProvider protocols using only primitive types (String, UInt, Bool), so SentryTestUtils can conform without importing KSCrash. - Adds production KSCrashInstaller wrapping KSCrash.shared.install(with:). - Implements init in SentryKSCrashIntegration: installs with MonitorType.productionSafeMinimal monitors, respects cacheDirectoryPath and experimental.enableUnhandledCPPExceptionsV2, sets crashReporterInstalled and fatalDetected flags. - Wires SentryDependencyContainer to KSCrashInstallerProvider. - Adds TestKSCrashInstaller in SentryTestUtils and four integration tests.
… cycle The test file imported KSCrashInstallations directly to reference MonitorType.productionSafeMinimal.rawValue in an assertion. This created a dependency cycle: Installations → SentryTests+KSCrash → Sentry+KSCrash → Installations Fix: expose kscrashProductionSafeMonitors (a plain UInt) from KSCrashInstaller.swift, which already @_implementationOnly imports KSCrashInstallations, so the constant is available in SentrySwift without leaking KSCrash types. Tests now import only Sentry and SentryTestUtils, breaking the cycle.
KSCrash maintains a process-lifetime atomic bool (g_installed) in C. Once kscrash_install() succeeds, every subsequent call returns KSCrashInstallErrorAlreadyInstalled (domain KSCrashErrorDomain, code 1). This caused SentryKSCrashIntegration.init to return nil on any SDK re-initialisation (e.g. between test cases sharing a process), leaving crashReporterInstalled = false and breaking onLastRunStatusDetermined. Fix: catch the alreadyInstalled error in KSCrashInstaller and treat it as a no-op. The crash handler is already active; we still set the SDK flags correctly.
Swift Testing relies on Swift concurrency primitives (Actor, isolation()) which require macOS 10.15+. The KSCrash integration tests were rewritten to use Swift Testing, causing the TestV10KSCrash macOS build to fail with: 'Actor' is only available in macOS 10.15 or newer 'isolation()' is only available in macOS 10.15 or newer SentryTests.xcconfig includes DeploymentTargets.xcconfig (which sets 10.14) and then overrides MACOSX_DEPLOYMENT_TARGET to 10.15. This means all test targets (SentryTests and SentryTests+KSCrash, via SentryTestsKSCrash.xcconfig) pick up 10.15 while the SDK framework targets are unaffected — they include DeploymentTargets.xcconfig directly without going through SentryTests.xcconfig.
Reverts two commits: - 'test: update to use swift testing because the future is now' (d40cf78) — rewrote KSCrash integration tests with Swift Testing framework (@test, #expect, struct suites, Tag extension) - 'build: bump macOS deployment target to 10.15 for test targets only' (46e7cef) — was required because Swift Testing's concurrency APIs (Actor, isolation()) don't compile below macOS 10.15 Restores XCTestCase subclass with XCTAssert* assertions, removes the MACOSX_DEPLOYMENT_TARGET = 10.15 override from SentryTests.xcconfig, restores @_spi(Private) public on MockKSCrashInstaller, and removes the Swift Testing ONLY_TESTING note from Tests/AGENTS.md.
Apply the test<Function>_when<Condition>_should<Expected>() pattern
from Tests/AGENTS.md to all four KSCrash integration tests:
testInstallCalledOnInit
→ testInstall_whenCrashHandlerEnabled_shouldCallInstallOnce
testCrashedLastLaunchSetsFlag
→ testInstall_whenCrashedLastLaunch_shouldSetFatalDetected
testInstallFailureReturnsNil
→ testInstall_whenInstallThrows_shouldReturnNil
testInitSkipsWhenCrashHandlerDisabled
→ testInstall_whenCrashHandlerDisabled_shouldSkipInstall
MonitorType comes from KSCrashInstallations, which is imported @_implementationOnly in both SentryKSCrash+Integration.swift and SentryKSCrash+Installer.swift. @_implementationOnly makes the type completely invisible outside the Sentry module — even to @testable importers — so the test could not resolve MonitorType when it tried to call .rawValue on productionSafeMonitors. Fix: declare productionSafeMonitors as UInt (the raw type), computing it once with MonitorType([...]).rawValue inside the module where KSCrashInstallations is visible. The internal call site drops the now- redundant .rawValue suffix, and the test assertion compares UInt to UInt directly.
The integration appends 'KSCrash/<bundleIdentifier>' to the SDK's cacheDirectoryPath before passing it to the installer. The existing 'shouldCallInstallOnce' test was asserting installPath == cacheDirectoryPath directly, which became stale once the path was modified. Fix the stale assertion and add a dedicated test that mirrors the same URL construction, asserting the installer receives the expected subdirectory path.
fatalDetected is also set by things like the watchdog integration
…h types in function signatures)
NinjaLikesCheez
added
the
run-full-ci
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
Adds a KSCrash variant of the SentryTestUtils so that KSCrash is linked to the testing utilities.
💡 Motivation and Context
This enables 'easier' test writing and a more safe and concise API surface because we can leverage KSCrash types in APIs (see the MonitorType in this changeset).
💚 How did you test it?
Must not merged until #8469 does. This is not stacked because we're trying to keep that review stack small and focused.
#skip-changelog