From b89d0912c2ecfc11a8d9bcb85ce88c84c349d1da Mon Sep 17 00:00:00 2001 From: Bill Date: Sun, 19 Oct 2025 22:54:08 -0400 Subject: [PATCH] docs: design document for 100% test coverage via dependency injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add comprehensive design for achieving 100% test coverage through dependency injection refactoring. The design introduces adapter interfaces to decouple tool classes from Obsidian API dependencies, enabling cleaner, more maintainable tests. Key elements: - IVaultAdapter, IMetadataCacheAdapter, IFileManagerAdapter interfaces - Factory pattern for production usage - Phased implementation approach (adapters → VaultTools → NoteTools → integration) - Mock adapter pattern for simplified test setup - Coverage strategy organized by feature areas Goal: Codebase confidence for future refactoring and feature work. --- ...-10-19-100-percent-test-coverage-design.md | 367 ++++++++++++++++++ 1 file changed, 367 insertions(+) create mode 100644 docs/plans/2025-10-19-100-percent-test-coverage-design.md diff --git a/docs/plans/2025-10-19-100-percent-test-coverage-design.md b/docs/plans/2025-10-19-100-percent-test-coverage-design.md new file mode 100644 index 0000000..393d2b8 --- /dev/null +++ b/docs/plans/2025-10-19-100-percent-test-coverage-design.md @@ -0,0 +1,367 @@ +# 100% Test Coverage via Dependency Injection + +**Date:** 2025-10-19 +**Goal:** Achieve 100% test coverage through dependency injection refactoring +**Current Coverage:** 90.58% overall (VaultTools: 71.72%, NoteTools: 92.77%) + +## Motivation + +We want codebase confidence for future refactoring and feature work. The current test suite has good coverage but gaps remain in: +- Error handling paths +- Edge cases (type coercion, missing data) +- Complex conditional branches + +The current testing approach directly mocks Obsidian's `App` object, leading to: +- Complex, brittle mock setups +- Duplicated mocking code across test files +- Difficulty isolating specific behaviors +- Hard-to-test error conditions + +## Solution: Dependency Injection Architecture + +### Core Principle +Extract interfaces for Obsidian API dependencies, allowing tools to depend on abstractions rather than concrete implementations. This enables clean, simple mocks in tests while maintaining production functionality. + +### Architecture Overview + +**Current State:** +```typescript +class NoteTools { + constructor(private app: App) {} + // Methods use: this.app.vault.X, this.app.metadataCache.Y, etc. +} +``` + +**Target State:** +```typescript +class NoteTools { + constructor( + private vault: IVaultAdapter, + private metadata: IMetadataCacheAdapter, + private fileManager: IFileManagerAdapter + ) {} + // Methods use: this.vault.X, this.metadata.Y, etc. +} + +// Production usage via factory: +function createNoteTools(app: App): NoteTools { + return new NoteTools( + new VaultAdapter(app.vault), + new MetadataCacheAdapter(app.metadataCache), + new FileManagerAdapter(app.fileManager) + ); +} +``` + +## Interface Design + +### IVaultAdapter +Wraps file system operations from Obsidian's Vault API. + +```typescript +interface IVaultAdapter { + // File reading + read(path: string): Promise; + + // File existence and metadata + exists(path: string): boolean; + stat(path: string): { ctime: number; mtime: number; size: number } | null; + + // File retrieval + getAbstractFileByPath(path: string): TAbstractFile | null; + getMarkdownFiles(): TFile[]; + + // Directory operations + getRoot(): TFolder; +} +``` + +### IMetadataCacheAdapter +Wraps metadata and link resolution from Obsidian's MetadataCache API. + +```typescript +interface IMetadataCacheAdapter { + // Cache access + getFileCache(file: TFile): CachedMetadata | null; + + // Link resolution + getFirstLinkpathDest(linkpath: string, sourcePath: string): TFile | null; + + // Backlinks + getBacklinksForFile(file: TFile): { [key: string]: any }; + + // Additional metadata methods as needed +} +``` + +### IFileManagerAdapter +Wraps file modification operations from Obsidian's FileManager API. + +```typescript +interface IFileManagerAdapter { + // File operations + rename(file: TAbstractFile, newPath: string): Promise; + delete(file: TAbstractFile): Promise; + create(path: string, content: string): Promise; + modify(file: TFile, content: string): Promise; +} +``` + +## Implementation Strategy + +### Directory Structure +``` +src/ +├── adapters/ +│ ├── interfaces.ts # Interface definitions +│ ├── vault-adapter.ts # VaultAdapter implementation +│ ├── metadata-adapter.ts # MetadataCacheAdapter implementation +│ └── file-manager-adapter.ts # FileManagerAdapter implementation +├── tools/ +│ ├── note-tools.ts # Refactored to use adapters +│ └── vault-tools.ts # Refactored to use adapters +tests/ +├── __mocks__/ +│ ├── adapters.ts # Mock adapter factories +│ └── obsidian.ts # Existing Obsidian mocks (minimal usage going forward) +``` + +### Migration Approach + +**Step 1: Create Adapters** +- Define interfaces in `src/adapters/interfaces.ts` +- Implement concrete adapters (simple pass-through wrappers initially) +- Create mock adapter factories in `tests/__mocks__/adapters.ts` + +**Step 2: Refactor VaultTools** +- Update constructor to accept adapter interfaces +- Replace all `this.app.X` calls with `this.X` (using injected adapters) +- Create `createVaultTools(app: App)` factory function +- Update tests to use mock adapters + +**Step 3: Refactor NoteTools** +- Same pattern as VaultTools +- Create `createNoteTools(app: App)` factory function +- Update tests to use mock adapters + +**Step 4: Integration** +- Update ToolRegistry to use factory functions +- Update main.ts to use factory functions +- Verify all existing functionality preserved + +### Backward Compatibility + +**Plugin Code (main.ts, ToolRegistry):** +- Uses factory functions: `createNoteTools(app)`, `createVaultTools(app)` +- No awareness of adapters - just passes the App object +- Public API unchanged + +**Tool Classes:** +- Constructors accept adapters (new signature) +- All methods work identically (internal implementation detail) +- External callers use factory functions + +## Test Suite Overhaul + +### Mock Adapter Pattern + +**Centralized Mock Creation:** +```typescript +// tests/__mocks__/adapters.ts +export function createMockVaultAdapter(overrides?: Partial): IVaultAdapter { + return { + read: jest.fn(), + exists: jest.fn(), + stat: jest.fn(), + getAbstractFileByPath: jest.fn(), + getMarkdownFiles: jest.fn(), + getRoot: jest.fn(), + ...overrides + }; +} + +export function createMockMetadataCacheAdapter(overrides?: Partial): IMetadataCacheAdapter { + return { + getFileCache: jest.fn(), + getFirstLinkpathDest: jest.fn(), + getBacklinksForFile: jest.fn(), + ...overrides + }; +} + +export function createMockFileManagerAdapter(overrides?: Partial): IFileManagerAdapter { + return { + rename: jest.fn(), + delete: jest.fn(), + create: jest.fn(), + modify: jest.fn(), + ...overrides + }; +} +``` + +**Test Setup Simplification:** +```typescript +// Before: Complex App mock with nested properties +const mockApp = { + vault: { read: jest.fn(), ... }, + metadataCache: { getFileCache: jest.fn(), ... }, + fileManager: { ... }, + // Many more properties... +}; + +// After: Simple, targeted mocks +const vaultAdapter = createMockVaultAdapter({ + read: jest.fn().mockResolvedValue('file content') +}); +const tools = new VaultTools(vaultAdapter, mockMetadata, mockFileManager); +``` + +### Coverage Strategy by Feature Area + +**1. Frontmatter Operations** +- Test string tags → array conversion +- Test array tags → preserved as array +- Test missing frontmatter → base metadata only +- Test frontmatter parsing errors → error handling path +- Test all field types (title, aliases, custom fields) + +**2. Wikilink Validation** +- Test resolved links → included in results +- Test unresolved links → included with error details +- Test missing file → error path +- Test heading links (`[[note#heading]]`) +- Test alias links (`[[note|alias]]`) + +**3. Backlinks** +- Test `includeSnippets: true` → snippets included +- Test `includeSnippets: false` → snippets removed +- Test `includeUnlinked: true` → unlinked mentions included +- Test `includeUnlinked: false` → only linked mentions +- Test error handling paths + +**4. Search Utilities** +- Test glob pattern filtering +- Test regex search with matches +- Test regex search with no matches +- Test invalid regex → error handling +- Test edge cases (empty results, malformed patterns) + +**5. Note CRUD Operations** +- Test all conflict strategies: error, overwrite, rename +- Test version mismatch → conflict error +- Test missing file on update → error path +- Test permission errors → error handling +- Test all edge cases in uncovered lines + +**6. Path Validation Edge Cases** +- Test all PathUtils error conditions +- Test leading/trailing slash handling +- Test `..` traversal attempts +- Test absolute path rejection + +## Implementation Phases + +### Phase 1: Foundation (Adapters) +**Deliverables:** +- `src/adapters/interfaces.ts` - All interface definitions +- `src/adapters/vault-adapter.ts` - VaultAdapter implementation +- `src/adapters/metadata-adapter.ts` - MetadataCacheAdapter implementation +- `src/adapters/file-manager-adapter.ts` - FileManagerAdapter implementation +- `tests/__mocks__/adapters.ts` - Mock adapter factories +- Tests for adapters (basic pass-through verification) + +**Success Criteria:** +- All adapters compile without errors +- Mock adapters available for test usage +- Simple adapter tests pass + +### Phase 2: VaultTools Refactoring +**Deliverables:** +- Refactored VaultTools class using adapters +- `createVaultTools()` factory function +- Updated vault-tools.test.ts using mock adapters +- New tests for uncovered lines: + - Frontmatter extraction (lines 309-352) + - Wikilink validation error path (lines 716-735) + - Backlinks snippet removal (lines 824-852) + - Other uncovered paths + +**Success Criteria:** +- VaultTools achieves 100% coverage (all metrics) +- All existing tests pass +- No breaking changes to public API + +### Phase 3: NoteTools Refactoring +**Deliverables:** +- Refactored NoteTools class using adapters +- `createNoteTools()` factory function +- Updated note-tools.test.ts using mock adapters +- New tests for uncovered error paths and edge cases + +**Success Criteria:** +- NoteTools achieves 100% coverage (all metrics) +- All existing tests pass +- No breaking changes to public API + +### Phase 4: Integration & Verification +**Deliverables:** +- Updated ToolRegistry using factory functions +- Updated main.ts using factory functions +- Full test suite passing +- Coverage report showing 100% across all files +- Build succeeding with no errors + +**Success Criteria:** +- 100% test coverage: statements, branches, functions, lines +- All 400+ tests passing +- `npm run build` succeeds +- Manual smoke test in Obsidian confirms functionality + +## Risk Mitigation + +**Risk: Breaking existing functionality** +- Mitigation: Incremental refactoring, existing tests updated alongside code changes +- Factory pattern keeps plugin code nearly unchanged + +**Risk: Incomplete interface coverage** +- Mitigation: Start with methods actually used by tools, add to interfaces as needed +- Adapters are simple pass-throughs, easy to extend + +**Risk: Complex migration** +- Mitigation: Phased approach allows stopping after any phase +- Git worktree isolates changes from main branch + +**Risk: Test maintenance burden** +- Mitigation: Centralized mock factories reduce duplication +- Cleaner mocks are easier to maintain than complex App mocks + +## Success Metrics + +**Coverage Goals:** +- Statement coverage: 100% +- Branch coverage: 100% +- Function coverage: 100% +- Line coverage: 100% + +**Quality Goals:** +- All existing tests pass +- No type errors in build +- Plugin functions correctly in Obsidian +- Test code is cleaner and more maintainable + +**Timeline:** +- Phase 1: ~2-3 hours (adapters + mocks) +- Phase 2: ~3-4 hours (VaultTools refactor + tests) +- Phase 3: ~2-3 hours (NoteTools refactor + tests) +- Phase 4: ~1 hour (integration + verification) +- Total: ~8-11 hours of focused work + +## Future Benefits + +**After this refactoring:** +- Adding new tools is easier (use existing adapters) +- Testing new features is trivial (mock only what you need) +- Obsidian API changes isolated to adapter layer +- Confidence in comprehensive test coverage enables fearless refactoring +- New team members can understand test setup quickly