Conversation
nberlette
commented
Mar 12, 2026
- tests(utils): add more test cases for internal utils
- tests(utils): add ansi_codes and component utils tests
- fix(tests): move misplaced @ts-expect-error comment to the correct line
- tests(event-emitter): add EventEmitter test cases
- tests(components): add unit tests for Component APIs
- tests(canvas): add Canvas API unit tests
- tests: add text fixtures module
- config(gitignore): add .coverage dir to .gitignore
- chore: update lockfile
- tests: add smoke tests for mod.ts barrel entry points
- tests(input): add input test cases
- tests: add unit tests for theme, layout, and view APIs
- tests(signals): add unit tests for signals APIs
- tests(stdio): add stdio unit tests
- chore: remove contributing.md since it was practically empty
- config: add tasks to deno.json
There was a problem hiding this comment.
Pull request overview
Adds a broad set of new unit tests across the library (signals, stdio, canvas, components, input, layout/theme/view) and introduces local deno task helpers to make running tests/lint/fmt (and collecting coverage) easier.
Changes:
- Added many new unit tests and a shared
tests/fixtures.tshelper to improve coverage across core modules. - Added smoke tests for barrel exports (e.g.,
mod.ts,src/*/mod.ts). - Updated repo config: ignore coverage output, add
deno taskentries, and updatedeno.lock(Node typings).
Reviewed changes
Copilot reviewed 35 out of 37 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/view.test.ts | Adds View constructor behavior tests. |
| tests/utils/strings.test.ts | Expands string utility coverage. |
| tests/utils/sorted_array.test.ts | Adds more SortedArray behavior assertions. |
| tests/utils/numbers.test.ts | Restructures/expands numbers utility tests. |
| tests/utils/component.test.ts | Adds tests for component-related utils. |
| tests/utils/async.test.ts | Expands sleep() tests (includes timing + concurrency). |
| tests/utils/ansi_codes.test.ts | Adds ANSI constants/moveCursor tests. |
| tests/theme.test.ts | Adds tests for theme helpers and Theme factories. |
| tests/stdio/stdout.test.ts | Adds Stdout adapter + writable stream tests. |
| tests/stdio/stdin.test.ts | Adds Stdin adapter + readable stream tests. |
| tests/stdio/stderr.test.ts | Adds Stderr adapter + writable stream tests. |
| tests/stdio/mod.test.ts | Adds stdio barrel export smoke test. |
| tests/stdio/console_size.test.ts | Adds console size resolution/fallback tests. |
| tests/signals/signalify.test.ts | Adds signalify wrapping behavior tests. |
| tests/signals/mod.test.ts | Adds signals barrel export smoke test. |
| tests/signals/flusher.test.ts | Adds Flusher dependency/flush behavior test. |
| tests/signals/effect.test.ts | Adds Effect/effect lifecycle behavior tests. |
| tests/signals/dependency_tracking.test.ts | Adds dependency tracking/optimization tests. |
| tests/signals/computed.test.ts | Adds Computed/computed behavior tests. |
| tests/mod.test.ts | Adds root mod.ts export smoke test. |
| tests/layout.test.ts | Adds public layout API tests and error assertions. |
| tests/input.test.ts | Adds keyboard/mouse/buffer decoding tests. |
| tests/fixtures.ts | Introduces shared test fixtures (fake root/canvas/stdout helpers). |
| tests/event_emitter.test.ts | Expands EventEmitter tests (cleanup/off behavior). |
| tests/components/text.test.ts | Adds Text component draw/sizing tests. |
| tests/components/mod.test.ts | Adds components barrel export smoke test. |
| tests/components/box.test.ts | Adds Box component draw wiring test. |
| tests/component.test.ts | Adds Component interaction + drawn object visibility tests. |
| tests/canvas/text.test.ts | Adds TextObject sizing + multi-codepoint tests. |
| tests/canvas/renderable.test.ts | Adds Renderable rerender/out-of-bounds tests. |
| tests/canvas/mod.test.ts | Adds canvas barrel export smoke test. |
| tests/canvas/core.test.ts | Adds Canvas intersection + render behavior tests. |
| tests/canvas/box.test.ts | Adds BoxObject rerender + omit-cell behavior test. |
| deno.lock | Updates lockfile (includes Node typings). |
| deno.json | Adds convenience tasks for test/lint/fmt and coverage output. |
| CONTRIBUTING.md | Removes (previously minimal/empty) contributing guide. |
| .gitignore | Ignores .coverage output directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| const original = globalThis.process; | ||
| Object.defineProperty(globalThis, "process", { | ||
| value: { | ||
| stdout: { columns: 0, rows: 0, getWindowSize: undefined }, | ||
| stderr: { columns: 0, rows: 0 }, | ||
| env: { COLUMNS: "90", LINES: "33" }, | ||
| }, | ||
| configurable: true, | ||
| }); | ||
|
|
||
| try { | ||
| assertEquals(consoleSize(), { columns: 90, rows: 33 }); | ||
| } finally { | ||
| Object.defineProperty(globalThis, "process", { | ||
| value: original, | ||
| configurable: true, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Same globalThis.process restoration issue as above: this finally block redefines the property rather than restoring/removing it, which can pollute global state across tests. Consider capturing and restoring the original property descriptor (or deleting the property if it was originally absent).
| const defaults = { columns: 7, rows: 9 }; | ||
| Object.defineProperty(globalThis, "process", { | ||
| get() { | ||
| throw new Error("boom"); | ||
| }, | ||
| configurable: true, | ||
| }); | ||
|
|
||
| try { | ||
| const size = consoleSize(defaults); | ||
| assertEquals(size, defaults); | ||
| assertNotStrictEquals(size, defaults); | ||
| } finally { | ||
| Object.defineProperty(globalThis, "process", { | ||
| value: original, | ||
| configurable: true, | ||
| }); | ||
| } |
There was a problem hiding this comment.
After installing a throwing getter for globalThis.process, the finally block should restore the original descriptor or remove the property if it wasn't originally present. Replacing it with { value: original } changes the semantics for subsequent tests (e.g., "process" in globalThis).
| import { Canvas } from "../src/canvas/canvas.ts"; | ||
| import { EventEmitter } from "../src/event_emitter.ts"; | ||
| import { Signal } from "../src/signals/signal.ts"; |
There was a problem hiding this comment.
This new file is missing the repository’s required license header (// Copyright 2023 Im-Beast. MIT license.). CI runs a header-check step for .ts files (excluding only deps.ts), so this will be flagged/rewritten during CI.
| await it("is a function", () => { | ||
| assert.strict(typeof sleep === "function"); | ||
| }); |
There was a problem hiding this comment.
node:assert's default import doesn't have an assert.strict(...) assertion function (it's an object / namespace). This call will throw at runtime; use assert.ok(...) / assert.strictEqual(..., true) or import from node:assert/strict instead.
| await it("returns a promise", () => { | ||
| const result = sleep(100); | ||
| assert.strict(isPromise(result), "Expected sleep to return a promise"); | ||
| }); |
There was a problem hiding this comment.
assert.strict(...) is not a valid assertion on the default node:assert import, so this check will throw. Use assert.ok(isPromise(result)), assert.strictEqual(isPromise(result), true), or switch to node:assert/strict.
| const original = globalThis.process; | ||
| Object.defineProperty(globalThis, "process", { | ||
| value: { | ||
| stdout: { columns: 111, rows: 22 }, | ||
| stderr: { columns: 1, rows: 1 }, | ||
| env: {}, | ||
| }, | ||
| configurable: true, | ||
| }); | ||
|
|
||
| try { | ||
| assertEquals(consoleSize(), { columns: 111, rows: 22 }); | ||
| } finally { | ||
| Object.defineProperty(globalThis, "process", { | ||
| value: original, | ||
| configurable: true, | ||
| }); | ||
| } |
There was a problem hiding this comment.
This test mutates globalThis.process, but the restore path always re-defines the property (even when it was originally absent/undefined). That can leak state into later tests because "process" in globalThis becomes true. Track whether process was originally an own property and restore by deleting it when it didn't exist, or restore the original property descriptor.