diff --git a/pass.xcodeproj/project.pbxproj b/pass.xcodeproj/project.pbxproj index d04dd81..e5522cb 100644 --- a/pass.xcodeproj/project.pbxproj +++ b/pass.xcodeproj/project.pbxproj @@ -114,8 +114,6 @@ 5F9D7B0D27AF6F7500A8AB22 /* CryptoTokenKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 5F9D7B0C27AF6F7300A8AB22 /* CryptoTokenKit.framework */; settings = {ATTRIBUTES = (Weak, ); }; }; 5F9D7B0E27AF6FCA00A8AB22 /* CryptoTokenKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 5F9D7B0C27AF6F7300A8AB22 /* CryptoTokenKit.framework */; settings = {ATTRIBUTES = (Weak, ); }; }; 5F9D7B0F27AF6FD200A8AB22 /* CryptoTokenKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 5F9D7B0C27AF6F7300A8AB22 /* CryptoTokenKit.framework */; settings = {ATTRIBUTES = (Weak, ); }; }; - 8A4716692F5EF56900C7A64D /* AppKeychainTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A4716682F5EF56900C7A64D /* AppKeychainTest.swift */; }; - 8A4716712F5EF7A900C7A64D /* PersistenceControllerTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A47166F2F5EF7A900C7A64D /* PersistenceControllerTest.swift */; }; 8AD8EBF32F5E2723007475AB /* Fixtures in Resources */ = {isa = PBXBuildFile; fileRef = 8AD8EBF22F5E268D007475AB /* Fixtures */; }; 9A1D1CE526E5D1CE0052028E /* OneTimePassword in Frameworks */ = {isa = PBXBuildFile; productRef = 9A1D1CE426E5D1CE0052028E /* OneTimePassword */; }; 9A1D1CE726E5D2230052028E /* OneTimePassword in Frameworks */ = {isa = PBXBuildFile; productRef = 9A1D1CE626E5D2230052028E /* OneTimePassword */; }; @@ -198,7 +196,7 @@ DC4914961E434301007FF592 /* LabelTableViewCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC4914941E434301007FF592 /* LabelTableViewCell.swift */; }; DC4914991E434600007FF592 /* PasswordDetailTableViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC4914981E434600007FF592 /* PasswordDetailTableViewController.swift */; }; DC5F385B1E56AADB00C69ACA /* PGPKeyArmorImportTableViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC5F385A1E56AADB00C69ACA /* PGPKeyArmorImportTableViewController.swift */; }; - DC6474532D20DD0C004B4BBC /* PersistenceController.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC6474522D20DD0C004B4BBC /* PersistenceController.swift */; }; + DC6474532D20DD0C004B4BBC /* CoreDataStack.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC6474522D20DD0C004B4BBC /* CoreDataStack.swift */; }; DC64745C2D29BE9B004B4BBC /* PasswordEntityTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC6474592D29BD43004B4BBC /* PasswordEntityTest.swift */; }; DC64745D2D29BEA9004B4BBC /* CoreDataTestCase.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC6474582D29BD43004B4BBC /* CoreDataTestCase.swift */; }; DC64745F2D45B240004B4BBC /* GitRepository.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC64745E2D45B23A004B4BBC /* GitRepository.swift */; }; @@ -425,8 +423,6 @@ 30F6C1B327664C7200BE5AB2 /* SVProgressHUD.xcframework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcframework; name = SVProgressHUD.xcframework; path = Carthage/Build/SVProgressHUD.xcframework; sourceTree = ""; }; 30FD2F77214D9E0E005E0A92 /* ParserTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ParserTest.swift; sourceTree = ""; }; 5F9D7B0C27AF6F7300A8AB22 /* CryptoTokenKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CryptoTokenKit.framework; path = System/Library/Frameworks/CryptoTokenKit.framework; sourceTree = SDKROOT; }; - 8A4716682F5EF56900C7A64D /* AppKeychainTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppKeychainTest.swift; sourceTree = ""; }; - 8A47166F2F5EF7A900C7A64D /* PersistenceControllerTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistenceControllerTest.swift; sourceTree = ""; }; 8AD8EBF22F5E268D007475AB /* Fixtures */ = {isa = PBXFileReference; lastKnownFileType = folder; path = Fixtures; sourceTree = ""; }; 9A1EF0B324C50DD80074FEAC /* passBeta.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = passBeta.entitlements; sourceTree = ""; }; 9A1EF0B424C50E780074FEAC /* passBetaAutoFillExtension.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = passBetaAutoFillExtension.entitlements; sourceTree = ""; }; @@ -504,7 +500,7 @@ DC4914941E434301007FF592 /* LabelTableViewCell.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LabelTableViewCell.swift; sourceTree = ""; }; DC4914981E434600007FF592 /* PasswordDetailTableViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PasswordDetailTableViewController.swift; sourceTree = ""; }; DC5F385A1E56AADB00C69ACA /* PGPKeyArmorImportTableViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PGPKeyArmorImportTableViewController.swift; sourceTree = ""; }; - DC6474522D20DD0C004B4BBC /* PersistenceController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistenceController.swift; sourceTree = ""; }; + DC6474522D20DD0C004B4BBC /* CoreDataStack.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CoreDataStack.swift; sourceTree = ""; }; DC6474582D29BD43004B4BBC /* CoreDataTestCase.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CoreDataTestCase.swift; sourceTree = ""; }; DC6474592D29BD43004B4BBC /* PasswordEntityTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PasswordEntityTest.swift; sourceTree = ""; }; DC64745E2D45B23A004B4BBC /* GitRepository.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GitRepository.swift; sourceTree = ""; }; @@ -630,7 +626,6 @@ 301F6464216164670071A4CE /* Helpers */ = { isa = PBXGroup; children = ( - 8A4716682F5EF56900C7A64D /* AppKeychainTest.swift */, 3032328922C9FBA2009EBD9C /* KeyFileManagerTest.swift */, ); path = Helpers; @@ -768,14 +763,6 @@ path = Crypto; sourceTree = ""; }; - 8A4716702F5EF7A900C7A64D /* Controllers */ = { - isa = PBXGroup; - children = ( - 8A47166F2F5EF7A900C7A64D /* PersistenceControllerTest.swift */, - ); - path = Controllers; - sourceTree = ""; - }; 9A58664F25AADB66006719C2 /* Services */ = { isa = PBXGroup; children = ( @@ -894,7 +881,6 @@ A26075861EEC6F34005DB03E /* passKitTests */ = { isa = PBXGroup; children = ( - 8A4716702F5EF7A900C7A64D /* Controllers */, DC64745A2D29BD43004B4BBC /* CoreData */, 30A86F93230F235800F821A4 /* Crypto */, 30BAC8C322E3BA4300438475 /* Testbase */, @@ -930,7 +916,7 @@ children = ( 30697C3121F63C8B0064FCAC /* PasscodeLockPresenter.swift */, 30697C3221F63C8B0064FCAC /* PasscodeLockViewController.swift */, - DC6474522D20DD0C004B4BBC /* PersistenceController.swift */, + DC6474522D20DD0C004B4BBC /* CoreDataStack.swift */, ); path = Controllers; sourceTree = ""; @@ -1622,7 +1608,7 @@ 3087574F2343E42A00B971A2 /* Colors.swift in Sources */, 30697C2C21F63C5A0064FCAC /* FileManagerExtension.swift in Sources */, 30697C3321F63C8B0064FCAC /* PasscodeLockPresenter.swift in Sources */, - DC6474532D20DD0C004B4BBC /* PersistenceController.swift in Sources */, + DC6474532D20DD0C004B4BBC /* CoreDataStack.swift in Sources */, 30697C3D21F63C990064FCAC /* UIViewExtension.swift in Sources */, 30697C3A21F63C990064FCAC /* UIViewControllerExtension.swift in Sources */, 30697C2E21F63C5A0064FCAC /* Utils.swift in Sources */, @@ -1641,7 +1627,6 @@ 30A86F95230F237000F821A4 /* CryptoFrameworkTest.swift in Sources */, 30A1D2AC21B32C2A00E2D1F7 /* TokenBuilderTest.swift in Sources */, 30DAFD4C240985E3002456E7 /* Array+SlicesTest.swift in Sources */, - 8A4716712F5EF7A900C7A64D /* PersistenceControllerTest.swift in Sources */, 301F646D216166AA0071A4CE /* AdditionFieldTest.swift in Sources */, 9ADC954124418A5F0005402E /* PasswordStoreTest.swift in Sources */, 30BAC8CB22E3BB6C00438475 /* DictBasedKeychain.swift in Sources */, @@ -1649,7 +1634,6 @@ A2699ACF24027D9500F36323 /* PasswordTableEntryTest.swift in Sources */, 30FD2F78214D9E0E005E0A92 /* ParserTest.swift in Sources */, A2AA934622DE3A8000D79A00 /* PGPAgentTest.swift in Sources */, - 8A4716692F5EF56900C7A64D /* AppKeychainTest.swift in Sources */, 30695E2524FAEF2600C9D46E /* GitCredentialTest.swift in Sources */, 30BAC8C622E3BAAF00438475 /* TestBase.swift in Sources */, 30B04860209A5141001013CA /* PasswordTest.swift in Sources */, diff --git a/passKit/Controllers/PersistenceController.swift b/passKit/Controllers/CoreDataStack.swift similarity index 90% rename from passKit/Controllers/PersistenceController.swift rename to passKit/Controllers/CoreDataStack.swift index 5d05001..0452259 100644 --- a/passKit/Controllers/PersistenceController.swift +++ b/passKit/Controllers/CoreDataStack.swift @@ -1,5 +1,5 @@ // -// PersistenceController.swift +// CoreDataStack.swift // passKit // // Created by Mingshen Sun on 12/28/24. @@ -18,19 +18,19 @@ public class PersistenceController { let container: NSPersistentContainer - init(storeURL: URL? = nil) { + init(isUnitTest: Bool = false) { self.container = NSPersistentContainer(name: Self.modelName, managedObjectModel: .sharedModel) let description = container.persistentStoreDescriptions.first description?.shouldMigrateStoreAutomatically = false description?.shouldInferMappingModelAutomatically = false - description?.url = storeURL ?? URL(fileURLWithPath: Globals.dbPath) + if isUnitTest { + description?.url = URL(fileURLWithPath: "/dev/null") + } else { + description?.url = URL(fileURLWithPath: Globals.dbPath) + } setup() } - static func forUnitTests() -> PersistenceController { - PersistenceController(storeURL: URL(fileURLWithPath: "/dev/null")) - } - func setup() { container.loadPersistentStores { _, error in if error != nil { diff --git a/passKitTests/Controllers/PersistenceControllerTest.swift b/passKitTests/Controllers/PersistenceControllerTest.swift deleted file mode 100644 index 4ae642f..0000000 --- a/passKitTests/Controllers/PersistenceControllerTest.swift +++ /dev/null @@ -1,93 +0,0 @@ -// -// PersistenceControllerTest.swift -// passKitTests -// -// Created by Lysann Tranvouez on 9/3/26. -// Copyright © 2026 Bob Sun. All rights reserved. -// - -import CoreData -import XCTest - -@testable import passKit - -final class PersistenceControllerTest: XCTestCase { - func testModelLoads() { - let controller = PersistenceController.forUnitTests() - let context = controller.viewContext() - - let entityNames = context.persistentStoreCoordinator!.managedObjectModel.entities.map(\.name) - XCTAssertEqual(entityNames, ["PasswordEntity"]) - } - - func testInsertAndFetch() { - let controller = PersistenceController.forUnitTests() - let context = controller.viewContext() - XCTAssertEqual(PasswordEntity.fetchAll(in: context).count, 0) - - PasswordEntity.insert(name: "test", path: "test.gpg", isDir: false, into: context) - try? context.save() - - XCTAssertEqual(PasswordEntity.fetchAll(in: context).count, 1) - } - - func testReinitializePersistentStoreClearsData() { - let controller = PersistenceController.forUnitTests() - let context = controller.viewContext() - - PasswordEntity.insert(name: "test1", path: "test1.gpg", isDir: false, into: context) - PasswordEntity.insert(name: "test2", path: "test2.gpg", isDir: false, into: context) - try? context.save() - XCTAssertEqual(PasswordEntity.fetchAll(in: context).count, 2) - - controller.reinitializePersistentStore() - - // After reinitialize, old data should be gone - // (reinitializePersistentStore calls initPasswordEntityCoreData with the default repo URL, - // which won't exist in tests, so the result should be an empty store) - let remaining = PasswordEntity.fetchAll(in: context) - XCTAssertEqual(remaining.count, 0) - } - - func testMultipleControllersAreIndependent() { - let controller1 = PersistenceController.forUnitTests() - let controller2 = PersistenceController.forUnitTests() - - let context1 = controller1.viewContext() - let context2 = controller2.viewContext() - - PasswordEntity.insert(name: "only-in-1", path: "only-in-1.gpg", isDir: false, into: context1) - try? context1.save() - - XCTAssertEqual(PasswordEntity.fetchAll(in: context1).count, 1) - XCTAssertEqual(PasswordEntity.fetchAll(in: context2).count, 0) - } - - func testSaveAndLoadFromFile() throws { - let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) - try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) - defer { try? FileManager.default.removeItem(at: tempDir) } - let storeURL = tempDir.appendingPathComponent("test.sqlite") - - // Write - let controller1 = PersistenceController(storeURL: storeURL) - let context1 = controller1.viewContext() - PasswordEntity.insert(name: "saved", path: "saved.gpg", isDir: false, into: context1) - PasswordEntity.insert(name: "dir", path: "dir", isDir: true, into: context1) - controller1.save() - - // Load in a fresh controller from the same file - let controller2 = PersistenceController(storeURL: storeURL) - let context2 = controller2.viewContext() - let allEntities = PasswordEntity.fetchAll(in: context2) - - XCTAssertEqual(allEntities.count, 2) - XCTAssertNotNil(allEntities.first { $0.name == "saved" && !$0.isDir }) - XCTAssertNotNil(allEntities.first { $0.name == "dir" && $0.isDir }) - } - - func testSaveError() throws { - // NOTE: save() calls fatalError on Core Data save failures, so error propagation - // cannot be tested without refactoring save() to throw... - } -} diff --git a/passKitTests/CoreData/CoreDataTestCase.swift b/passKitTests/CoreData/CoreDataTestCase.swift index b11ddd1..fa356bf 100644 --- a/passKitTests/CoreData/CoreDataTestCase.swift +++ b/passKitTests/CoreData/CoreDataTestCase.swift @@ -20,7 +20,7 @@ class CoreDataTestCase: XCTestCase { override func setUpWithError() throws { try super.setUpWithError() - controller = PersistenceController.forUnitTests() + controller = PersistenceController(isUnitTest: true) } override func tearDown() { diff --git a/passKitTests/CoreData/PasswordEntityTest.swift b/passKitTests/CoreData/PasswordEntityTest.swift index ad70f73..6362e2a 100644 --- a/passKitTests/CoreData/PasswordEntityTest.swift +++ b/passKitTests/CoreData/PasswordEntityTest.swift @@ -85,99 +85,4 @@ final class PasswordEntityTest: CoreDataTestCase { XCTAssertEqual(PasswordEntity.fetchAll(in: context).count, 0) } - - // MARK: - initPasswordEntityCoreData tests - - func testInitPasswordEntityCoreDataBuildsTree() throws { - let rootDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) - try FileManager.default.createDirectory(at: rootDir, withIntermediateDirectories: true) - defer { try? FileManager.default.removeItem(at: rootDir) } - - // Create directory structure: - // email/ - // work.gpg - // personal.gpg - // social/ - // mastodon.gpg - // toplevel.gpg - // notes.txt (non-.gpg file) - let emailDir = rootDir.appendingPathComponent("email") - let socialDir = rootDir.appendingPathComponent("social") - try FileManager.default.createDirectory(at: emailDir, withIntermediateDirectories: true) - try FileManager.default.createDirectory(at: socialDir, withIntermediateDirectories: true) - try Data("test1".utf8).write(to: emailDir.appendingPathComponent("work.gpg")) - try Data("test2".utf8).write(to: emailDir.appendingPathComponent("personal.gpg")) - try Data("test3".utf8).write(to: socialDir.appendingPathComponent("mastodon.gpg")) - try Data("test4".utf8).write(to: rootDir.appendingPathComponent("toplevel.gpg")) - try Data("test5".utf8).write(to: rootDir.appendingPathComponent("notes.txt")) - - let context = controller.viewContext() - PasswordEntity.initPasswordEntityCoreData(url: rootDir, in: context) - - // Verify total counts - let allEntities = PasswordEntity.fetchAll(in: context) - let files = allEntities.filter { !$0.isDir } - let dirs = allEntities.filter(\.isDir) - XCTAssertEqual(files.count, 5) // 4 .gpg + 1 .txt - XCTAssertEqual(dirs.count, 2) // email, social - - // Verify .gpg extension is stripped - let workEntity = allEntities.first { $0.path == "email/work.gpg" } - XCTAssertNotNil(workEntity) - XCTAssertEqual(workEntity!.name, "work") - - // Verify non-.gpg file keeps its extension - let notesEntity = allEntities.first { $0.path == "notes.txt" } - XCTAssertNotNil(notesEntity) - XCTAssertEqual(notesEntity!.name, "notes.txt") - - // Verify parent-child relationships - let emailEntity = allEntities.first { $0.path == "email" && $0.isDir } - XCTAssertNotNil(emailEntity) - XCTAssertEqual(emailEntity!.children.count, 2) - - // Verify top-level files have no parent (root was deleted) - let toplevelEntity = allEntities.first { $0.path == "toplevel.gpg" } - XCTAssertNotNil(toplevelEntity) - XCTAssertEqual(toplevelEntity!.name, "toplevel") - XCTAssertNil(toplevelEntity!.parent) - } - - func testInitPasswordEntityCoreDataSkipsHiddenFiles() throws { - let rootDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) - try FileManager.default.createDirectory(at: rootDir, withIntermediateDirectories: true) - defer { try? FileManager.default.removeItem(at: rootDir) } - - try Data("test".utf8).write(to: rootDir.appendingPathComponent("visible.gpg")) - try Data("test".utf8).write(to: rootDir.appendingPathComponent(".hidden.gpg")) - try Data("test".utf8).write(to: rootDir.appendingPathComponent(".gpg-id")) - try FileManager.default.createDirectory(at: rootDir.appendingPathComponent(".git"), withIntermediateDirectories: true) - try Data("test".utf8).write(to: rootDir.appendingPathComponent(".git/config")) - - let context = controller.viewContext() - PasswordEntity.initPasswordEntityCoreData(url: rootDir, in: context) - - let allEntities = PasswordEntity.fetchAll(in: context) - XCTAssertEqual(allEntities.count, 1) - XCTAssertEqual(allEntities.first!.name, "visible") - } - - func testInitPasswordEntityCoreDataHandlesEmptyDirectory() throws { - let rootDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) - try FileManager.default.createDirectory(at: rootDir, withIntermediateDirectories: true) - defer { try? FileManager.default.removeItem(at: rootDir) } - - try FileManager.default.createDirectory(at: rootDir.appendingPathComponent("emptydir"), withIntermediateDirectories: true) - - let context = controller.viewContext() - PasswordEntity.initPasswordEntityCoreData(url: rootDir, in: context) - - let allEntities = PasswordEntity.fetchAll(in: context) - let dirs = allEntities.filter(\.isDir) - let files = allEntities.filter { !$0.isDir } - XCTAssertEqual(dirs.count, 1) - XCTAssertEqual(dirs.first!.name, "emptydir") - XCTAssertEqual(dirs.first!.children.count, 0) - XCTAssertEqual(files.count, 0) - } } diff --git a/passKitTests/Helpers/AppKeychainTest.swift b/passKitTests/Helpers/AppKeychainTest.swift deleted file mode 100644 index 9b420fc..0000000 --- a/passKitTests/Helpers/AppKeychainTest.swift +++ /dev/null @@ -1,101 +0,0 @@ -// -// AppKeychainTest.swift -// passKitTests -// -// Created by Lysann Tranvouez on 9/3/26. -// Copyright © 2026 Bob Sun. All rights reserved. -// - -import XCTest - -@testable import passKit - -final class AppKeychainTest: XCTestCase { - private let keychain = AppKeychain.shared - private let testPrefix = "test.AppKeychainTest." - - override func tearDown() { - super.tearDown() - keychain.removeAllContent(withPrefix: testPrefix) - } - - private func key(_ name: String) -> String { - "\(testPrefix)\(name)" - } - - // MARK: - Basic round-trip - - func testAddAndGet() { - keychain.add(string: "hello", for: key("addGet")) - - XCTAssertEqual(keychain.get(for: key("addGet")), "hello") - } - - func testGetMissingKeyReturnsNil() { - XCTAssertNil(keychain.get(for: key("nonexistent"))) - } - - func testOverwriteValue() { - keychain.add(string: "first", for: key("overwrite")) - keychain.add(string: "second", for: key("overwrite")) - - XCTAssertEqual(keychain.get(for: key("overwrite")), "second") - } - - func testAddNilRemovesValue() { - keychain.add(string: "value", for: key("addNil")) - keychain.add(string: nil, for: key("addNil")) - - XCTAssertNil(keychain.get(for: key("addNil"))) - XCTAssertFalse(keychain.contains(key: key("addNil"))) - } - - // MARK: - contains - - func testContainsReturnsTrueForExistingKey() { - keychain.add(string: "value", for: key("exists")) - - XCTAssertTrue(keychain.contains(key: key("exists"))) - } - - func testContainsReturnsFalseForMissingKey() { - XCTAssertFalse(keychain.contains(key: key("missing"))) - } - - // MARK: - removeContent - - func testRemoveContent() { - keychain.add(string: "value", for: key("remove")) - keychain.removeContent(for: key("remove")) - - XCTAssertNil(keychain.get(for: key("remove"))) - XCTAssertFalse(keychain.contains(key: key("remove"))) - } - - func testRemoveContentForMissingKeyDoesNotThrow() { - keychain.removeContent(for: key("neverExisted")) - // No assertion needed — just verifying it doesn't crash - } - - // MARK: - removeAllContent(withPrefix:) - - func testRemoveAllContentWithPrefix() { - keychain.add(string: "1", for: key("prefixA.one")) - keychain.add(string: "2", for: key("prefixA.two")) - keychain.add(string: "3", for: key("prefixB.one")) - - keychain.removeAllContent(withPrefix: key("prefixA")) - - XCTAssertNil(keychain.get(for: key("prefixA.one"))) - XCTAssertNil(keychain.get(for: key("prefixA.two"))) - XCTAssertEqual(keychain.get(for: key("prefixB.one")), "3") - } - - func testRemoveAllContentWithPrefixNoMatches() { - keychain.add(string: "value", for: key("survivor")) - - keychain.removeAllContent(withPrefix: key("noMatch")) - - XCTAssertEqual(keychain.get(for: key("survivor")), "value") - } -} diff --git a/passKitTests/Models/GitCredentialTest.swift b/passKitTests/Models/GitCredentialTest.swift index 9a0d3e8..0e2c237 100644 --- a/passKitTests/Models/GitCredentialTest.swift +++ b/passKitTests/Models/GitCredentialTest.swift @@ -76,7 +76,6 @@ final class GitCredentialTest: XCTestCase { } func testSSHKeyCredentialProvider() throws { - throw XCTSkip("Skipped. This test failed in CI environment. Reason still unknown.") let credentialProvider = getCredentialProvider(authenticationMethod: .key) XCTAssertNotNil(credentialProvider.credential(for: .sshCustom, url: nil, userName: nil)) diff --git a/passKitTests/Models/PasswordStoreTest.swift b/passKitTests/Models/PasswordStoreTest.swift index de3f7b3..812a96b 100644 --- a/passKitTests/Models/PasswordStoreTest.swift +++ b/passKitTests/Models/PasswordStoreTest.swift @@ -128,10 +128,10 @@ final class PasswordStoreTest: XCTestCase { let password3 = Password(name: "test3", path: "folder/test3.gpg", plainText: "lorem ipsum") let password4 = Password(name: "test4", path: "test4.gpg", plainText: "you are valuable and you matter") - for password in [password1, password2, password3, password4] { + [password1, password2, password3, password4].forEach { password in expectation(forNotification: .passwordStoreUpdated, object: nil) - let savedEntity = try passwordStore.add(password: password) + let savedEntity = try? passwordStore.add(password: password) XCTAssertEqual(savedEntity!.name, password.name) waitForExpectations(timeout: 1, handler: nil) @@ -147,17 +147,6 @@ final class PasswordStoreTest: XCTestCase { XCTAssertEqual(passwordStore.numberOfLocalCommits, numLocalCommitsBefore + 4) } - func testAddAndDecryptRoundTrip() throws { - try cloneRepository(.empty) - try importSinglePGPKey() - - let password = Password(name: "test", path: "test.gpg", plainText: "foobar") - let savedEntity = try passwordStore.add(password: password) - - let decryptedPassword = try passwordStore.decrypt(passwordEntity: savedEntity!, requestPGPKeyPassphrase: requestPGPKeyPassphrase) - XCTAssertEqual(decryptedPassword.plainText, "foobar") - } - func testDeletePassword() throws { try cloneRepository(.withGPGID) let numCommitsBefore = passwordStore.numberOfCommits! @@ -246,7 +235,7 @@ final class PasswordStoreTest: XCTestCase { let numCommitsBefore = passwordStore.numberOfCommits! let numLocalCommitsBefore = passwordStore.numberOfLocalCommits - _ = try passwordStore.add(password: Password(name: "test", path: "test.gpg", plainText: "foobar")) + _ = try? passwordStore.add(password: Password(name: "test", path: "test.gpg", plainText: "foobar")) try passwordStore.delete(passwordEntity: passwordStore.fetchPasswordEntity(with: "personal/github.com.gpg")!) expectation(forNotification: .passwordStoreUpdated, object: nil) diff --git a/plans/01-improve-test-coverage-plan.md b/plans/01-improve-test-coverage-plan.md new file mode 100644 index 0000000..4b197d4 --- /dev/null +++ b/plans/01-improve-test-coverage-plan.md @@ -0,0 +1,115 @@ +# Improve Test Coverage Plan + +## Motivation + +The passKit codebase has ~100 test methods but critical components that will be heavily refactored (for multi-store support and other changes) have little or no test coverage. Adding regression tests now prevents silent breakage during future work. + +This is standalone — it should be done before any other refactoring. + +--- + +## Current Test Coverage + +### Well-tested areas +- Password parsing (`Password`, `Parser`, `AdditionField`, OTP, `TokenBuilder`) — ~40 tests +- `PGPAgent` — 8 tests covering multiple key types, error cases, passphrase handling +- `PasswordGenerator` — 8 tests +- `GitRepository` — 8 tests (uses real temp git repos on disk) +- `GitCredential` — 6 tests (SSH test is skipped/"failed in CI") +- `PasswordEntity` Core Data operations — 6 tests (uses in-memory store via `CoreDataTestCase`) +- `KeyFileManager` — 7 tests +- `QRKeyScanner` — 6 tests +- String/Array extensions — 6 tests + +### Critical gaps (zero tests) + +| Component | Notes | +|-----------|-------| +| **`PasswordStore`** (36 methods) | Only 1 integration test that clones from GitHub. No unit tests for pull, push, add, delete, edit, decrypt, encrypt, reset, erase, eraseStoreData, deleteCoreData, fetchPasswordEntityCoreData, initPasswordEntityCoreData. | +| **`AppKeychain`** | Zero tests. Only exercised indirectly via `DictBasedKeychain` mock. | +| **`PersistenceController` / Core Data stack** | Only the `isUnitTest: true` path is exercised. No tests for `reinitializePersistentStore`, `deletePersistentStore`, error recovery. | +| **Services** (`PasswordDecryptor`, `PasswordEncryptor`, `PasswordManager`, `PasswordNavigationDataSource`) | Zero tests. Core business logic that ties `PasswordStore` + `PGPAgent` together. | +| **All view controllers (28+)** | Zero tests. No UI test target exists. | +| **AutoFill / Share / Shortcuts extensions** | Zero tests. No test targets for extensions. | +| **`PasscodeLock`** | Zero tests. Security-critical. | + +### Test infrastructure that already exists +- `CoreDataTestCase` — base class with in-memory `PersistenceController` (reusable) +- `DictBasedKeychain` — in-memory `KeyStore` mock (reusable) +- `TestPGPKeys` — PGP key fixtures for RSA2048, RSA4096, ED25519, NISTP384, multi-key sets + +--- + +## Implementation + +### 1. Fixture password-store repo + +A pre-built bare git repo checked into `passKitTests/Fixtures/password-store.git/`. Contains: + +- A `.gpg-id` file with test key ID(s) +- Several `.gpg` files encrypted with the test keys from `TestPGPKeys` (at various directory depths) +- A subdirectory structure to exercise the BFS walk (nested folders, empty dirs) +- A git history with at least a couple of commits + +Since it's a bare repo, its contents (`HEAD`, `objects/`, `refs/`, etc.) are just regular files from the outer repo's perspective — no submodule issues. + +**Xcode project setup**: The fixture directory must be added to the Xcode project as a **folder reference** (blue folder) in the passKitTests target, and with "Build Rules" set to "Apply Once to Folder", so it's included in the "Copy Bundle Resources" build phase. In Xcode: drag the `Fixtures/` directory into the passKitTests group → select "Create folder references" → check only the passKitTests target. Without this, the files won't be accessible from the test bundle at runtime. + +To update the fixture, pull from origin (already set to `https://github.com/mssun/passforios-password-store.git`) or replace with any local bare repo: +```sh +# Update from origin +cd passKitTests/Fixtures/password-store.git +git fetch origin +git update-ref refs/heads/master origin/master + +# Or replace with a custom local repo +git clone --bare /path/to/local/repo passKitTests/Fixtures/password-store.git +``` + +### 2. `PasswordStore` unit tests (highest priority) + +- **Test `initPasswordEntityCoreData`**: Clone the fixture repo → verify correct `PasswordEntity` tree in Core Data (names, paths, directories, parent-child relationships). +- **Test `deleteCoreData`**: Populate, then delete, verify empty. +- **Test `eraseStoreData`**: Verify repo directory deleted, Core Data cleared, git handle nil'd. +- **Test `erase`**: Verify full cleanup (keychain, defaults, passcode, PGP state). +- **Test `fetchPasswordEntityCoreData`**: Verify fetch with parent filter, withDir filter. +- **Test encrypt → save → decrypt round-trip**: Using `DictBasedKeychain` + test PGP keys + local repo. +- **Test `add` / `delete` / `edit`**: Verify filesystem + Core Data + git commit. +- **Test `reset`**: Verify Core Data rebuilt to match filesystem after git reset. + +### 3. `PasswordEntity` relationship tests + +Extend `PasswordEntityTest` (already uses `CoreDataTestCase`): + +- **Test `initPasswordEntityCoreData` BFS walk**: Create a temp directory tree with `.gpg` files, call the static method, verify entity tree matches filesystem. +- **Test that `.gpg` extension is stripped** from names but non-`.gpg` files keep their names. +- **Test hidden files are skipped**. +- **Test empty directories**. + +### 4. `AppKeychain` tests + +Basic tests against the real Keychain API (or a test wrapper): + +- **Test `add` / `get` / `removeContent`** round-trip. +- **Test `removeAllContent`**. +- **Test `contains`**. +- **Test `removeAllContent(withPrefix:)`** — this method already exists and will be useful for per-store cleanup. + +### 5. `PersistenceController` tests + +- **Test `reinitializePersistentStore`** — verify existing data is gone after reinit. +- **Test model loading** — verify the `.momd` loads correctly. + +--- + +## Implementation Order + +| Step | Description | +|------|-------------| +| 1 | Fixture password-store bare repo | +| 2 | `PasswordStore` unit tests (uses fixture from step 1) | +| 3 | `PasswordEntity` BFS walk + relationship tests | +| 4 | `AppKeychain` tests | +| 5 | `PersistenceController` tests | + +Steps 2–5 can be done in parallel once step 1 is complete. Steps 3–5 are also independent of step 1.