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

115 lines
5.8 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# 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 25 can be done in parallel once step 1 is complete. Steps 35 are also independent of step 1.