Skip to content

Commit 71a16bf

Browse files
pajomapajoma
authored
Merge pull request #226 from pajoma/feat/225-release-pipeline
ci: GitHub Actions release pipeline — build & publish .vsix on merge to main
2 parents bda4bd3 + d758464 commit 71a16bf

5 files changed

Lines changed: 438 additions & 0 deletions

File tree

‎.github/workflows/release.yml‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
name: Release
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
8+
permissions:
9+
contents: write
10+
11+
jobs:
12+
release:
13+
runs-on: ubuntu-latest
14+
timeout-minutes: 20
15+
16+
steps:
17+
- name: Checkout
18+
uses: actions/checkout@v4
19+
20+
- name: Setup Node.js
21+
uses: actions/setup-node@v4
22+
with:
23+
node-version: "20"
24+
cache: npm
25+
26+
- name: Install dependencies
27+
run: npm install
28+
29+
- name: Build extension
30+
run: npm run compile
31+
32+
- name: Build tests
33+
run: npm run compile-tests
34+
35+
- name: Run extension tests
36+
run: xvfb-run -a npm test
37+
38+
- name: Read version
39+
id: package-version
40+
run: echo "version=$(node -p "require('./package.json').version")" >> $GITHUB_OUTPUT
41+
42+
- name: Package extension
43+
run: npx @vscode/vsce package
44+
45+
- name: Create GitHub Release
46+
uses: softprops/action-gh-release@3bb12739c298aeb8a4eeaf626c5b8d85266b0e65 # v2
47+
with:
48+
tag_name: v${{ steps.package-version.outputs.version }}
49+
name: vscode-journal v${{ steps.package-version.outputs.version }}
50+
files: "*.vsix"
51+
generate_release_notes: true

