test(e2e): cover the client's lvt-* attribute handler registry

[adnaan]

Important

Draft — do not merge until @livetemplate/client ships the attribute registry.

This test asserts an API that does not exist in the published client yet. lvt/testing fetches @livetemplate/client@latest from the CDN, so on main today this test fails with registerAttribute is not a function — correctly. It goes green the moment the client releases with livetemplate/client#159.

That failure is the point: it is what makes this a real test rather than a tautology.

The browser half of phase 1 of livetemplate/livetemplate#473. Pairs with livetemplate/client#159, which adds the registry and the public registerAttribute() API.

Why a new test, when the built-in attributes are already covered

The existing e2e suites exercise attribute behaviour — they drive lvt-fx:scroll, modals, event delegation. They pass identically whether those attributes are dispatched by a hardcoded call sequence or by a registry, which is exactly what made them the right regression net for the refactor. It also means they cannot see the wiring at all.

The registry's failure mode is silence. A handler that never runs, or one that warns on every page load, breaks nothing any existing assertion looks at. During development a severed array literal in the client left 4 of 18 handlers registered and all 861 unit tests green — only the bundle size gave it away.

What it asserts

1. No registry diagnostics on a well-formed page. The registry must not warn about the framework's own handlers.
2. Late registration takes effect immediately. A handler registered from a plain <script> after the client tag reaches the DOM already on screen — no render is triggered afterwards on purpose. This is the only path available to a third-party bundle: under the documented defer pattern the core bundle has already auto-initialized and connected by the time a second script evaluates.
3. ctx.value reads the attribute from the already-rendered element, proving the catch-up scan actually matched rather than merely running.
4. Both spellings of the global resolve. --global-name=LiveTemplateClient over a module that also exports a class of that name makes window.LiveTemplateClient the module namespace — so LiveTemplateClient.registerAttribute(...), what every doc example writes, resolves to a module-level named export, and the class sits at LiveTemplateClient.LiveTemplateClient. Indistinguishable in a unit test, trivially confused in a refactor.

Two deliberate choices worth reviewing

The console assertion is scoped, not absolute. It looks for this feature's own diagnostics (AttributeRegistry, already claims this name, never both, has an empty value) rather than demanding zero console output. The fixture serves no WebSocket endpoint, so the client legitimately reports a transport failure; asserting on total silence would make the test hostage to unrelated noise and it would rot on the first unrelated log line.

It was falsified before being trusted. Run against the published bundle: fails. Against a build of livetemplate/client#159: passes. A browser test asserting a new API is otherwise indistinguishable from one asserting nothing.

How it was verified locally

The client bundle was built from livetemplate/client#159 and dropped into lvt's test-client disk cache, then the suite run with -count=1:

go test -tags=browser -count=1 -timeout 25m \
  -run 'TestRendering_|TestModalFunctionality|TestE2E_|TestAttributeRegistry' ./e2e/ -v

19/19 PASS (18 pre-existing + this one). -count=1 is mandatory: without it go test replays a cached result and the suite "passes" against a bundle it never loaded — which happened once during development and was caught only by checking the byte count in [lvt/testing] Using cached client library (…).

🤖 Generated with Claude Code

https://claude.ai/code/session_01N6hr7ZCG9o4pCmtnA8qhSq

[claude]

Review

Nice test — the doc comment does real work explaining why this test exists (silent-failure mode, the module-namespace-vs-class-static trap, the "no pre-init window" constraint on third-party bundles), and the draft framing (falsified against main, verified against the dependent client PR, -count=1 warning) is exactly the kind of rigor this class of test needs.

Potential bug: unguarded read of consoleLog in the error path

```go
err := chromedp.Run(ctx, ...)
if err != nil {
t.Fatalf("browser run failed: %v\nconsole:\n%s", err, strings.Join(consoleLog, "\n"))
}
```

consoleLog is appended to from the chromedp.ListenTarget callback goroutine, guarded by consoleMu. This t.Fatalf reads consoleLog directly, without acquiring consoleMu first — a real data race under -race if a console event arrives while chromedp.Run is failing (arguably the likeliest moment for one to arrive, e.g. a script error). Every other hand-rolled console capture in this package guards the read in the failure path too, even inside t.Fatalf calls (e.g. edit_modal_reopen_test.go:240-248, modal_test.go:320-327). Worth acquiring the lock here as well.

Reuse: e2etest.NewConsoleLogger() already does this, and does more

testing/console.go provides ConsoleLogger, already used in livetemplate_core_test.go, with a sync.Mutex-protected, defensively-copied GetLogs(). Two things it gives you for free that the hand-rolled version here doesn't:

• It also listens for runtime.EventExceptionThrown, so a thrown JS exception (e.g. from a bug in the registry's catch-up scan) would show up in the captured log — a manual EventConsoleAPICalled-only listener misses those entirely.
• It sidesteps the race above by construction (GetLogs() returns a copy under lock).

Given the whole point of this test is catching silent registry failures, being able to see thrown exceptions in the diagnostic dump seems valuable, not just a style nit.

Minor style nit

consoleMu := make(chan struct{}, 1) used as a mutex (acquire = receive, release = send) works, but it's the only place in the e2e package doing it this way — everywhere else in this package (complete_workflow_test.go, embedded_browser_test.go, edit_modal_reopen_test.go, modal_test.go, etc.) uses a plain sync.Mutex/sync.RWMutex for the identical console-log-slice-guard pattern. Switching to the shared ConsoleLogger helper above would resolve this too.

Test coverage

Scope matches what the PR description claims: silence-on-well-formed-page, late-registration catch-up reaching the already-rendered DOM, ctx.value being a live accessor rather than a captured string, and both global spellings (LiveTemplateClient.registerAttribute vs LiveTemplateClient.LiveTemplateClient.registerAttribute). That's a solid, non-tautological set of assertions for a wiring-layer refactor. One possible follow-up (not blocking, and maybe out of scope for Phase 1): no test yet exercises the registry emitting the diagnostics this test asserts are absent (duplicate name claims, mutually exclusive options, empty-value warnings) — a companion "registry actually complains about malformed handlers" test would round this out nicely once the client dependency lands.

Security / performance

No concerns — test-only code, reuses the existing pooled-Chrome infrastructure (GetPooledChrome), no untrusted input handling.

Build-tag / draft status

Correctly gated behind //go:build browser, and the PR is honest about being blocked on livetemplate/client#159 — nothing to flag there.