mirror of
https://github.com/phranck/TUIkit.git
synced 2026-05-21 09:50:35 +00:00
Fix: Resolve NSLock deadlock and re-enable localization test suites
- Fix re-entrant NSLock deadlock in string(for:) -> translationValue -> translations(for:) by restructuring to lock-free internal helpers (_translations, _translationValue)
- Fix wrong Bundle.module subdirectory path ("Localization/translations" -> "translations") that prevented translations from ever loading
- Fix didSet side effects during init() by computing language once before single Phase-1 assignment
- Remove redundant setNeedsRender() call in AppState.setLanguage()
- Add config directory injection for test isolation (LocalizationService(configDirectoryPath:))
- Re-enable LocalizationServiceTests and LocalizationKeyConsistencyTests (42 tests)
- Persistence tests now use isolated temp directories instead of real config path
This commit is contained in:
@@ -47,6 +47,5 @@ extension AppState {
|
||||
/// - Parameter language: The language to activate.
|
||||
public func setLanguage(_ language: LocalizationService.Language) {
|
||||
LocalizationService.shared.setLanguage(language)
|
||||
setNeedsRender()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -50,22 +50,50 @@ public final class LocalizationService: @unchecked Sendable {
|
||||
/// Lock for thread-safe access
|
||||
private let lock = NSLock()
|
||||
|
||||
/// Optional config directory override for testing.
|
||||
private let configDirectoryOverride: String?
|
||||
|
||||
/// Creates and initializes the localization service.
|
||||
///
|
||||
/// Loads the stored language preference, falling back to system locale
|
||||
/// or English if unavailable.
|
||||
public init() {
|
||||
self.currentLanguage = .english
|
||||
self.configDirectoryOverride = nil
|
||||
|
||||
// Try to load stored preference
|
||||
// Compute initial language before assignment to avoid didSet side effects.
|
||||
let initial: Language
|
||||
if let stored = Self.loadLanguagePreference() {
|
||||
self.currentLanguage = stored
|
||||
initial = stored
|
||||
} else if let systemLocale = Self.systemPreferredLanguage() {
|
||||
self.currentLanguage = systemLocale
|
||||
initial = systemLocale
|
||||
} else {
|
||||
initial = .english
|
||||
}
|
||||
self.currentLanguage = initial
|
||||
|
||||
// Preload current language translations
|
||||
_ = translations(for: currentLanguage)
|
||||
lock.lock()
|
||||
_ = _translations(for: currentLanguage)
|
||||
lock.unlock()
|
||||
}
|
||||
|
||||
/// Creates a localization service with a custom config directory.
|
||||
///
|
||||
/// Used for testing to isolate file system access.
|
||||
init(configDirectoryPath: String) {
|
||||
self.configDirectoryOverride = configDirectoryPath
|
||||
|
||||
let initial: Language
|
||||
if let stored = Self.loadLanguagePreference(from: configDirectoryPath) {
|
||||
initial = stored
|
||||
} else {
|
||||
initial = .english
|
||||
}
|
||||
self.currentLanguage = initial
|
||||
|
||||
lock.lock()
|
||||
_ = _translations(for: currentLanguage)
|
||||
lock.unlock()
|
||||
}
|
||||
|
||||
/// Changes the active language and persists the preference.
|
||||
@@ -89,13 +117,13 @@ public final class LocalizationService: @unchecked Sendable {
|
||||
defer { lock.unlock() }
|
||||
|
||||
// Try current language
|
||||
if let value = translationValue(key, in: currentLanguage) {
|
||||
if let value = _translationValue(key, in: currentLanguage) {
|
||||
return value
|
||||
}
|
||||
|
||||
// Fall back to English
|
||||
if currentLanguage != .english,
|
||||
let value = translationValue(key, in: .english) {
|
||||
let value = _translationValue(key, in: .english) {
|
||||
return value
|
||||
}
|
||||
|
||||
@@ -103,13 +131,12 @@ public final class LocalizationService: @unchecked Sendable {
|
||||
return key
|
||||
}
|
||||
|
||||
// MARK: - Private Helpers
|
||||
// MARK: - Private Helpers (caller MUST hold lock)
|
||||
|
||||
/// Gets translations dictionary for a language, loading from JSON if needed.
|
||||
private func translations(for language: Language) -> [String: String] {
|
||||
lock.lock()
|
||||
defer { lock.unlock() }
|
||||
|
||||
///
|
||||
/// - Important: Caller must hold `lock`.
|
||||
private func _translations(for language: Language) -> [String: String] {
|
||||
let code = language.rawValue
|
||||
|
||||
// Return cached if available
|
||||
@@ -128,9 +155,10 @@ public final class LocalizationService: @unchecked Sendable {
|
||||
}
|
||||
|
||||
/// Retrieves a value from translations using dot-notation path.
|
||||
private func translationValue(_ key: String, in language: Language) -> String? {
|
||||
let trans = translations(for: language)
|
||||
return trans[key]
|
||||
///
|
||||
/// - Important: Caller must hold `lock`.
|
||||
private func _translationValue(_ key: String, in language: Language) -> String? {
|
||||
_translations(for: language)[key]
|
||||
}
|
||||
|
||||
/// Loads translations from bundled JSON file.
|
||||
@@ -138,7 +166,7 @@ public final class LocalizationService: @unchecked Sendable {
|
||||
guard let url = Bundle.module.url(
|
||||
forResource: language,
|
||||
withExtension: "json",
|
||||
subdirectory: "Localization/translations"
|
||||
subdirectory: "translations"
|
||||
) else {
|
||||
return nil
|
||||
}
|
||||
@@ -167,9 +195,14 @@ public final class LocalizationService: @unchecked Sendable {
|
||||
return nil
|
||||
}
|
||||
|
||||
/// Loads stored language preference from config file.
|
||||
/// Loads stored language preference from the default config file.
|
||||
private static func loadLanguagePreference() -> Language? {
|
||||
let path = configFilePath()
|
||||
loadLanguagePreference(from: defaultConfigDirectoryPath())
|
||||
}
|
||||
|
||||
/// Loads stored language preference from a specific config directory.
|
||||
private static func loadLanguagePreference(from configDirectory: String) -> Language? {
|
||||
let path = (configDirectory as NSString).appendingPathComponent("language")
|
||||
guard FileManager.default.fileExists(atPath: path) else {
|
||||
return nil
|
||||
}
|
||||
@@ -186,7 +219,7 @@ public final class LocalizationService: @unchecked Sendable {
|
||||
|
||||
/// Saves language preference to config file.
|
||||
private func saveLanguagePreference(_ language: Language) {
|
||||
let path = Self.configFilePath()
|
||||
let path = configFilePath()
|
||||
let dirPath = (path as NSString).deletingLastPathComponent
|
||||
|
||||
// Create directory if needed
|
||||
@@ -212,18 +245,24 @@ public final class LocalizationService: @unchecked Sendable {
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the XDG-compatible config file path for language preference.
|
||||
private static func configFilePath() -> String {
|
||||
/// Returns the config file path, using override if set.
|
||||
private func configFilePath() -> String {
|
||||
let dir = configDirectoryOverride ?? Self.defaultConfigDirectoryPath()
|
||||
return (dir as NSString).appendingPathComponent("language")
|
||||
}
|
||||
|
||||
/// Returns the XDG-compatible default config directory path.
|
||||
static func defaultConfigDirectoryPath() -> String {
|
||||
#if os(macOS)
|
||||
let appSupport = FileManager.default.urls(
|
||||
for: .applicationSupportDirectory,
|
||||
in: .userDomainMask
|
||||
).first?.path ?? NSHomeDirectory()
|
||||
return (appSupport as NSString).appendingPathComponent("tuikit/language")
|
||||
return (appSupport as NSString).appendingPathComponent("tuikit")
|
||||
#else
|
||||
let configHome = ProcessInfo.processInfo.environment["XDG_CONFIG_HOME"]
|
||||
?? ((NSHomeDirectory() as NSString).appendingPathComponent(".config"))
|
||||
return (configHome as NSString).appendingPathComponent("tuikit/language")
|
||||
return (configHome as NSString).appendingPathComponent("tuikit")
|
||||
#endif
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,7 +16,7 @@ import Testing
|
||||
/// 1. Every key in the enum exists in the English translation file
|
||||
/// 2. No extra keys are in the translation files that don't exist in the enum
|
||||
/// 3. All enum keys are actually used (no dead code)
|
||||
@Suite("LocalizationKeyConsistency", .disabled("Disabled: Bundle loading in init() causes hangs on CI"))
|
||||
@Suite("LocalizationKeyConsistency")
|
||||
final class LocalizationKeyConsistencyTests {
|
||||
private var englishTranslations: [String: String] = [:]
|
||||
|
||||
@@ -31,7 +31,7 @@ final class LocalizationKeyConsistencyTests {
|
||||
var url = Bundle.module.url(
|
||||
forResource: "en",
|
||||
withExtension: "json",
|
||||
subdirectory: "Localization/translations"
|
||||
subdirectory: "translations"
|
||||
)
|
||||
|
||||
// If not found, try to load from the project directory (for tests)
|
||||
|
||||
@@ -10,13 +10,13 @@ import Testing
|
||||
|
||||
// MARK: - Localization Service Tests
|
||||
|
||||
@Suite("LocalizationService", .disabled("Disabled: Complex test setup interactions - refactor required"))
|
||||
@Suite("LocalizationService")
|
||||
final class LocalizationServiceTests {
|
||||
var sut: LocalizationService!
|
||||
let fileManager = FileManager.default
|
||||
var sut: LocalizationService
|
||||
|
||||
init() {
|
||||
sut = LocalizationService()
|
||||
sut.setLanguage(.english)
|
||||
}
|
||||
|
||||
// MARK: - Bundle Loading Tests
|
||||
@@ -153,52 +153,62 @@ final class LocalizationServiceTests {
|
||||
|
||||
// MARK: - Persistence Tests
|
||||
|
||||
@Test("Saves language preference to config file", .disabled("Disabled: File I/O conflicts with parallel test execution"))
|
||||
func savesLanguagePreference() {
|
||||
sut.setLanguage(.german)
|
||||
let path = Self.configFilePath()
|
||||
let content = try? String(contentsOfFile: path, encoding: .utf8).trimmingCharacters(
|
||||
@Test("Saves language preference to config file")
|
||||
func savesLanguagePreference() throws {
|
||||
let tempDir = NSTemporaryDirectory() + "tuikit-test-\(UUID().uuidString)"
|
||||
defer { try? FileManager.default.removeItem(atPath: tempDir) }
|
||||
|
||||
let service = LocalizationService(configDirectoryPath: tempDir)
|
||||
service.setLanguage(.german)
|
||||
|
||||
let path = (tempDir as NSString).appendingPathComponent("language")
|
||||
let content = try String(contentsOfFile: path, encoding: .utf8).trimmingCharacters(
|
||||
in: .whitespacesAndNewlines
|
||||
)
|
||||
#expect(content == "de")
|
||||
}
|
||||
|
||||
@Test("Loads language preference from config file", .disabled("Disabled: File I/O conflicts with parallel test execution"))
|
||||
func loadsLanguagePreference() {
|
||||
let path = Self.configFilePath()
|
||||
let dirPath = (path as NSString).deletingLastPathComponent
|
||||
try? fileManager.createDirectory(
|
||||
atPath: dirPath,
|
||||
@Test("Loads language preference from config file")
|
||||
func loadsLanguagePreference() throws {
|
||||
let tempDir = NSTemporaryDirectory() + "tuikit-test-\(UUID().uuidString)"
|
||||
defer { try? FileManager.default.removeItem(atPath: tempDir) }
|
||||
|
||||
try FileManager.default.createDirectory(
|
||||
atPath: tempDir,
|
||||
withIntermediateDirectories: true,
|
||||
attributes: nil
|
||||
)
|
||||
try? "fr".write(toFile: path, atomically: true, encoding: .utf8)
|
||||
let path = (tempDir as NSString).appendingPathComponent("language")
|
||||
try "fr".write(toFile: path, atomically: true, encoding: .utf8)
|
||||
|
||||
let newService = LocalizationService()
|
||||
#expect(newService.currentLanguage == .french)
|
||||
let service = LocalizationService(configDirectoryPath: tempDir)
|
||||
#expect(service.currentLanguage == .french)
|
||||
}
|
||||
|
||||
@Test("Handles missing config file gracefully", .disabled("Disabled: File I/O conflicts with parallel test execution"))
|
||||
@Test("Handles missing config file gracefully")
|
||||
func handlesMissingConfigFile() {
|
||||
try? fileManager.removeItem(atPath: Self.configFilePath())
|
||||
let tempDir = NSTemporaryDirectory() + "tuikit-test-\(UUID().uuidString)"
|
||||
defer { try? FileManager.default.removeItem(atPath: tempDir) }
|
||||
|
||||
let newService = LocalizationService()
|
||||
#expect(newService.currentLanguage == .english)
|
||||
let service = LocalizationService(configDirectoryPath: tempDir)
|
||||
#expect(service.currentLanguage == .english)
|
||||
}
|
||||
|
||||
@Test("Handles invalid config file content gracefully", .disabled("Disabled: File I/O conflicts with parallel test execution"))
|
||||
func handlesInvalidConfigFile() {
|
||||
let path = Self.configFilePath()
|
||||
let dirPath = (path as NSString).deletingLastPathComponent
|
||||
try? fileManager.createDirectory(
|
||||
atPath: dirPath,
|
||||
@Test("Handles invalid config file content gracefully")
|
||||
func handlesInvalidConfigFile() throws {
|
||||
let tempDir = NSTemporaryDirectory() + "tuikit-test-\(UUID().uuidString)"
|
||||
defer { try? FileManager.default.removeItem(atPath: tempDir) }
|
||||
|
||||
try FileManager.default.createDirectory(
|
||||
atPath: tempDir,
|
||||
withIntermediateDirectories: true,
|
||||
attributes: nil
|
||||
)
|
||||
try? "invalid_lang_code".write(toFile: path, atomically: true, encoding: .utf8)
|
||||
let path = (tempDir as NSString).appendingPathComponent("language")
|
||||
try "invalid_lang_code".write(toFile: path, atomically: true, encoding: .utf8)
|
||||
|
||||
let newService = LocalizationService()
|
||||
#expect(newService.currentLanguage == .english)
|
||||
let service = LocalizationService(configDirectoryPath: tempDir)
|
||||
#expect(service.currentLanguage == .english)
|
||||
}
|
||||
|
||||
// MARK: - Language Enum Tests
|
||||
@@ -263,28 +273,13 @@ final class LocalizationServiceTests {
|
||||
#expect(englishAgain == "Save")
|
||||
}
|
||||
|
||||
// MARK: - Helper Methods
|
||||
|
||||
private static func configFilePath() -> String {
|
||||
#if os(macOS)
|
||||
let appSupport = FileManager.default.urls(
|
||||
for: .applicationSupportDirectory,
|
||||
in: .userDomainMask
|
||||
).first?.path ?? NSHomeDirectory()
|
||||
return (appSupport as NSString).appendingPathComponent("tuikit/language")
|
||||
#else
|
||||
let configHome = ProcessInfo.processInfo.environment["XDG_CONFIG_HOME"]
|
||||
?? ((NSHomeDirectory() as NSString).appendingPathComponent(".config"))
|
||||
return (configHome as NSString).appendingPathComponent("tuikit/language")
|
||||
#endif
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Localization Key Tests
|
||||
|
||||
@Suite("LocalizationKey")
|
||||
final class LocalizationKeyTests {
|
||||
var service: LocalizationService!
|
||||
var service: LocalizationService
|
||||
|
||||
init() {
|
||||
service = LocalizationService()
|
||||
|
||||
Reference in New Issue
Block a user