Skip to content

Commit 4e4ea82

Browse files
pajomapajoma
authored
Merge pull request #218 from pajoma/feat/212-infrastructure-seams
feat(arch): introduce IFileSystem seam for testability (#212)
2 parents b9426c0 + 2ed29c8 commit 4e4ea82

22 files changed

Lines changed: 582 additions & 71 deletions

‎docs/plans/2026-05-18-212-infrastructure-seams.md‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# Plan: Infrastructure Abstraction Seams for Testability — #212
2+
3+
## Reference spec
4+
[docs/specs/2026-05-18-212-infrastructure-seams.md](../specs/2026-05-18-212-infrastructure-seams.md)
5+
6+
## Approach
7+
Introduce a platform-agnostic `IFileSystem` interface using string-based URI paths and custom stat types. Implement a `VscodeFileSystem` adapter for production and an `InMemoryFileSystem` for headless unit tests. Refactor `Writer`, `ScanEntries`, and `fileExists` to depend on the interface, enabling pure-Node testing of core domain logic.
8+
9+
## Steps
10+
11+
### 1 — Define Domain FS Types (`src/model/fs.ts`)
12+
Create new file. Define `JFileType` (enum) and `JFileStat` (interface) with zero `vscode` imports. This establishes the platform-agnostic data structures for the filesystem.
13+
14+
### 2 — Define `IFileSystem` Interface (`src/model/interfaces.ts`)
15+
Add `IFileSystem` to the central interfaces file. Use `string` for all path parameters to avoid `vscode.Uri` coupling. Update `JournalController` and `IDialogues` (if needed) to ensure zero `vscode` leaks in method signatures.
16+
17+
### 3 — Implement `VscodeFileSystem` Adapter (`src/vscode/vscode-fs.ts`)
18+
Create production implementation that wraps `vscode.workspace.fs`.
19+
- Convert `string` paths → `vscode.Uri.parse(path)`.
20+
- Map `vscode.FileType` ↔ `JFileType`.
21+
- Map `vscode.FileStat` → `JFileStat`.
22+
- Ensure errors are re-thrown so that `.code === 'FileNotFound'` remains consistent.
23+
24+
### 4 — Thread `IFileSystem` through `Ctrl` (`src/util/controller.ts`)
25+
- Add `fs: IFileSystem` property to `JournalController` interface.
26+
- Add `private _fs?: IFileSystem` and `public get fs()` to `Ctrl`.
27+
- Update `initServices(logger: ILogger)` to accept `fs: IFileSystem` (or construct `VscodeFileSystem` internally if appropriate for current architecture).
28+
- **Update:** Based on #208 (constructor injection), update `Startup.initServices` to construct and pass the `fs` implementation.
29+
30+
### 5 — Refactor `fileExists` (`src/util/fs-exists.ts`)
31+
- Change signature to `fileExists(fs: IFileSystem, path: string): Promise<boolean>`.
32+
- Replace `vscode.workspace.fs.stat` with `fs.stat`.
33+
- Catch `{ code: 'FileNotFound' }` for the false branch.
34+
35+
### 6 — Refactor `Writer` (`src/journal/writer.ts`)
36+
- Add `private fs: IFileSystem` and `private ui: IDialogues` to constructor.
37+
- Replace `vscode.workspace.fs.writeFile` with `fs.writeFile`.
38+
- Replace `vscode.workspace.openTextDocument` with `this.ui.openDocument`.
39+
- Convert `string` content → `Uint8Array` (Buffer.from) for the `writeFile` call.
40+
41+
### 7 — Refactor `ScanEntries` (`src/features/entries/scan-entries.ts`)
42+
- Add `private fs: IFileSystem` to constructor.
43+
- Replace `vscode.workspace.fs.readDirectory` and `.stat` calls with `fs` equivalents.
44+
- Use `JFileType` for logic branches.
45+
46+
### 8 — Implement `InMemoryFileSystem` (`src/test/in-memory-fs.ts`)
47+
Create test double for headless unit tests.
48+
- Use `Map<string, Uint8Array>` for file storage.
49+
- Use `Map<string, [string, JFileType][]>` for directory listings.
50+
- Mock `stat` to return `JFileStat` from map keys.
51+
- Throw `{ code: 'FileNotFound' }` for missing entries.
52+
53+
### 9 — Add Headless Unit Tests (`src/test/suite/*-unit.test.ts`)
54+
- `writer-unit.test.ts`: Verify `createSaveLoadTextDocument` writes correct content to `InMemoryFileSystem`.
55+
- `scan-entries-unit.test.ts`: Verify `walkDir` correctly traverses a mock directory tree.
56+
- Run via `mocha` directly (headless).
57+
58+
## Test scenarios
59+
- **Production Integration:** Existing tests must pass using `VscodeFileSystem`.
60+
- **Headless Isolation:** `npm test -- --grep "-unit"` must pass in < 5s without the extension host.
61+
- **Error Consistency:** Verify `fileExists` correctly handles missing files in both implementations.
62+
63+
## Rollback
64+
`git revert <commit>` — structural refactor with no side effects on user data.

‎docs/specs/2026-05-18-212-infrastructure-seams.md‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
# Spec: Infrastructure Abstraction Seams for Testability (#212)
2+
3+
## Goal
4+
5+
Introduce an `IFileSystem` interface (free of `vscode.*` types) that decouples `vscode.workspace.fs` from core domain logic, enabling pure-Node unit tests for file creation and directory walking without the VS Code Extension Host.
6+
7+
## Why Now
8+
9+
The test suite is entirely extension-host-bound: every test spins up a real VS Code process (~30 s suite run) because core logic calls `vscode.workspace.fs` directly. Introducing a seam here unblocks fast TDD cycles (< 5 s plain-Node) for the most I/O-heavy domain classes (`Writer`, `ScanEntries`, `fileExists`). This is the smallest architectural step with the highest return on test speed.
10+
11+
## Existing vs. New
12+
13+
The existing `IWriter`, `IReader`, `IInject`, `IDialogues` interfaces in `src/model/interfaces.ts` already serve as service-level seams. However they are insufficient for headless testing because:
14+
15+
- `Writer.createSaveLoadTextDocument` calls `vscode.workspace.fs.writeFile` and `vscode.workspace.openTextDocument` directly (inconsistently — `Reader` already delegates the `openDocument` step to `IDialogues`).
16+
- `fileExists()` in `src/util/fs-exists.ts` calls `vscode.workspace.fs.stat` directly.
17+
- `ScanEntries.walkDir()` calls `vscode.workspace.fs.readDirectory` and `.stat` directly.
18+
19+
The issue also mentions a `WorkspaceUI` interface. `IDialogues` in `src/model/interfaces.ts` already covers exactly that surface (`openDocument`, `showDocument`, `showError`, `getUserInput`). No new interface needed; the issue's `WorkspaceUI` name maps to `IDialogues`.
20+
21+
## Amendment (post-review)
22+
23+
Initial spec used `vscode.Uri` and `vscode.FileStat`/`vscode.FileType` in the `IFileSystem` interface. This was correctly identified as a type leak: `vscode.Uri` is a runtime class from the Extension Host, so any file importing the interface would still require the extension host for plain-Node tests.
24+
25+
Changes from the initial draft:
26+
1. `IFileSystem` now uses `string` paths (URI string representation, e.g., `file:///…`) and defines its own `JFileStat` / `JFileType` in the model — zero `vscode.*` imports in the interface.
27+
2. `Writer.createSaveLoadTextDocument` delegates `openTextDocument` to `IDialogues.openDocument` (aligning with `Reader`'s pattern) — eliminates the remaining direct `vscode` call from `Writer`.
28+
3. `IWorkspaceEditor` / `WorkspaceEdit` seam is **out of scope** — `Inject.ts` operates on already-open editor buffers, which is editor I/O, not filesystem I/O. Different seam, separate issue.
29+
4. Removing all `vscode.*` types from `IWriter`/`IReader`/`IInject`/`IDialogues` (the `TextDocument`, `Position`, `TextEditor` surface) is Phase 2.1 work — deferred. `IFileSystem` itself is fully clean.
30+
31+
## In Scope
32+
33+
1. **Define `JFileType` and `JFileStat`** in `src/model/fs.ts` (new file, no `vscode` import):
34+
```ts
35+
export enum JFileType { Unknown = 0, File = 1, Directory = 2, SymbolicLink = 64 }
36+
export interface JFileStat { type: JFileType; ctime: number; mtime: number; size: number; }
37+
```
38+
39+
2. **Define `IFileSystem` interface** in `src/model/interfaces.ts` — all paths are `string` (URI string form):
40+
- `stat(path: string): Promise<JFileStat>`
41+
- `readFile(path: string): Promise<Uint8Array>`
42+
- `writeFile(path: string, content: Uint8Array): Promise<void>`
43+
- `readDirectory(path: string): Promise<[string, JFileType][]>`
44+
- `createDirectory(path: string): Promise<void>`
45+
- `delete(path: string, options?: { recursive?: boolean }): Promise<void>`
46+
47+
3. **Provide `VscodeFileSystem`** (`src/vscode/vscode-fs.ts`) — converts `string → vscode.Uri.file(path)` for every call, maps `vscode.FileType ↔ JFileType` and `vscode.FileStat → JFileStat`. Production implementation. Zero vscode imports visible to callers. Note: `Uri.file` is correct because callers pass OS paths (not URI strings); `extensionKind: ["workspace"]` ensures the extension runs on the remote host so OS paths are remote-local.
48+
49+
4. **Thread `IFileSystem` into `Ctrl`** (`src/util/controller.ts`): add `fs: IFileSystem` getter. `Startup.initServices` constructs `new VscodeFileSystem()`.
50+
51+
5. **Replace direct `vscode.workspace.fs` calls**:
52+
- `src/util/fs-exists.ts` — change to `fileExists(fs: IFileSystem, path: string): Promise<boolean>`. Catch `{ code: 'FileNotFound' }` instead of `vscode.FileSystemError`.
53+
- `src/journal/writer.ts` — inject `IFileSystem` (for `writeFile`) and `IDialogues` (for `openDocument`) via constructor, removing the two direct `vscode` calls.
54+
- `src/features/entries/scan-entries.ts` — inject `IFileSystem`, replace `.stat` and `.readDirectory` calls; map `JFileType.Directory` instead of `vscode.FileType.Directory`.
55+
56+
6. **Ship `InMemoryFileSystem` test double** (`src/test/in-memory-fs.ts`) — `Map<string, Uint8Array>` backing store + `Map<string, [string, JFileType][]>` for directory entries. Throws `{ code: 'FileNotFound' }` for missing paths. Zero vscode imports.
57+
58+
7. **Add pure-Node unit tests** (no extension host, run via `node` / mocha-direct):
59+
- `src/test/suite/writer-unit.test.ts` — verifies `createSaveLoadTextDocument` writes correct bytes at the correct path, with stubbed `IDialogues.openDocument`.
60+
- `src/test/suite/scan-entries-unit.test.ts` — verifies `walkDir` traversal against an in-memory directory tree.
61+
62+
## Out of Scope
63+
64+
- Removing `vscode.TextDocument` from `IWriter`/`IReader` return types — Phase 2.1.
65+
- Introducing `JournalDocument` type — Phase 2.1.
66+
- Renaming `IDialogues` to `WorkspaceUI` — no functional value now.
67+
- `IWorkspaceEditor` / `WorkspaceEdit` seam for `Inject.ts` — editor-buffer I/O, not filesystem I/O; separate seam, separate issue.
68+
- Full constructor-injection removal of `Ctrl` — Phase 2.1, issue #208.
69+
70+
## Acceptance Criteria
71+
72+
1. `JFileType`, `JFileStat`, `IFileSystem` defined in `src/model/` with zero `vscode` imports (grep-verifiable).
73+
2. `VscodeFileSystem` passes all current integration tests unchanged (no behaviour change).
74+
3. `InMemoryFileSystem` in `src/test/in-memory-fs.ts` — zero vscode imports; used by both new test files.
75+
4. `npm test -- --grep "writer-unit|scan-entries-unit"` runs plain-Node (no Extension Host), completing < 5 s.
76+
5. `npm run compile-tests` and `npm test` pass with zero new errors.
77+
6. No raw `vscode.workspace.fs` call remains in `Writer`, `fileExists`, or `ScanEntries` (grep check).
78+
7. `Writer.createSaveLoadTextDocument` no longer calls `vscode.workspace.openTextDocument` directly.
79+
80+
## Entities / Contracts
81+
82+
### `JFileType` and `JFileStat` (new, `src/model/fs.ts`)
83+
84+
```ts
85+
export enum JFileType { Unknown = 0, File = 1, Directory = 2, SymbolicLink = 64 }
86+
export interface JFileStat {
87+
type: JFileType;
88+
ctime: number;
89+
mtime: number;
90+
size: number;
91+
}
92+
```
93+
94+
### `IFileSystem` (new, `src/model/interfaces.ts`)
95+
96+
```ts
97+
export interface IFileSystem {
98+
stat(path: string): Promise<JFileStat>;
99+
readFile(path: string): Promise<Uint8Array>;
100+
writeFile(path: string, content: Uint8Array): Promise<void>;
101+
readDirectory(path: string): Promise<[string, JFileType][]>;
102+
createDirectory(path: string): Promise<void>;
103+
delete(path: string, options?: { recursive?: boolean }): Promise<void>;
104+
}
105+
```
106+
107+
### `VscodeFileSystem` (new, `src/vscode/vscode-fs.ts`)
108+
109+
Converts `string → vscode.Uri.file(path)`. Maps `vscode.FileType ↔ JFileType` and `vscode.FileStat → JFileStat`. No logic beyond mapping; purely a type-boundary adapter.
110+
111+
### `fileExists(fs: IFileSystem, path: string): Promise<boolean>`
112+
113+
Calls `fs.stat(path)`; catches `{ code: 'FileNotFound' }` → `false`; rethrows others. No `vscode.FileSystemError` reference.
114+
115+
### `Writer` constructor change
116+
117+
```ts
118+
constructor(
119+
private config: IConfiguration,
120+
private logger: ILogger,
121+
private inject: IInject,
122+
private fs: IFileSystem, // new
123+
private ui: IDialogues, // new (was absent; openDocument delegated here)
124+
)
125+
```
126+
127+
### `ScanEntries` constructor change
128+
129+
```ts
130+
constructor(
131+
private config: IConfiguration,
132+
private logger: ILogger,
133+
private fs: IFileSystem, // new
134+
)
135+
```
136+
137+
### `JournalController` extension
138+
139+
```ts
140+
export interface JournalController {
141+
// ... existing ...
142+
fs: IFileSystem; // new
143+
}
144+
```
145+
146+
## Constraints
147+
148+
- `VscodeFileSystem` uses `vscode.Uri.file(path)` — callers pass OS paths; `extensionKind: ["workspace"]` places the extension on the remote host so OS paths resolve correctly there.
149+
- `InMemoryFileSystem` lives under `src/test/`, never in the extension bundle.
150+
- `{ code: 'FileNotFound' }` is the error shape used by `InMemoryFileSystem`; `VscodeFileSystem` re-throws `vscode.FileSystemError` as-is (its `.code` is already `'FileNotFound'`), so `fileExists` catch logic is stable.
151+
- Enum values of `JFileType` mirror `vscode.FileType` numeric values to simplify the mapping in `VscodeFileSystem`.
152+
153+
## Open Questions
154+
155+
None.
156+
157+
## Related Issues
158+
159+
- Blocks: none
160+
- Blocked by: none
161+
- Related: #208 (Phase 2.1 DI; this spec is a compatible preparatory step)

‎src/features/entries/load-note.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export class LoadNotes {
4242
public async loadNote(path: string, content: string): Promise<vscode.TextDocument> {
4343
this.ctrl.logger.trace("Entering loadNote() in features/load-note.ts for path: ", path);
4444

45-
const exists = await J.Util.fileExists(vscode.Uri.file(path));
45+
const exists = await J.Util.fileExists(this.ctrl.fs, path);
4646
if (exists) {
4747
return this.ctrl.ui.openDocument(path);
4848
}

‎src/features/entries/scan-entries.ts‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as vscode from 'vscode';
22
import * as J from '../..';
33
import * as Path from 'path';
44
import { SCOPE_DEFAULT } from '../../vscode';
5-
import { FileEntry, IConfiguration, ILogger } from '../../model';
5+
import { FileEntry, IConfiguration, IFileSystem, ILogger, JFileType } from '../../model';
66

77
export interface DecoratedQuickPickItem extends vscode.QuickPickItem {
88
parsedInput?: J.Model.Input;
@@ -25,7 +25,7 @@ export interface TimedQuickPick extends vscode.QuickPick<DecoratedQuickPickItem>
2525
export class ScanEntries {
2626

2727
private cache: Map<String, J.Model.FileEntry>;
28-
constructor(private config: IConfiguration, private logger: ILogger) {
28+
constructor(private config: IConfiguration, private logger: ILogger, private fs: IFileSystem) {
2929
this.cache = new Map();
3030
}
3131

@@ -57,7 +57,7 @@ export class ScanEntries {
5757
// for each file, check if it is an entry, a note or an attachement
5858
for (const directory of directories) {
5959
try {
60-
await vscode.workspace.fs.stat(vscode.Uri.file(directory.path));
60+
await this.fs.stat(directory.path);
6161
} catch {
6262
this.logger.error("Invalid configuration, base directory does not exist with path", directory.path);
6363
continue;
@@ -129,7 +129,7 @@ export class ScanEntries {
129129

130130
private async scanDirectory(thresholdInMs: number, callback: Function, picker: any, type: J.Model.JournalPageType, directory: J.Model.ScopeDirectory): Promise<void> {
131131
try {
132-
await vscode.workspace.fs.stat(vscode.Uri.file(directory.path));
132+
await this.fs.stat(directory.path);
133133
} catch {
134134
this.logger.error("Invalid configuration, base directory does not exist");
135135
return;
@@ -164,9 +164,9 @@ export class ScanEntries {
164164
* @param callback
165165
*/
166166
private async walkDir(dir: string, thresholdInMs: number, callback: Function): Promise<void> {
167-
let entries: [string, vscode.FileType][];
167+
let entries: [string, JFileType][];
168168
try {
169-
entries = await vscode.workspace.fs.readDirectory(vscode.Uri.file(dir));
169+
entries = await this.fs.readDirectory(dir);
170170
} catch {
171171
return; // ignore errors
172172
}
@@ -180,7 +180,7 @@ export class ScanEntries {
180180
for (const [name, type] of entries) {
181181
if (name.startsWith(".")) { continue; }
182182
const childPath = Path.join(dir, name);
183-
if (type === vscode.FileType.Directory) {
183+
if (type === JFileType.Directory) {
184184
subdirs.push(childPath);
185185
} else {
186186
files.push({ name, childPath });
@@ -190,12 +190,12 @@ export class ScanEntries {
190190
const statResults = await Promise.all(
191191
files.map(async ({ name, childPath }) => {
192192
try {
193-
const stat = await vscode.workspace.fs.stat(vscode.Uri.file(childPath));
193+
const stat = await this.fs.stat(childPath);
194194
return {
195195
path: childPath,
196196
name,
197197
updateAt: stat.mtime,
198-
accessedAt: stat.mtime, // vscode.FileStat does not expose atime
198+
accessedAt: stat.mtime,
199199
createdAt: stat.ctime
200200
} as FileEntry;
201201
} catch {
@@ -210,11 +210,11 @@ export class ScanEntries {
210210
await Promise.all(subdirs.map(d => this.walkDir(d, thresholdInMs, callback)));
211211
}
212212

213-
// deprecated — converted to async vscode.workspace.fs for remote compatibility
213+
// deprecated — use walkDir instead
214214
private async walkDirSync(dir: string, thresholdDateInMs: number, callback: Function): Promise<void> {
215-
let entries: [string, vscode.FileType][];
215+
let entries: [string, JFileType][];
216216
try {
217-
entries = await vscode.workspace.fs.readDirectory(vscode.Uri.file(dir));
217+
entries = await this.fs.readDirectory(dir);
218218
} catch {
219219
return;
220220
}
@@ -224,9 +224,9 @@ export class ScanEntries {
224224

225225
const childPath = Path.join(dir, name);
226226
try {
227-
const stat = await vscode.workspace.fs.stat(vscode.Uri.file(childPath));
227+
const stat = await this.fs.stat(childPath);
228228

229-
if (type === vscode.FileType.Directory && stat.mtime > thresholdDateInMs) {
229+
if (type === JFileType.Directory && stat.mtime > thresholdDateInMs) {
230230
await this.walkDirSync(childPath, thresholdDateInMs, callback);
231231
} else if (stat.mtime > thresholdDateInMs) {
232232
callback(new Array({

‎src/features/sync/sync-daily-links.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export class SyncDailyLinks {
5050

5151
const fullPath = Path.normalize(Path.join(pathTpl.value, fileTpl.value));
5252
const uri = vscode.Uri.file(fullPath);
53-
if (await fileExists(uri)) {
53+
if (await fileExists(this.ctrl.fs, fullPath)) {
5454
result.push(uri);
5555
}
5656
}

‎src/journal/reader.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,20 @@
1919
'use strict';
2020

2121
import * as vscode from 'vscode';
22-
import { IConfiguration, IDialogues, ILogger, IWriter, Input } from '../model';
22+
import { IConfiguration, IDialogues, IFileSystem, ILogger, IWriter, Input } from '../model';
2323
import { isNullOrUndefined, fileExists } from '../util';
2424
import { resolvePath } from './paths';
2525

2626
export class Reader {
2727
public onNotesInjected?: (doc: vscode.TextDocument, date: Date) => void;
2828

29-
constructor(private config: IConfiguration, private logger: ILogger, private writer: IWriter, private ui: IDialogues) {
30-
}
29+
constructor(
30+
private config: IConfiguration,
31+
private logger: ILogger,
32+
private writer: IWriter,
33+
private ui: IDialogues,
34+
private fs: IFileSystem,
35+
) { }
3136

3237

3338
/**
@@ -107,7 +112,7 @@ export class Reader {
107112
path: string,
108113
create: () => Promise<vscode.TextDocument>,
109114
): Promise<vscode.TextDocument> {
110-
const exists = await fileExists(vscode.Uri.file(path));
115+
const exists = await fileExists(this.fs, path);
111116
if (exists) {
112117
return this.ui.openDocument(path);
113118
}

0 commit comments

Comments
 (0)