Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| const apiPort = opts.apiPort ?? 9200; | ||
| // Wipe any prior run's data so each suite starts clean. | ||
| await rm(dkgHome, { recursive: true, force: true }).catch(() => {}); |
There was a problem hiding this comment.
🔴 Bug: This unconditionally wipes <repo>/.devnet/node1 before every run. That path is also used by local devnet tooling, so pnpm test:ui can delete a developer's existing node state/auth token and interfere with any concurrent local daemon. Use a per-run temp DKG_HOME instead, and point the Vite proxy/token injection at that temp location rather than reusing the shared repo devnet directory.
| use: { | ||
| baseURL: `http://localhost:${PORT}/ui/`, | ||
| // Run headed locally so you can see the browser; CI keeps it headless. | ||
| headless: false, |
There was a problem hiding this comment.
🔴 Bug: headless: false is applied in CI too, despite the comment. On a standard headless Linux runner this makes playwright test fail unless the job is wrapped in Xvfb, and this PR does not add that. Gate this with !CI or move it to a local-only override so the suite still runs in CI/containers.
| headers: { Authorization: `Bearer ${daemon.authToken}` }, | ||
| body: form, | ||
| }); | ||
| const importJson = (await importResp.json().catch(() => ({}))) as { |
There was a problem hiding this comment.
🟡 Issue: The file-import seed ignores importResp.ok and silently returns empty assertionUri/fileHash on failure. That makes global setup succeed even when /import-file is broken, which hides exactly the fixture regression this harness is supposed to catch. Fail fast here (including the response body) before returning SeedState.
| // now we click the write-backed assertion `qa-seed-doc`; the modal | ||
| // opens and we exercise its overlay/close/loading states. Once the | ||
| // daemon bug is fixed, switch the click to `seed.fileAssertionName`. | ||
| const link = page.locator('.v10-assertion-item-name', { hasText: seed.assertionName }); |
There was a problem hiding this comment.
🟡 Issue: This spec opens the write-backed qa-seed-doc assertion instead of the imported file fixture, so it never exercises the actual file-preview branches (<iframe>, <img>, <pre>, download CTA). As written it can stay green while real file preview rendering is broken. Wire it to seed.fileAssertionName (or open FilePreviewModal directly with that assertion) and assert one of the real preview surfaces.
|
|
||
| test('participating ⤑ toggle keeps the project in the tree', async ({ leftPanel, seed }) => { | ||
| await leftPanel.waitForReady(); | ||
| await leftPanel.toggleParticipating(seed.contextGraphName); |
There was a problem hiding this comment.
🟡 Issue: This assertion only proves the project is still somewhere in the tree. If the move toggle becomes a no-op, the test still passes, so it doesn't protect the My/Participating transfer behavior this case claims to cover. Assert the section change explicitly, or at least that the row leaves its original section after the click.
Brings the Playwright suite back to green against the post-merge UI:
Sidebar refactor (PR #088dbc17 / PR #2a4f4c66) flattened project rows
and removed Memory Stack from the tree. Layer access moved from nested
sidebar items to the LayerSwitcher inside ProjectView. Page objects
and specs updated:
- left-panel.po: drop clickLayer / clickMemoryStack / toggleParticipating;
add openProject + getIntegrationRows; replace oraclePlaceholder with
text-anchored oracleEmptyState.
- project-view.po: include the new "query" layer and a clickImport()
helper for the LayerSwitcher action bar.
- header.po: expose the new OriginTrail wordmark / brand lockup.
- selectors.ts: add header.otBrand / header.otWordmark and
leftPanel.integrationItem; drop dead oraclePlaceholder / moveBtn.
Spec updates:
- left-navigation: drop assertions on the removed Memory Stack row,
chevron toggle, layer expansion, and ⤑ participating toggle. Rewrite
the Context Oracle test to accept either a populated public-CG list
or the new empty-state copy, with a hard guard against the v9
"coming soon" placeholder regressing.
- tab-management: drop layer-as-tab tests; add a positive guard that
LayerSwitcher swaps the view in-place without spawning a tab.
- header: assert the OriginTrail "powered by" wordmark + aria-label.
- import-files / project-view / file-preview / memory-layers: route
through projectView.switchLayer() / projectView.clickImport().
file-preview and memory-layers spec files removed — the components
they test (FilePreviewModal mounted only in MemoryLayerView, the
standalone Memory Stack page) are unreachable in v10.
New coverage for v10 features (PR #326):
- query-catalog.spec.ts (11 tests): ⟐ Query layer activation, the
built-in catalog (Whole graph: All triples / Graphs / Types), default
SPARQL prefilled, Run / Save / Reset buttons, catalog-query loading
into the textarea, Save panel validation (Name required), Reset
restores the default, and a guard that switching layers does not
open a new center-panel tab.
- sidebar-integrations.spec.ts (5 tests): live Hermes / OpenClaw row
rendering with status dots from /api/local-agent-integrations, the
"No agents detected." empty state, removal of the legacy Obsidian
placeholder. Pre-seeds dkg-journey-stage=2 in localStorage so the
section's stage>=1 gate is satisfied without going through
CreateProjectModal.
Suite result locally: 289 passed / 12 skipped / 0 failed in 2.7m.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| headers: { Authorization: `Bearer ${daemon.authToken}` }, | ||
| body: form, | ||
| }); | ||
| const importJson = (await importResp.json().catch(() => ({}))) as { |
There was a problem hiding this comment.
🔴 Bug: this swallows import-file failures and keeps global setup green with an invalid seed. If the endpoint returns 4xx/5xx, assertionUri/fileHash become empty strings and later specs fail far away from the real cause. Mirror the checks above and throw on non-OK before returning the seed state.
| const resultsTable = page.locator('.v10-mlv-table'); | ||
| const noResults = page.getByText('No results for this query.'); | ||
| const status = page.locator('.v10-mlv-status'); | ||
| const someVisible = await Promise.race([ |
There was a problem hiding this comment.
🔴 Bug: counting any .v10-mlv-status as success makes this pass while the query is still stuck on Loading... or has errored immediately. That means a broken Run flow can still go green here. Wait for loading to clear, then assert on a real terminal state (results table or explicit empty state), and keep error handling in a separate failure-path test if needed.
| * the import entry-point moved out of the sidebar tree and into the | ||
| * project view's own action bar. | ||
| */ | ||
| async clickImport() { |
There was a problem hiding this comment.
🟡 Issue: clickImport() is already defined earlier in this class, so this second definition silently overrides the first one. That leaves dead code behind and makes future edits easy to apply to the wrong implementation. Keep a single clickImport() method with the scoped selector you want.
| // Either disabled (validation caught it) or enabled (submit will hit | ||
| // server-side validation). Both are valid behaviours. | ||
| const disabled = await createProjectModal.isSubmitDisabled(); | ||
| expect([true, false]).toContain(disabled); |
There was a problem hiding this comment.
🟡 Issue: expect([true, false]).toContain(disabled) can never fail, so this spec doesn't actually protect the punctuation-only-name edge case it describes. Either assert the current expected behavior, or submit and verify the visible outcome (validation error vs. graceful handling) so this catches regressions.
The setup.test.ts header comment promised a hoisted spy would be injected
into TWO mock surfaces — the dkg-core barrel AND
`@origintrail-official/dkg-core/dist/faucet.js` — so that the spy fires
no matter which import path reaches it. Only the barrel mock was actually
wired up, with a fresh `vi.fn()` instead of the hoisted spy. The hoisted
spy itself was dead code.
Without the in-package mock, `fundWalletsBestEffort` inside dkg-core's
faucet-orchestration.ts (which does `import { requestFaucetFunding } from
'./faucet.js'`) bypasses the barrel and hits the real implementation. The
five "runSetup Step 5 — faucet funding" tests were failing on main with
"expected vi.fn() to be called 1 times, but got 0 times" because the
production call never touched the spy the assertions read.
Fix: route the barrel mock through the hoisted spy and add the missing
`vi.mock('@origintrail-official/dkg-core/dist/faucet.js', ...)` surface,
sharing the same hoisted spy.
No assertions were relaxed — the tests already asserted the right
behavior, the spy just wasn't catching calls that always existed.
Local: 994/994 vitest tests pass in adapter-openclaw (was 989/994).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
||
| const apiPort = opts.apiPort ?? 9200; | ||
| // Wipe any prior run's data so each suite starts clean. | ||
| await rm(dkgHome, { recursive: true, force: true }).catch(() => {}); |
There was a problem hiding this comment.
🔴 Bug: This wipes .devnet/node1 on every UI test run. That directory is also used by the repo's devnet scripts and other local tooling, so running the E2E suite now destroys a developer's existing node state and auth token. Please move the test daemon to its own temp DKG_HOME and pass the proxy/token to Vite explicitly instead of reusing the shared devnet home.
| }); | ||
| // Accept any HTTP response — auth-protected routes return 401, which | ||
| // still proves the server is up and routing. | ||
| if (resp.status > 0) return; |
There was a problem hiding this comment.
🔴 Bug: waitForApi() treats any HTTP response on :9200 as success, so if a developer already has another daemon bound there this startup path will happily attach to the wrong process and mask the spawn failure. That leaves the suite talking to an unrelated node with a mismatched token. Check that the spawned child is still alive and validate that the responding daemon is the one you just started before returning ready.
| const buf = await readFile(filePath); | ||
| form.append('file', new Blob([buf], { type: 'text/plain' }), 'qa-seed.txt'); | ||
| const fileAssertionName = `${assertionName}-file`; | ||
| const importResp = await fetch(`${base}/api/assertion/${fileAssertionName}/import-file`, { |
There was a problem hiding this comment.
🔴 Bug: The file-import seed path never checks importResp.ok or that assertionUri/fileHash were returned. A failed import currently degrades into empty strings and the suite keeps running with broken fixture state, which makes later failures hard to diagnose. Fail fast here on non-2xx and assert the required fields are present before returning SeedState.
| use: { | ||
| baseURL: `http://localhost:${PORT}/ui/`, | ||
| // Run headed locally so you can see the browser; CI keeps it headless. | ||
| headless: true, |
There was a problem hiding this comment.
🟡 Issue: This now forces headless mode in every environment, even though the comment and the new test:ui:debug workflow imply local runs should be debuggable in a visible browser. If headed locally is intended, gate this on CI (or an explicit env var) instead of hardcoding true.
| this.retentionSelect = this.telemetryCard.locator('select').first(); | ||
| this.retentionPruneBtn = this.telemetryCard.getByRole('button', { name: /Prune/ }); | ||
| this.retentionCancelBtn = this.telemetryCard.getByRole('button', { name: 'Cancel' }); | ||
| this.consentModal = page.getByText('Enable Telemetry Streaming?').locator('xpath=ancestor::div[3]'); |
There was a problem hiding this comment.
🟡 Issue: This locator depends on the modal staying exactly three DOM ancestors above the heading text, which is very brittle for a page object that backs multiple tests. A wrapper div or layout tweak will break the whole Settings suite. Prefer locating the consent dialog by a stable container/role and then scoping from that.
1 participant
Summary
Replaces the mock-driven Playwright tests with a live-daemon E2E harness for the Node UI. Every run spawns a real
dkgdaemon in an isolated.devnet/node1/, seeds one CG + a WM assertion via the daemon's HTTP API, then runs the suite against the real Vite-served UI. No mocks anywhere.End state: 317 passed / 11 skipped in ~4 minutes. The 11 skips are documented production-bug guards — unskip when each fix lands.
What's new
How to run
```bash
pnpm test:ui # build CLI, spawn daemon, run suite (~4 min, headless)
pnpm test:ui:headed # same with browser visible
pnpm test:ui:debug # Playwright UI mode
```
Harness handles daemon + Vite + Chrome lifecycle. Daemon runs isolated (`relay: "none"`) so the suite is deterministic.
Test plan checklist
🐛 Production bugs surfaced by the suite
🔴 Medium-high
🟡 Low
🛡️ Security note
When connected to live testnet, the Background Sync Status dropdown shows unsanitised CG names from peers (including `../../etc/passwd*` and `*agent-context`). React escapes HTML so it's not a literal XSS, but worth strict charset validation on `POST /api/context-graph/create` and sanitising on display.