‎docs/plans/2026-05-19-202-decouple-test-config.md‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
# Plan: Decouple Tests from Global VS Code Configuration State — #202
2+
3+
## Reference spec
4+
[docs/specs/2026-05-19-202-decouple-test-config.md](../specs/2026-05-19-202-decouple-test-config.md)
5+
6+
## Approach
7+
Introduce a thin `IWorkspaceConfigReader` interface (single `get<T>` overload pair) so `Configuration` and `Ctrl` can be constructed with a plain object instead of `vscode.WorkspaceConfiguration`. Add `FakeWorkspaceConfig` test double. Refactor 14 test files to build `Ctrl` from `FakeWorkspaceConfig({…})` and delete all `original*` state variables + `afterEach` blocks that only reset config state.
8+
9+
Trade-off: `FakeWorkspaceConfig` is a flat dictionary lookup. `Configuration` exclusively reads top-level keys (confirmed: `base`, `locale`, `ext`, `scopes`, `patterns`, `templates`, `weeklySync`, `navigation.mode`, `dev`, `openInNewEditorGroup`, `syntax-highlighting`, `entryGranularity`, `tpl-*`) — no nested path reads — so flat lookup is safe and no special path parser is needed.
10+
11+
## Steps
12+
13+
### 1 — Add `IWorkspaceConfigReader` to `src/model/interfaces.ts`
14+
15+
Add after the existing interface declarations:
16+
17+
```typescript
18+
export interface IWorkspaceConfigReader {
19+
get<T>(section: string): T | undefined;
20+
get<T>(section: string, defaultValue: T): T;
21+
}
22+
```
23+
24+
**Why first:** `Configuration` and `Ctrl` import from `../model`; the interface must exist before their constructors are changed. `vscode.WorkspaceConfiguration` satisfies this structurally — TypeScript confirms at compile time that no cast is needed in `Startup.ts`.
25+
26+
### 2 — Update `Configuration` constructor (`src/vscode/conf.ts`)
27+
28+
Change constructor signature:
29+
```typescript
30+
// Before
31+
constructor(vscodeConfig: vscode.WorkspaceConfiguration) {
32+
this._config = vscodeConfig;
33+
}
34+
// After
35+
import { IWorkspaceConfigReader } from '../model';
36+
constructor(private readonly config: IWorkspaceConfigReader) {}
37+
```
38+
39+
Note: the field is already named `config` throughout `conf.ts` — only the type annotation changes. Remove `vscode.WorkspaceConfiguration` from the constructor parameter type; keep all `this.config.get<…>(…)` call sites unchanged.
40+
41+
**Why:** This is the primary seam. After this change, `Configuration` no longer depends on a VS Code API type.
42+
43+
### 3 — Update `Ctrl` constructor (`src/util/controller.ts`)
44+
45+
```typescript
46+
// Before
47+
constructor(vscodeConfig: vscode.WorkspaceConfiguration) {
48+
this._config = new Configuration(vscodeConfig);
49+
}
50+
// After
51+
import { IWorkspaceConfigReader } from '../model';
52+
constructor(configSource: IWorkspaceConfigReader) {
53+
this._config = new Configuration(configSource);
54+
}
55+
```
56+
57+
**Why:** `Ctrl` is the entry point tests use. Changing its constructor to `IWorkspaceConfigReader` lets tests pass `new FakeWorkspaceConfig(…)` directly.
58+
59+
### 4 — Verify `npm run compile` (structural typing gate)
60+
61+
```bash
62+
npm run compile
63+
```
64+
65+
Must exit 0. If `Startup.ts` passes `vscode.workspace.getConfiguration('journal')` to `new Ctrl(…)`, TypeScript will silently accept it — `vscode.WorkspaceConfiguration` structurally satisfies `IWorkspaceConfigReader`. Zero changes expected in `Startup.ts`. Any compile error at this step means `IWorkspaceConfigReader` is missing a method that `Configuration` calls on `this.config`.
66+
67+
### 5 — Create `FakeWorkspaceConfig` (`src/test/fake-workspace-config.ts`)
68+
69+
```typescript
70+
import { IWorkspaceConfigReader } from '../model';
71+
72+
export class FakeWorkspaceConfig implements IWorkspaceConfigReader {
73+
constructor(private readonly settings: Record<string, unknown> = {}) {}
74+
75+
get<T>(section: string, defaultValue?: T): T | undefined {
76+
return (section in this.settings
77+
? this.settings[section]
78+
: defaultValue) as T | undefined;
79+
}
80+
}
81+
```
82+
83+
**Why:** Single implementation used by all 14 test files. Constructor accepts a plain object — tests declare their needed settings inline with no async I/O.
84+
85+
### 6 — Refactor all 14 test files
86+
87+
For each file listed below, apply the same mechanical change:
88+
89+
**Pattern A — module-level `beforeEach` with single `ctrl` per suite:**
90+
```typescript
91+
// Before (in beforeEach / before)
92+
const config = vscode.workspace.getConfiguration('journal');
93+
await config.update('base', tmpBase, vscode.ConfigurationTarget.Workspace);
94+
const refreshed = vscode.workspace.getConfiguration('journal');
95+
ctrl = new J.Util.Ctrl(refreshed);
96+
97+
// After
98+
ctrl = new J.Util.Ctrl(new FakeWorkspaceConfig({ base: tmpBase }));
99+
```
100+
101+
**Pattern B — per-describe `beforeEach` with `original*` save/restore:**
102+
```typescript
103+
// Before
104+
let originalBase: string | undefined;
105+
beforeEach(async () => {
106+
const config = vscode.workspace.getConfiguration('journal');
107+
originalBase = config.get<string>('base');
108+
await config.update('base', tmpBase, vscode.ConfigurationTarget.Workspace);
109+
ctrl = new J.Util.Ctrl(vscode.workspace.getConfiguration('journal'));
110+
});
111+
afterEach(async () => {
112+
await config.update('base', originalBase, vscode.ConfigurationTarget.Workspace);
113+
});
114+
115+
// After
116+
beforeEach(() => {
117+
ctrl = new J.Util.Ctrl(new FakeWorkspaceConfig({ base: tmpBase }));
118+
});
119+
// afterEach DELETED (no global state to restore)
120+
// originalBase variable DELETED
121+
```
122+
123+
**Caution for `commands-prev-next.test.ts`:** some `afterEach` blocks also restore `vscode.window.showInformationMessage`. Keep those restores — only remove the `config.update` lines and the `original*` config variables.
124+
125+
**Files to refactor:**
126+
1. `commands-inject.test.ts`
127+
2. `week-input.test.ts`
128+
3. `notes-sync.test.ts`
129+
4. `issue-168-entry-granularity.test.ts`
130+
5. `commands-entry.test.ts`
131+
6. `phase1-regression.test.ts`
132+
7. `commands-prev-next.test.ts`
133+
8. `read-templates.test.ts`
134+
9. `issue-185-weekly-sync.test.ts`
135+
10. `input.test.ts`
136+
11. `commands-note.test.ts`
137+
12. `issue-51-remote-create.test.ts`
138+
13. `commands-weekly.test.ts`
139+
14. `scan-entries-cache.test.ts`
140+
141+
For each file, also remove any `import … ConfigurationTarget` from the VS Code import once all uses are gone.
142+
143+
### 7 — Verify `npm run compile-tests` and `npm test`
144+
145+
```bash
146+
npm run compile-tests # tsc — catches type errors in refactored tests
147+
npm test # full suite — confirms all 14 suites pass
148+
```
149+
150+
Both must exit 0 with zero errors.
151+
152+
## Test scenarios
153+
154+
**T1 — Interface satisfies structurally**
155+
`npx tsc --noEmit` on `src/vscode/startup.ts` without changing it → zero errors. Proves `vscode.WorkspaceConfiguration` satisfies `IWorkspaceConfigReader`.
156+
157+
**T2 — FakeWorkspaceConfig key lookup**
158+
`new FakeWorkspaceConfig({ base: '/tmp/x' }).get('base')` returns `'/tmp/x'`.
159+
`new FakeWorkspaceConfig({}).get('base', 'fallback')` returns `'fallback'`.
160+
Verifiable in `node -e` without VS Code.
161+
162+
**T3 — No live config mutation in tests**
163+
`grep -rn "config\.update\|ConfigurationTarget" src/test/suite/` → zero results.
164+
165+
**T4 — No getConfiguration in tests**
166+
`grep -rn "vscode\.workspace\.getConfiguration" src/test/suite/` → zero results.
167+
168+
**T5 — No dead original\* variables**
169+
`grep -rn "originalBase\|originalScopes\|originalMode\|originalExt" src/test/suite/` → zero results.
170+
171+
**T6 — Compile clean**
172+
`npm run compile` → exit 0. `npm run compile-tests` → exit 0.
173+
174+
**T7 — All tests green**
175+
`npm test` → all suites pass, same count as before.
176+
177+
## Dependencies
178+
- **#198 must merge first.** After #198 lands, `TemplateService` is behind `IRawConfigProvider`. This plan then completes the isolation story: `FakeWorkspaceConfig` enables constructing `Configuration` without VS Code, which in turn means tests can supply a testable `IRawConfigProvider` (i.e., a real `Configuration(fakeConfig)`) without the Extension Host.
179+
180+
## Risk
181+
**Risk:** A test currently uses `config.update` mid-test (not just in `beforeEach`) to change settings between assertions. Switching to `FakeWorkspaceConfig` would require constructing a new `ctrl` rather than mutating settings.
182+
**Mitigation:** Step 6 audit — read each test file before refactoring. If a mid-test `config.update` is found, split the test into two tests each with its own `ctrl`.
183+
184+
**Risk:** `commands-prev-next.test.ts` `afterEach` blocks contain both config resets AND `showInformationMessage` restores. Partial deletion could leave orphaned `original*` variables.
185+
**Mitigation:** T5 grep catches any lingering `original*` variables. Audit `afterEach` content before deleting.
186+
187+
## Rollback
188+
`git revert <merge-commit>` — no schema, no migration, no data. Pure TypeScript + test change.

