Skip to content

SY-4231: Improve Aether Test Scaffolding Utilities#2377

Open
emilbon99 wants to merge 2 commits into
rcfrom
sy-4231-improve-aether-test-scaffolding-utilities
Open

SY-4231: Improve Aether Test Scaffolding Utilities#2377
emilbon99 wants to merge 2 commits into
rcfrom
sy-4231-improve-aether-test-scaffolding-utilities

Conversation

@emilbon99

@emilbon99 emilbon99 commented May 27, 2026

Copy link
Copy Markdown
Contributor

Issue Pull Request

Linear Issue

SY-4231

Description

Adds a production-shaped test framework for aether components. Today 78% of aether components have zero aether-side test coverage — Diagram, LinePlot, Table, every visualization renderer, every control component. The existing patterns either go through React (Button/Toggle specs) or hand-build parentCtxValues maps that bypass the production wiring entirely (Log spec, anti-pattern). Neither scales to renderer-class components, which is what unblocked the deferred renderer tests from SY-4186.

The new framework has two layers, both isolated by concern:

  • pluto/src/aether/test/mount.ts (Layer 1) mounts the component under test under a real worker-side provider stack — Root → alamos.Provider → status.Aggregator → synnax.Provider → theming.Provider → telem.Provider → component. Every level runs its real afterUpdate lifecycle; context flows through real Composite._updateContext recursion. No hand-built context maps, no create: () => existingInstance factory tricks. Per-provider state overrides go through each provider's real Zod schema. The harness handle exposes the component, its state, child operations, and direct references to every provider in the stack.

  • pluto/src/vis/render/test/ ships a Proxy-based call recorder that doubles as a duck-typed render.Context. Drop it into mount's renderContext option and a RenderProvider injects it into the worker tree; every scissor / erase / loop.set and every 2D / GL call ends up in a structured list for assertion. Independent of the aether harness — usable on its own to test any code that draws into a canvas context.

  • pluto/src/testutil/renderAether.tsx (Layer 2) wraps RTL render with Aether.Provider plus Aether.Composite at the telem base path, so React components using Aether.use register under the existing worker provider stack. Returns RTL's render result plus root and providers for worker-side inspection.

  • TestLeaf and TestComposite are bundled aether stubs registered under any type string the test wants — same pattern as telemTest.TestSink/TestSource. They record every afterUpdate and afterDelete. Drop in to stand in for any real child while testing a parent's orchestration.

  • pluto/src/vis/diagram/aether/Diagram.spec.ts is a pilot conversion proving the end-to-end shape: real Diagram component, real provider stack, recorder substituted for the render context, full assertions on render-loop / scissor / erase calls plus child orchestration. Writable in single-digit lines of setup.

The Log.spec.ts anti-pattern is not migrated in this PR — it's 1071 lines and the mechanical conversion would balloon the diff. With the framework in place, that migration is a separate follow-up.

Basic Readiness

  • I have performed a self-review of my code.
  • I have added relevant, automated tests to cover the changes.
  • I have updated documentation to reflect the changes.

Greptile Summary

This PR introduces a two-layer production-shaped test harness for aether components, closing the gap where 78% of aether components had zero worker-side test coverage.

  • Layer 1 (aether/test/mount.ts): Builds a real Root → alamos → status → synnax → theming → telem → [RenderProvider] → component stack; exposes a Handle with setState, child, setChildState, deleteChild, and unmount. TestLeaf and TestComposite stubs are bundled and auto-registered.
  • Layer 2 (testutil/renderAether.tsx): Wraps RTL render with Aether.Provider + Aether.Composite so Aether.use calls register under the same worker-side provider stack; returns RTL result plus root and providers.
  • vis/render/test/Recorder.ts: Proxy-based duck-typed render.Context replacement capturing every 2D/GL method, scissor, erase, and loop.set call for assertion. Diagram.spec.ts is included as a pilot conversion.

Confidence Score: 5/5

Safe to merge — the change adds new test infrastructure with no production code modifications, and the harness itself is well-tested by the bundled spec files.

All changed files are test utilities and specs. The harness wiring is exercised by accompanying spec files. The two Diagram spec tests with misleading names are the only findings; they are test documentation gaps, not defects in the harness or the components under test.

pluto/src/vis/diagram/aether/Diagram.spec.ts — two tests have names that over-promise what their assertions verify.

Important Files Changed

Filename Overview
pluto/src/aether/test/mount.ts Core Layer-1 harness: builds full provider stack and exposes a Handle with setState/child/unmount helpers. Architecture is clean; no logic issues found.
pluto/src/vis/render/test/Recorder.ts Proxy-based duck-typed render context recorder. The clear() doc comment inaccurately claims region/dpr are reset (previously flagged); implementation is otherwise sound.
pluto/src/aether/test/RenderProvider.ts Thin Composite that injects a render context under a hardcoded key — CONTEXT_KEY drift risk previously flagged. No other issues.
pluto/src/vis/diagram/aether/Diagram.spec.ts Pilot Diagram integration spec. Two tests have misleading names whose assertions don't cover what the names promise — see inline comments.
pluto/src/testutil/renderAether.tsx Layer-2 RTL wrapper: clean implementation returning RTL result augmented with root and providers.
pluto/src/aether/test/mount.spec.ts Unit tests for the mount harness: exercises Leaf/Composite lifecycle, provider stack access, state overrides, and custom registry. Well-structured and complete.

