Skip to content

Commit c9f522a

Browse files
trangdoan982claude
andcommitted
Harden e2e test setup: vault selection, page detection, and review fixes
- Fix vault selection by manipulating obsidian.json instead of unsupported --vault flag - Find correct workspace page via DOM query instead of assuming pages()[0] - Load .env via dotenv for OBSIDIAN_TEST_VAULT configuration - Replace blind waitForTimeout calls with proper selector/function waits - Add ensureVaultWithPlugin for safe setup on existing vaults - Add CDP connection retry logic - Scope eslint-disables to individual page.evaluate blocks - Use crypto.randomBytes for proper hex vault IDs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 529c984 commit c9f522a

10 files changed

Lines changed: 558 additions & 42 deletions

File tree

apps/obsidian/e2e/NOTES.md

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
# E2E Testing for Obsidian Plugin — Notes
2+
3+
## Approaches Considered
4+
5+
### Option 1: Playwright `electron.launch()`
6+
7+
The standard Playwright approach for Electron apps — point `executablePath` at the binary and let Playwright manage the process lifecycle.
8+
9+
**Pros:**
10+
11+
- First-class Playwright API — `app.evaluate()` runs code in the main process, not just renderer
12+
- Automatic process lifecycle management (launch, close, cleanup)
13+
- Access to Electron-specific APIs (e.g., `app.evaluate(() => process.env)`)
14+
- Well-documented, widely used for Electron testing
15+
16+
**Cons:**
17+
18+
- **Does not work with Obsidian.** Obsidian's executable is a launcher that loads an `.asar` package (`obsidian-1.11.7.asar`) and forks a new Electron process. Playwright connects to the initial process, which exits, causing `kill EPERM` and connection failures.
19+
- No workaround without modifying Obsidian's startup or using a custom Electron shell
20+
21+
**Verdict:** Not viable for Obsidian.
22+
23+
---
24+
25+
### Option 2: CDP via `chromium.connectOverCDP()` (chosen)
26+
27+
Launch Obsidian as a subprocess with `--remote-debugging-port=9222`, then connect via Chrome DevTools Protocol.
28+
29+
**Pros:**
30+
31+
- Works with Obsidian's forked process architecture — the debug port is inherited by the child process
32+
- Full access to renderer via `page.evaluate()` — Obsidian's global `app` object is available
33+
- Keyboard/mouse interaction works normally
34+
- Can take screenshots, traces, and use all Playwright assertions
35+
- Process is managed explicitly — clear control over startup and teardown
36+
37+
**Cons:**
38+
39+
- No main process access (can't call Electron APIs directly, only renderer-side `window`/`app`)
40+
- Must manually manage process lifecycle (spawn, pkill, port polling)
41+
- Fixed debug port (9222) means tests can't run in parallel across multiple Obsidian instances without port management
42+
- Port polling adds ~2-5s startup overhead
43+
- `pkill -f Obsidian` in setup is aggressive — kills ALL Obsidian instances, not just test ones
44+
45+
**Verdict:** Works well for PoC. Sufficient for single-worker CI/local testing.
46+
47+
---
48+
49+
### Option 3: Obsidian's built-in plugin testing (not explored)
50+
51+
Obsidian has no official testing framework. Some community approaches exist (e.g., `obsidian-jest`, hot-reload-based testing), but none are mature or maintained.
52+
53+
**Verdict:** Not a real option today.
54+
55+
---
56+
57+
## What We Learned
58+
59+
### Obsidian internals accessible via `page.evaluate()`
60+
61+
- `app.plugins.plugins["@discourse-graph/obsidian"]` — check plugin loaded
62+
- `app.vault.getMarkdownFiles()` — list files
63+
- `app.vault.read(file)` — read file content
64+
- `app.vault.create(name, content)` — create files
65+
- `app.workspace.openLinkText(path, "", false)` — open a file in the editor
66+
- `app.commands.executeCommandById(id)` — could execute commands directly (alternative to command palette UI)
67+
68+
### Plugin command IDs
69+
70+
Commands are registered with IDs like `@discourse-graph/obsidian:create-discourse-node`. The command palette shows them as "Discourse Graph: Create discourse node".
71+
72+
### Modal DOM structure
73+
74+
The `ModifyNodeModal` renders React inside Obsidian's `.modal-container`:
75+
76+
- Node type: `<select>` element (`.modal-container select`)
77+
- Content: `<input type="text">` (`.modal-container input[type='text']`)
78+
- Confirm: `<button class="mod-cta">`
79+
80+
### Vault configuration
81+
82+
Minimum config for plugin to load:
83+
84+
- `.obsidian/community-plugins.json``["@discourse-graph/obsidian"]`
85+
- `.obsidian/app.json``{"livePreview": true}` (restricted mode must be off, but this is handled by Obsidian detecting the plugins dir)
86+
- Plugin files (`main.js`, `manifest.json`, `styles.css`) in `.obsidian/plugins/@discourse-graph/obsidian/`
87+
88+
---
89+
90+
## Proposal: Full Agentic Testing Flow
91+
92+
### Goal
93+
94+
AI coding agents (Cursor, Claude Code) can run `pnpm test:e2e` after making changes to automatically verify features work end-to-end. The test suite should be comprehensive enough to catch regressions, fast enough to run frequently, and deterministic enough to trust the results.
95+
96+
### Phase 1: Stabilize the PoC (current state + hardening)
97+
98+
**Isolation improvements:**
99+
100+
- Use a unique temp directory per test run (`os.tmpdir()`) instead of a fixed `test-vault/` path to avoid stale state
101+
- Use a random debug port to allow parallel runs
102+
- Replace `pkill -f Obsidian` with tracking the specific child PID — parse it from `lsof -i :<port>` after launch
103+
- Add a global setup/teardown in Playwright config to manage the single Obsidian instance across all tests
104+
105+
**Reliability improvements:**
106+
107+
- Replace `waitForTimeout()` calls with proper waitFor conditions (e.g., `waitForSelector`, `waitForFunction`)
108+
- Add retry logic for CDP connection (currently fails hard on timeout)
109+
- Add a `test.beforeEach` that resets vault state (delete all non-config files) instead of full vault recreation
110+
111+
### Phase 2: Expand test coverage
112+
113+
**Core plugin features to test:**
114+
115+
- Create each discourse node type (Question, Claim, Evidence, Source)
116+
- Verify frontmatter (`nodeTypeId`) is set correctly
117+
- Verify file naming conventions (e.g., `QUE - `, `CLM - `, `EVD - `, `SRC - `)
118+
- Open node type menu via hotkey (`Cmd+\`)
119+
- Discourse context view toggle
120+
- Settings panel opens and renders
121+
122+
**Vault-level tests:**
123+
124+
- Create multiple nodes and verify they appear in file explorer
125+
- Verify node format regex matching (files follow the format pattern)
126+
127+
**Use `app.commands.executeCommandById()` as the primary way to trigger commands** — faster, more reliable, and avoids flaky command palette typing. Reserve command palette tests for testing the palette itself.
128+
129+
### Phase 3: Agentic integration
130+
131+
**For agents to use the tests effectively:**
132+
133+
1. **Fast feedback loop** — Tests should complete in <30s total. Current PoC is ~14s for 2 tests, which is good. Keep Obsidian running between test files using Playwright's `globalSetup`/`globalTeardown`.
134+
135+
2. **Clear error messages** — When a test fails, the agent needs to understand WHY. Add descriptive assertion messages:
136+
137+
```ts
138+
expect(
139+
pluginLoaded,
140+
"Plugin should be loaded — check dist/ is built and plugin ID matches manifest.json",
141+
).toBe(true);
142+
```
143+
144+
3. **Screenshot-on-failure for visual debugging** — Already configured. Consider adding `page.screenshot()` at key checkpoints even on success, so agents can visually verify state.
145+
146+
4. **Test file organization** — One test file per feature area:
147+
148+
```
149+
e2e/tests/
150+
├── plugin-load.spec.ts # Plugin loads, settings exist
151+
├── node-creation.spec.ts # Create each node type
152+
├── command-palette.spec.ts # Command palette interactions
153+
├── discourse-context.spec.ts # Context view, relations
154+
└── settings.spec.ts # Settings panel
155+
```
156+
157+
5. **CI integration** — Run in GitHub Actions with a macOS runner. Obsidian would need to be pre-installed on the runner (or downloaded in a setup step). This is the biggest open question — Obsidian doesn't have a headless mode, so CI would need `xvfb` or a virtual display.
158+
159+
6. **Agent-executable test commands:**
160+
```bash
161+
pnpm test:e2e # run all tests
162+
pnpm test:e2e -- --grep "node creation" # run specific tests
163+
pnpm test:e2e:ui # interactive Playwright UI (for humans)
164+
```
165+
166+
### Phase 4: Advanced (future)
167+
168+
- **Visual regression testing** — Compare screenshots against baselines to catch UI regressions
169+
- **Obsidian version matrix** — Test against multiple Obsidian versions (download different `.asar` files)
170+
- **Headless mode wrapper** — Investigate running Obsidian with `--disable-gpu --headless` flags (may not work due to Obsidian's renderer requirements)
171+
- **Test data fixtures** — Pre-built vaults with specific node/relation configurations for testing complex scenarios
172+
- **Performance benchmarks** — Measure plugin load time, command execution time
173+
174+
### Open Questions
175+
176+
1. **CI runner setup** — How to install Obsidian on GitHub Actions macOS runners? Is there a `.dmg` download URL that's stable? Or do we cache the `.app` bundle?
177+
2. **Obsidian updates** — Obsidian auto-updates the `.asar`. Should tests pin a specific version? How to prevent auto-update during test runs?
178+
3. **Multiple vaults** — Obsidian tracks known vaults globally. Test vaults may accumulate in Obsidian's vault list. Need cleanup strategy.
179+
4. **Restricted mode** — The PoC doesn't explicitly disable restricted mode via config. The plugin loads because the `community-plugins.json` file is present, but a fresh Obsidian install might prompt the user to enable community plugins. Need to investigate if there's a config flag to skip this.

apps/obsidian/e2e/helpers/commands.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@ export const executeCommand = async (
88
page: Page,
99
commandId: string,
1010
): Promise<void> => {
11+
/* eslint-disable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access */
1112
await page.evaluate((id) => {
1213
// @ts-expect-error - Obsidian's global `app` is available at runtime
1314
app.commands.executeCommandById(`@discourse-graph/obsidian:${id}`);
1415
}, commandId);
15-
await page.waitForTimeout(500);
16+
/* eslint-enable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access */
1617
};
1718

1819
/**
@@ -24,20 +25,24 @@ export const executeCommandViaPalette = async (
2425
commandLabel: string,
2526
): Promise<void> => {
2627
await page.keyboard.press("Meta+p");
27-
await page.waitForTimeout(500);
28+
await page.waitForSelector(".prompt-input", { timeout: 10_000 });
2829

2930
await page.keyboard.type(commandLabel, { delay: 30 });
30-
await page.waitForTimeout(500);
31+
await page.waitForSelector(".suggestion-item", { timeout: 10_000 });
3132

3233
await page.keyboard.press("Enter");
33-
await page.waitForTimeout(1_000);
34+
await page.waitForSelector(".prompt-container", {
35+
state: "hidden",
36+
timeout: 10_000,
37+
});
3438
};
3539

3640
/**
3741
* Ensure an active editor exists by creating and opening a scratch file.
3842
* Required before running editorCallback commands like "Create discourse node".
3943
*/
4044
export const ensureActiveEditor = async (page: Page): Promise<void> => {
45+
/* eslint-disable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */
4146
await page.evaluate(async () => {
4247
// @ts-expect-error - Obsidian's global `app` is available at runtime
4348
const vault = app.vault;
@@ -46,5 +51,6 @@ export const ensureActiveEditor = async (page: Page): Promise<void> => {
4651
// @ts-expect-error - Obsidian's global `app` is available at runtime
4752
await app.workspace.openLinkText(file.path, "", false);
4853
});
49-
await page.waitForTimeout(1_000);
54+
/* eslint-enable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */
55+
await page.waitForSelector(".cm-editor", { timeout: 10_000 });
5056
};

0 commit comments

Comments
 (0)