‎docs/plans/2026-05-19-225-release-pipeline.md‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# Plan: GitHub Actions Release Pipeline (#225)
2+
3+
**Reference spec:** [docs/specs/2026-05-19-225-release-pipeline.md](../specs/2026-05-19-225-release-pipeline.md)
4+
5+
## Approach
6+
7+
Single-job workflow in `.github/workflows/release.yml`. Gate: full build + test must pass before packaging or releasing. Use `softprops/action-gh-release@v2` with `generate_release_notes: true`. Version read from `package.json` via `node -p`. No matrix — single Node 20 run is sufficient for a packaging job (not correctness testing).
8+
9+
Trade-off: single job means no parallelism, but avoids artifact-passing complexity. Reviewer comment noted `upload-artifact` for multi-job design — deferred as out of scope.
10+
11+
## Steps
12+
13+
1. **Create `.github/workflows/release.yml`**
14+
- Trigger: `push` to `main` only
15+
- `permissions: contents: write`
16+
- Single job `release` on `ubuntu-latest`
17+
- Steps in order:
18+
1. `actions/checkout@v4`
19+
2. `actions/setup-node@v4` — node 20, npm cache
20+
3. `npm install`
21+
4. `npm run compile`
22+
5. `npm run compile-tests`
23+
6. `xvfb-run -a npm test`
24+
7. Read version: `echo "version=$(node -p "require('./package.json').version")" >> $GITHUB_OUTPUT` with `id: package-version`
25+
8. `npx @vscode/vsce package`
26+
9. `softprops/action-gh-release@v2` — tag `v${{ steps.package-version.outputs.version }}`, name `vscode-journal v<version>`, `files: "*.vsix"`, `generate_release_notes: true`
27+
28+
2. **Verify no conflicts with existing `ci.yml`**
29+
- `ci.yml` triggers on `main` too — both will run on merge; that is intentional and acceptable (CI validates, release publishes)
30+
- No step overlap that could cause race conditions (releases are idempotent via `softprops`)
31+
32+
## Test Scenarios
33+
34+
| # | Scenario | Type | How to verify |
35+
|---|----------|------|---------------|
36+
| 1 | Merge to `main` with passing tests | e2e | Workflow green, GitHub Release `v<version>` created, `.vsix` attached |
37+
| 2 | Merge to `main` with failing test | e2e | Workflow red at test step, no release created |
38+
| 3 | Merge to `main` with compile error | e2e | Workflow red at compile step, no release |
39+
| 4 | Push to `develop` | e2e | Workflow does NOT trigger |
40+
| 5 | Duplicate version push (same `package.json` version) | e2e | `softprops` updates existing release, no duplicate tag error |
41+
42+
Scenarios 2–5 can be validated by reviewing workflow trigger config and `softprops` docs rather than requiring live test runs.
43+
44+
## Dependencies
45+
46+
None. No other PRs or issues must land first.
47+
48+
## Risk
49+
50+
- `xvfb-run` flakiness on GitHub runners — same risk as existing `ci.yml`; not new
51+
- `softprops/action-gh-release@v2` pinned to `v2` (floating major) — acceptable for internal tooling; can pin to SHA if policy requires
52+
53+
## Rollback
54+
55+
Delete `.github/workflows/release.yml`. No code changes, no DB migrations. Fully reversible.

‎docs/specs/2026-05-19-202-decouple-test-config.md‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
# Spec: Decouple Tests from Global VS Code Configuration State — #202
2+
3+
## Goal
4+
Make `Configuration` and `Ctrl` constructible without a live `vscode.WorkspaceConfiguration`, so tests supply settings as plain objects rather than mutating VS Code's global workspace config.
5+
6+
## Why now
7+
14 test files call `vscode.workspace.getConfiguration('journal')` and `config.update(…, ConfigurationTarget.Workspace)` to inject test values. This approach: (a) mutates global VS Code state, (b) is slow (each `update` is an async I/O write), and (c) creates test-order coupling — a test that crashes before teardown can leave dirty settings that break the next test. Must land after #198: once `TemplateService` lives behind `IRawConfigProvider`, the only remaining VS Code coupling in the resolution layer is the `Configuration` constructor seam. Fixing that seam makes both `TemplateService` and `Configuration` independently testable without the Extension Host.
8+
9+
## In scope
10+
- New `IWorkspaceConfigReader` interface (two `get<T>` overloads only) in `src/model/interfaces.ts`
11+
- `Configuration` constructor changed from `vscode.WorkspaceConfiguration` → `IWorkspaceConfigReader`
12+
- `Ctrl` constructor changed from `vscode.WorkspaceConfiguration` → `IWorkspaceConfigReader` (structural subtype — production callers in `Startup` pass `vscode.WorkspaceConfiguration` unchanged)
13+
- `FakeWorkspaceConfig` test double in `src/test/fake-workspace-config.ts`
14+
- Refactor all 14 affected test files: replace `vscode.workspace.getConfiguration` + `config.update` setup with `new FakeWorkspaceConfig({ … })`
15+
- Remove `ConfigurationTarget` imports from all test files
16+
17+
## Out of scope
18+
- Writing new headless `TemplateService` unit tests (separate issue)
19+
- Changes to `IConfiguration` interface
20+
- Infrastructure seams / `IFileSystem` (#212)
21+
- Further DI decomposition of `Ctrl` (#208)
22+
- Any change to `Startup.ts` or production extension code
23+
24+
## Acceptance criteria
25+
1. `IWorkspaceConfigReader` declared in `src/model/interfaces.ts`; `vscode.WorkspaceConfiguration` satisfies it structurally (no cast needed)
26+
2. `Configuration` constructor signature: `constructor(raw: IWorkspaceConfigReader)`
27+
3. `Ctrl` constructor signature: `constructor(configSource: IWorkspaceConfigReader)`
28+
4. `FakeWorkspaceConfig` in `src/test/fake-workspace-config.ts`; constructor accepts `Record<string, unknown>`
29+
5. Zero `vscode.workspace.getConfiguration` + `config.update` calls remain in any `src/test/suite/**` file
30+
6. Zero `ConfigurationTarget` imports remain in any `src/test/suite/**` file
31+
7. `npm run compile` exits 0, `npm test` passes (all existing tests green)
32+
33+
## Entities / contracts
34+
35+
### `IWorkspaceConfigReader` (new, `src/model/interfaces.ts`)
36+
```typescript
37+
export interface IWorkspaceConfigReader {
38+
get<T>(section: string): T | undefined;
39+
get<T>(section: string, defaultValue: T): T;
40+
}
41+
```
42+
`vscode.WorkspaceConfiguration` satisfies this structurally. No cast needed in `Startup.ts`.
43+
44+
### `FakeWorkspaceConfig` (new, `src/test/fake-workspace-config.ts`)
45+
```typescript
46+
export class FakeWorkspaceConfig implements IWorkspaceConfigReader {
47+
constructor(private readonly settings: Record<string, unknown> = {}) {}
48+
get<T>(section: string, defaultValue?: T): T | undefined {
49+
return (section in this.settings ? this.settings[section] : defaultValue) as T | undefined;
50+
}
51+
}
52+
```
53+
54+
### `Configuration` constructor (changed, `src/vscode/conf.ts`)
55+
```typescript
56+
// Before: constructor(vscodeConfig: vscode.WorkspaceConfiguration)
57+
// After: constructor(private readonly config: IWorkspaceConfigReader)
58+
```
59+
Import `IWorkspaceConfigReader` from `../model`. Remove the `vscode.WorkspaceConfiguration` reference from this constructor.
60+
61+
### `Ctrl` constructor (changed, `src/util/controller.ts`)
62+
```typescript
63+
// Before: constructor(vscodeConfig: vscode.WorkspaceConfiguration)
64+
// After: constructor(configSource: IWorkspaceConfigReader)
65+
```
66+
67+
### Test setup pattern (changed in all 14 test files)
68+
```typescript
69+
// Before
70+
const config = vscode.workspace.getConfiguration('journal');
71+
await config.update('base', tmpBase, vscode.ConfigurationTarget.Workspace);
72+
const refreshed = vscode.workspace.getConfiguration('journal');
73+
const ctrl = new J.Util.Ctrl(refreshed);
74+
75+
// After
76+
const ctrl = new J.Util.Ctrl(new FakeWorkspaceConfig({ base: tmpBase }));
77+
```
78+
79+
## Affected test files (14)
80+
`commands-inject.test.ts`, `week-input.test.ts`, `notes-sync.test.ts`,
81+
`issue-168-entry-granularity.test.ts`, `commands-entry.test.ts`, `phase1-regression.test.ts`,
82+
`commands-prev-next.test.ts`, `read-templates.test.ts`, `issue-185-weekly-sync.test.ts`,
83+
`input.test.ts`, `commands-note.test.ts`, `issue-51-remote-create.test.ts`,
84+
`commands-weekly.test.ts`, `scan-entries-cache.test.ts`
85+
86+
## Open questions
87+
None.
88+
89+
## Related issues
90+
- **Blocked by #198** (extract TemplateService — `IRawConfigProvider` seam must land first; #202 then completes the test isolation story)
91+
- Related to #212 (infrastructure seams / `IFileSystem`)
92+
- Related to #208 (constructor injection)

0 commit comments

Comments
 (0)