Sequence Diagram

sequenceDiagram
    participant Test
    participant mount
    participant buildProviderStack
    participant Root as aether.Root
    participant Stack as Provider Stack
    participant Component as Component Under Test

    Test->>mount: "mount(Component, { state, renderContext })"
    mount->>buildProviderStack: buildProviderStack(stackOptions)
    buildProviderStack->>Root: "aether.render({ worker, registry })"
    buildProviderStack->>Stack: driveUpdate x5 (alamos-status-synnax-theming-telem)
    buildProviderStack-->>mount: root, basePath, providers
    mount->>Component: driveUpdate([...basePath, key], TYPE, state)
    Component-->>mount: afterUpdate called
    mount-->>Test: "Handle { component, setState, child, providers, unmount }"

    Test->>mount: handle.setState(next)
    mount->>Component: driveUpdate with new state
    Component-->>Test: afterUpdate called

    Test->>mount: handle.unmount()
    mount->>Root: driveDelete([ROOT_KEY])
    Root-->>Test: entire tree deleted
Loading

Comments Outside Diff (1)

  1. pluto/src/vis/diagram/aether/Diagram.spec.ts, line 881-896 (link)

    P2 The assertion expect(recorder.loopCalls.length).toBeGreaterThan(0) only proves that loop.set was called at least once, but does not verify the content of that call — in particular, it doesn't confirm that the call represents a cleanup/removal (e.g. checking for a null render function or an expected key). The test name says "clears the auto-render interval on unmount", so a tighter assertion on recorder.loopCalls[0].args would better document and guard the intended behaviour.

Reviews (2): Last reviewed commit: "SY-4231: Accept schema-input shape for m..." | Re-trigger Greptile

Adds a production-shaped harness for testing aether components on the
worker side without React, plus a canvas recorder for asserting on
render-context output and a thin RTL wrapper for React-integration
tests.

- pluto/src/aether/test/mount.ts builds the real worker provider stack
  (Root, alamos, status, synnax, theming, telem, plus an optional
  RenderProvider) and exposes a handle for driving state, children,
  and unmount.
- pluto/src/vis/render/test exposes a Proxy-based call recorder that
  doubles as a duck-typed render.Context for use through
  mount()'s renderContext option.
- pluto/src/testutil/renderAether.tsx wraps RTL render with
  Aether.Provider plus Aether.Composite at the telem base path so
  React-mounted components register under the existing worker stack.
- TestLeaf, TestComposite, and a Diagram pilot spec demonstrate the
  intended usage.
Comment on lines +139 to +141
/** Resets every recording (calls, scissor/erase/loop) and the recorded region/dpr.
* Use between phases of a test when you only care about calls made since `clear`. */
clear(): void {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The doc comment claims clear() resets "the recorded region/dpr", but the implementation never touches this.region or this.dpr. After a clear() call between test phases, both stay at their last-resize()-set values, which can cause misleading assertions if a subsequent test phase reads recorder.region or recorder.dpr and expects the initial zero/one defaults.

Suggested change
/** Resets every recording (calls, scissor/erase/loop) and the recorded region/dpr.
* Use between phases of a test when you only care about calls made since `clear`. */
clear(): void {
/** Resets every recording (calls, scissor/erase/loop) since the last `clear`.
* Use between phases of a test when you only care about calls made since `clear`. */
clear(): void {

Rule Used: Flag misleading doc comments that promise fallback... (source)

context: z.any(),
});

/** Thin Composite that injects a `render.Context`-shaped value into the parent context

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 RENDER_CONTEXT_KEY duplicates the private Context.CONTEXT_KEY = CSS.B("render-context") value by hardcoding "pluto-render-context". If the CSS.B prefix ever changes (or Context.CONTEXT_KEY is renamed), RenderProvider will silently inject the context under the wrong key — components using render.Context.use(ctx) will find nothing, and no compile-time error will catch the drift. Consider exporting the key from the render context module so both sites share the same constant.

z.infer<S> requires all fields including defaulted ones (output type),
which forced callers to specify clearOverScan and similar on every
mount. Switch to z.input<S> so defaulted fields can be omitted at the
test call site.
@codecov

codecov Bot commented May 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.06349% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.53%. Comparing base (d2dec0a) to head (9156f98).
⚠️ Report is 155 commits behind head on rc.

Files with missing lines Patch % Lines
pluto/src/vis/render/test/Recorder.ts 84.84% 10 Missing ⚠️
pluto/src/aether/test/mount.ts 96.51% 3 Missing ⚠️
pluto/src/aether/test/TestComposite.ts 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               rc    #2377      +/-   ##
==========================================
- Coverage   66.62%   66.53%   -0.10%     
==========================================
  Files        2470     2477       +7     
  Lines      116204   115754     -450     
  Branches     8782     8692      -90     
==========================================
- Hits        77418    77013     -405     
+ Misses      32537    32495      -42     
+ Partials     6249     6246       -3     
Flag Coverage Δ
console 22.45% <ø> (ø)
pluto 60.22% <92.06%> (+0.38%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant