Skip to content

test: comprehensive TDD test suite for configurable constants & managed-container#991

Open
buihongduc132 wants to merge 1 commit into
browseros-ai:devfrom
buihongduc132:feat/tdd-constants-tests
Open

test: comprehensive TDD test suite for configurable constants & managed-container#991
buihongduc132 wants to merge 1 commit into
browseros-ai:devfrom
buihongduc132:feat/tdd-constants-tests

Conversation

@buihongduc132
Copy link
Copy Markdown

Summary

This PR introduces a TDD-first test suite (114 tests) for the configurable constants modules (timeouts.ts, limits.ts, paths.ts) and the ManagedContainer lifecycle. No implementation changes — test files only.

These tests define the contract for the upcoming env-override implementation. They will initially fail (expected in TDD red-green-refactor cycle) and will pass once the feature PR lands.

Test Coverage

timeouts.test.ts — 20 tests, 6 describe blocks

Category Tests What it verifies
Default values 3 All 26 keys pinned individually, key-count guard, positive/finite type check
Env overrides 5 5 keys across TOOL/MCP/CDP/SKILLS/OAUTH categories
Invalid env 2 Non-numeric ("abc"), trailing chars ("10s")
Negative env 1 Negative value falls back to default
Edge cases 7 Zero, float, empty string, whitespace, overflow (>MAX_SAFE_INTEGER), leading/trailing whitespace on valid number
KLAVIS retry array 3 Correct tuple, ascending order, length

limits.test.ts — 35 tests, 14 describe blocks

Category Tests What it verifies
Multi-group defaults 22 AGENT_LIMITS (3+5 non-overridable + key-count 23), TOOL_LIMITS (3+key-count), PAGINATION (1), CDP_LIMITS (2), CONTENT_LIMITS (2+key-count), AGENT_HARNESS_LIMITS (3)
Env overrides 3 MAX_TURNS, DEFAULT_CONTEXT_WINDOW, FILESYSTEM_READ_MAX_LINES
Invalid/Negative env 3 Non-numeric, trailing chars, negative
Edge cases 5 Zero, float, empty, whitespace, overflow
Structural integrity 2 All AGENT/TOOL values positive, finite numbers

paths.test.ts — 32 tests, 9 describe blocks

Category Tests What it verifies
Key-count guard 1 PATHS exports exactly 18 keys
Numeric defaults 3 SOUL_MAX_LINES, MEMORY_RETENTION_DAYS, SESSION_RETENTION_DAYS
String defaults 14 All directory/file names pinned individually (including TOOL_OUTPUT_DIR_NAME, OPENCLAW_DIR_NAME)
Dynamic defaults 1 DEFAULT_EXECUTION_DIR === process.cwd()
Env overrides 3 3 numeric keys overridable
Invalid/Negative 2 Non-numeric, negative
Edge cases 5 Zero, float, empty, whitespace, overflow
Structural checks 2 Numeric positive/integer, strings non-empty

managed-container.test.ts — 27 tests, 10 describe blocks

Category Tests What it verifies
State machine 4 Full lifecycle, errored probe, force-stop, cold-boot ordering
execProcess gating 4 not_installed rejection, errored rejection, async wait, timeout
buildExecArgv 2 Canonical limactl chain, empty env
reset 1 All 3 levels throw ResetNotSupportedError
Path translation 3 Host↔container round-trip, mount root, outside-mounts rejection
subscribeState 1 Fires transitions, stops after unsubscribe
isImageCurrent 4 Match, SHA-pinned, different, null/missing
getLogs/tailLogs 4 Success, missing container, non-zero error, tail unsubscribe
runOneShot 4 Normal flow, force-remove on throw, timeout drops onLog, name-in-use retry

Design Decisions

  • Subprocess isolation: Env-override tests use Bun.spawn() with inline scripts to guarantee a fresh module cache — process.env is read at import time with the overridden value, no state leakage between tests.
  • Shared test utility: test-utils.ts exports a single spawnWithEnv() helper used by all 3 constant test files (DRY).
  • Edge-case battery: Every constant file tests the same 6 edge cases (zero, float, empty, whitespace, overflow, non-numeric) to ensure consistent env parsing behavior.
  • Fake/stub pattern: managed-container.test.ts uses FakeCli/FakeLoader/FakeVm interfaces with a makeFakeDeps() factory — no real Docker/Lima dependencies.
  • Key-count guards: AGENT_LIMITS(23), TOOL_LIMITS(3), CONTENT_LIMITS(6), PATHS(18) — catches accidental additions/removals.
  • Resource cleanup: afterEach cleans temp dirs; spawned processes are drained via proc.exited.catch().

Files Changed

 packages/browseros-agent/packages/shared/package.json                   |   3 +-
 packages/browseros-agent/packages/shared/tests/constants/test-utils.ts  |  30 +
 packages/browseros-agent/packages/shared/tests/constants/limits.test.ts | 280 ++++++++++
 packages/browseros-agent/packages/shared/tests/constants/paths.test.ts  | 216 ++++++++
 packages/browseros-agent/packages/shared/tests/constants/timeouts.test.ts| 263 ++++++++++
 packages/browseros-agent/apps/server/tests/lib/container/managed/managed-container.test.ts | 230 ++++++++++
 5 files changed, 1021 insertions(+), 53 deletions(-)

Test Plan

These tests ARE the test plan. Run:

cd packages/browseros-agent
bun test packages/shared/tests/constants/ apps/server/tests/lib/container/managed/

Expected: env-override tests will fail (red phase) — that is correct TDD behavior. Default-value and edge-case tests may pass or fail depending on current parsing logic.

Follow-up

A companion PR will add the env-override implementation (timeouts.ts, limits.ts, paths.ts, tool-adapter.ts, .env.example) that makes these tests pass (green phase).

CLA

I have read the CLA Document and I hereby sign the CLA

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 10, 2026

PR author is not in the allowed authors list.

@github-actions
Copy link
Copy Markdown
Contributor

Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement.

To sign the CLA, please add a comment to this PR with the following text:

I have read the CLA Document and I hereby sign the CLA

You only need to sign once. After signing, this check will pass automatically.


Troubleshooting
  • Already signed but still failing? Comment recheck to trigger a re-verification.
  • Signed with a different email? Make sure your commit email matches your GitHub account email, or add your commit email to your GitHub account.
- - - I have read the CLA Document and I hereby sign the CLA - - - **buihongduc132** seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please [add the email address used for this commit to your account](https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/#commits-are-not-linked-to-any-user).
You can retrigger this bot by commenting **recheck** in this Pull Request. Posted by the **CLA Assistant Lite bot**.

@buihongduc132
Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

- timeouts.test.ts: 20 tests — 26-key defaults, env overrides, edge cases, KLAVIS array
- limits.test.ts: 35 tests — multi-group defaults with key-count guards, env overrides, structural checks
- paths.test.ts: 32 tests — 18-key guard, 14 string + 3 numeric defaults, env overrides, edge cases
- managed-container.test.ts: top-level import for ContainerNameInUseError (was dynamic)
- test-utils.ts: shared spawnWithEnv helper using Bun.spawn subprocess isolation
- package.json: add bun test script

Env-override tests use Bun.spawn subprocess isolation for clean module cache.
Edge cases cover: zero, float, empty string, whitespace, MAX_SAFE_INTEGER overflow.
@buihongduc132 buihongduc132 force-pushed the feat/tdd-constants-tests branch from 420eccc to 48b7daa Compare May 10, 2026 12:21
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