Skip to content

Fix CLI test runner preload and test-utils fallback for public mirror - #1203

Open
nordicnode wants to merge 1 commit into
CodebuffAI:mainfrom
nordicnode:fix-cli-test-runner
Open

Fix CLI test runner preload and test-utils fallback for public mirror#1203
nordicnode wants to merge 1 commit into
CodebuffAI:mainfrom
nordicnode:fix-cli-test-runner

Conversation

@nordicnode

Copy link
Copy Markdown

Problem & Context

Running CLI tests via Bun on the public repository failed immediately with two blocking issues:

  1. Missing Preload in cli/bunfig.toml: cli/bunfig.toml declared ../test/setup-scm-loader.ts under preload. The root test/ directory was unbundled from the public mirror in commit 61bbbed7, causing any invocation of bun test in cli to crash at startup before running any test files:

    error: preload not found "../test/setup-scm-loader.ts"
    

    (Modern Bun loads .scm text queries natively without requiring an external loader plugin).

  2. Missing packages/internal in cli/src/__tests__/test-utils.ts: When test suites invoked ensureCliTestEnv(), loadCliEnv() attempted to require ../../../packages/internal/src/env. Because packages/internal is an unexported private module omitted from the public repository, this threw an unhandled error (Cannot find module '../../../packages/internal/src/env') instead of utilizing the defined test defaults (TEST_CLIENT_ENV_DEFAULTS and TEST_SERVER_ENV_DEFAULTS).

Changes Made

  • cli/bunfig.toml: Removed the non-existent ../test/setup-scm-loader.ts entry from preload.
  • cli/src/__tests__/test-utils.ts: Wrapped the dynamic require('../../../packages/internal/src/env') in a fallback block that initializes cachedEnv with TEST_CLIENT_ENV_DEFAULTS and TEST_SERVER_ENV_DEFAULTS when running on the public repository.

Architecture & Conventions Conformance

  • Adheres to Dependency Injection (contracts defined in common/src/types/contracts/, no module monkey patching)
  • Terminal commands use terminalCommandBroker (no direct spawn or TUI-process bypass)
  • Environment hygiene respected (getCliEnv() for CLI, getSdkEnv() for SDK, no forbidden getProcessEnv() imports)
  • Freebuff mode compatibility (IS_FREEBUFF preserved, no paid features introduced)
  • Imports ordered and explicit (import type used for types)

Scope Verification

  • All modified files are within allowed public directories: cli/
  • NO modifications to web/, freebuff/web/, packages/internal/, packages/billing/, packages/bigquery/, or packages/build-tools/

Testing & Verification

  • bun run --cwd cli typecheck passed with 0 errors.
  • bun test src/__tests__/cli-args.test.ts passed 15/15 tests (was crashing on missing preload).
  • bun test src/__tests__/launcher-disconnect.test.ts passed (was crashing on missing packages/internal).
  • bun test src/__tests__/launcher-avx2-fallback.test.ts passed 31/31 tests.
  • bun test src/__tests__/unit/ passed 85/85 tests.
  • bun run build:sdk passed cleanly.
  • bun run build:freebuff passed cleanly.
  • bun cli/scripts/smoke-binary.ts cli/bin/freebuff passed cleanly.
  • Anti-flake principles observed.

@codebuff-team

Copy link
Copy Markdown
Contributor

Good, focused fix. Both changes address concrete breakage caused by the mirror export process rather than opinion or style:

  1. cli/bunfig.toml: removing the dangling ../test/setup-scm-loader.ts preload entry is correct — that file doesn't exist in the mirror, so bun test fails before collecting any tests. Worth double-checking on the private side whether this loader is still needed there (bun's native .scm support may vary by version pinned internally), but for the mirror this is clearly right.

  2. cli/src/__tests__/test-utils.ts: wrapping the packages/internal/src/env require in a try/catch with fallback to TEST_CLIENT_ENV_DEFAULTS/TEST_SERVER_ENV_DEFAULTS is a reasonable and minimal way to let tests run without the private package, without touching test intent. The comment explaining the fallback is clear and the change is small.

One nit: the outer try/catch already existed for a different purpose (env default setup failures) — nesting a second try/catch inside it works but slightly muddies what error is being caught where. A short comment distinguishing 'private module missing' from 'other load failures' would help future readers, since a genuine bug in packages/internal/src/env would now silently fall back to defaults instead of surfacing.

Overall this is a real, narrow bug fix with verification steps included, and it's confined to cli/, which is in scope. Worth porting, modulo the internal team deciding whether the scm-loader removal applies to their private tree as well.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree labels Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants