feature implementation plans

plans/01-improve-test-coverage-plan.md

@@ -0,0 +1,98 @@
+# 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. `PasswordStore` unit tests (highest priority)
+
+The single existing test (`testCloneAndDecryptMultiKeys`) depends on network access. Add offline unit tests using a local git repo fixture:
+
+- **Setup/teardown**: Create a temp directory, `git init`, add `.gpg-id` + encrypted `.gpg` files, so tests don't need network.
+- **Test `initPasswordEntityCoreData`**: Clone a local 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.
+
+### 2. `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**.
+
+### 3. `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.
+
+### 4. `PersistenceController` tests
+
+- **Test `reinitializePersistentStore`** — verify existing data is gone after reinit.
+- **Test model loading** — verify the `.momd` loads correctly.
+
+### 5. Test infrastructure: local git repo fixture builder
+
+A helper that creates a temp git repo with configurable `.gpg-id`, encrypted `.gpg` files, and directory structure. Replaces the current network-dependent clone in `PasswordStoreTest`.
+
+---
+
+## Implementation Order
+
+All steps are independent and can be done in parallel:
+
+| Step | Description |
+|------|-------------|
+| 1 | `PasswordStore` unit tests (offline, local git fixture) |
+| 2 | `PasswordEntity` BFS walk + relationship tests |
+| 3 | `AppKeychain` tests |
+| 4 | `PersistenceController` tests |
+| 5 | Local git repo fixture builder (prerequisite for step 1) |