From c4e97937ee6168feeecf9fee3f9152ac295bca22 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Fri, 2 Oct 2026 00:47:52 -0600 Subject: [PATCH] test(ios): read the log store off the main actor, and time the simulator job out `EditorLocalizationTests` reads its reports back with `OSLogStore`, a synchronous round trip to the log daemon, and did so on the main actor. On a loaded machine the daemon can take minutes to answer. For as long as it did, no other main-actor test in the run could make progress, and the ones waiting on a deadline failed. The read now runs on a thread of its own and gives up after 30 seconds. Each test restores the global localization state before it reads, so none holds that state across a suspension. The iOS Simulator job also gets a 20-minute timeout. A run whose main thread is stuck neither fails nor finishes, and one held its agent for 48 minutes. --- .buildkite/pipeline.yml | 3 + .../EditorLocalizationTests.swift | 170 ++++++++++++------ 2 files changed, 118 insertions(+), 55 deletions(-) diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index 1464179d7..2874f7528 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -118,6 +118,9 @@ steps: - label: ':swift: iOS Simulator Tests' key: swift-test-swift-package depends_on: build-react + # A run takes a few minutes. One whose main thread is stuck neither fails + # nor finishes, and would otherwise hold its agent for as long as it's stuck. + timeout_in_minutes: 20 command: | buildkite-agent artifact download dist.tar.gz . tar -xzf dist.tar.gz diff --git a/ios/Tests/GutenbergKitTests/EditorLocalizationTests.swift b/ios/Tests/GutenbergKitTests/EditorLocalizationTests.swift index c793ada83..1c015980a 100644 --- a/ios/Tests/GutenbergKitTests/EditorLocalizationTests.swift +++ b/ios/Tests/GutenbergKitTests/EditorLocalizationTests.swift @@ -4,7 +4,9 @@ import Testing @testable import GutenbergKit /// `EditorLocalization.localize` and its reporting state are process-global, so -/// these tests cannot safely interleave. +/// these tests cannot safely interleave. None of them holds that state across a +/// suspension, where another suite's main-actor test could run against it: each +/// restores it before reading the log store back. @MainActor @Suite(.serialized) struct EditorLocalizationTests { @@ -53,20 +55,20 @@ struct EditorLocalizationTests { /// the host assigns `localize`. Reporting those would name keys the host /// does translate, and reports that cry wolf get ignored. @Test - func readsBeforeAHostOverrideAreNotReported() throws { - try withLocalization(reportsMissingTranslations: true) { - let started = Date() + func readsBeforeAHostOverrideAreNotReported() async throws { + let started = Date() + withLocalization(reportsMissingTranslations: true) { // No host override installed: this is the editor reading its own // default, not a gap in anyone's translations. _ = EditorLocalization[.loadingEditor] - - let reports = try missingTranslationReports( - forKeyNamed: "loadingEditor", - since: started - ) - #expect(reports.isEmpty) } + + let reports = try await missingTranslationReports( + forKeyNamed: "loadingEditor", + since: started + ) + #expect(reports.isEmpty) } @Test @@ -100,79 +102,83 @@ struct EditorLocalizationTests { /// Call sites live in SwiftUI `body` methods that re-run on every render /// pass, so repeat lookups of one key must not each write a log entry. @Test - func repeatedFallbacksForOneKeyAreReportedOnce() throws { - try withLocalization(reportsMissingTranslations: true) { - EditorLocalization.localize = { _ in nil } + func repeatedFallbacksForOneKeyAreReportedOnce() async throws { + let started = Date() - let started = Date() + withLocalization(reportsMissingTranslations: true) { + EditorLocalization.localize = { _ in nil } for count in 1...5 { _ = EditorLocalization[.patternsCount(count)] } - - // One report despite five lookups, and despite the differing - // associated values, which must not split one key into many. - let reports = try missingTranslationReports( - forKeyNamed: "patternsCount", - since: started - ) - #expect(reports.count == 1) } + + // One report despite five lookups, and despite the differing + // associated values, which must not split one key into many. + let reports = try await missingTranslationReports( + forKeyNamed: "patternsCount", + since: started + ) + #expect(reports.count == 1) } @Test - func reportingCanBeDisabled() throws { + func reportingCanBeDisabled() async throws { + let started = Date() + // Enabled by the helper, then turned off here, so the assertion below // rests on this property rather than on the helper's default. - try withLocalization(reportsMissingTranslations: true) { + withLocalization(reportsMissingTranslations: true) { EditorLocalization.localize = { _ in nil } EditorLocalization.reportsMissingTranslations = false - let started = Date() _ = EditorLocalization[.lockdownModeDismiss] - - let reports = try missingTranslationReports( - forKeyNamed: "lockdownModeDismiss", - since: started - ) - #expect(reports.isEmpty) } + + let reports = try await missingTranslationReports( + forKeyNamed: "lockdownModeDismiss", + since: started + ) + #expect(reports.isEmpty) } /// Host apps are not required to configure `EditorLogger`, so the report /// has to reach the log store on its own. `debug` messages are held in an /// in-memory buffer and would not. @Test - func fallbackReachesTheLogStoreWithoutAHostLogger() throws { - let previousShared = EditorLogger.shared - let previousLevel = EditorLogger.logLevel + func fallbackReachesTheLogStoreWithoutAHostLogger() async throws { + let started = Date() - // Explicitly leave `EditorLogger` unconfigured. - EditorLogger.shared = nil - EditorLogger.logLevel = .error + do { + let previousShared = EditorLogger.shared + let previousLevel = EditorLogger.logLevel - defer { - EditorLogger.shared = previousShared - EditorLogger.logLevel = previousLevel - } + // Explicitly leave `EditorLogger` unconfigured. + EditorLogger.shared = nil + EditorLogger.logLevel = .error - try withLocalization(reportsMissingTranslations: true) { - EditorLocalization.localize = { key in - switch key { - case .showMore: "Mostrar más" - default: nil - } + defer { + EditorLogger.shared = previousShared + EditorLogger.logLevel = previousLevel } - let started = Date() - _ = EditorLocalization[.lockdownModeLearnMore] + withLocalization(reportsMissingTranslations: true) { + EditorLocalization.localize = { key in + switch key { + case .showMore: "Mostrar más" + default: nil + } + } - let reports = try missingTranslationReports( - forKeyNamed: "lockdownModeLearnMore", - since: started - ) - #expect(!reports.isEmpty) + _ = EditorLocalization[.lockdownModeLearnMore] + } } + + let reports = try await missingTranslationReports( + forKeyNamed: "lockdownModeLearnMore", + since: started + ) + #expect(!reports.isEmpty) } /// Reads the reports for one key back out of the system log store, which is @@ -181,10 +187,36 @@ struct EditorLocalizationTests { /// Scoped to a single key rather than a time window because /// `OSLogStore.position(date:)` resolves coarsely enough that entries from /// earlier tests fall inside the range. - private func missingTranslationReports( + /// + /// The read is a synchronous round trip to the log daemon, which answers + /// when it's ready: on a loaded machine that has taken minutes. So it runs + /// on a thread of its own rather than the main actor's, where every other + /// main-actor test in the run would wait behind it, and gives up after + /// `timeout`. + private nonisolated func missingTranslationReports( + forKeyNamed name: String, + since start: Date, + timeout: TimeInterval = 30 + ) async throws -> [String] { + try await withCheckedThrowingContinuation { continuation in + let answer = FirstAnswer(continuation) + + DispatchQueue.global(qos: .userInitiated).async { + answer.resume(with: Result { try Self.readMissingTranslationReports(forKeyNamed: name, since: start) }) + } + + DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + timeout) { + answer.resume(with: .failure(LogStoreTimedOut(seconds: timeout))) + } + } + } + + private nonisolated static func readMissingTranslationReports( forKeyNamed name: String, since start: Date ) throws -> [String] { + dispatchPrecondition(condition: .notOnQueue(.main)) + let store = try OSLogStore(scope: .currentProcessIdentifier) let entries = try store.getEntries( at: store.position(date: start), @@ -196,3 +228,31 @@ struct EditorLocalizationTests { .filter { $0.contains("Missing host translation for \(name),") } } } + +/// The log daemon didn't answer a read of the log store in time. +private struct LogStoreTimedOut: Error, CustomStringConvertible { + let seconds: TimeInterval + + var description: String { + "The log store didn't answer within \(Int(seconds)) seconds" + } +} + +/// Resumes a continuation with the first answer it's given, and drops any later one. +private final class FirstAnswer: @unchecked Sendable { + private let lock = NSLock() + private var continuation: CheckedContinuation? + + init(_ continuation: CheckedContinuation) { + self.continuation = continuation + } + + func resume(with result: Result) { + let continuation = lock.withLock { + defer { self.continuation = nil } + return self.continuation + } + + continuation?.resume(with: result) + } +}