From a85cde819b54730ad6b62c24161294ad2389d3d6 Mon Sep 17 00:00:00 2001 From: KeKs0r Date: Fri, 27 Feb 2026 02:16:04 +0800 Subject: [PATCH] Remove test flakiness by injecting dependencies as parameters Replace process.env mutation in client.test.ts and process.stdout.isTTY mutation in format.test.ts with pure dependency injection. Add optional env parameter to resolveConnectionConfig and isTTY parameter to resolveFormat, both defaulting to their original runtime behavior. Eliminates beforeEach/afterEach save/restore overhead and Object.defineProperty hacks, making tests deterministic with zero global state pollution. Co-Authored-By: Claude Haiku 4.5 --- src/client.test.ts | 95 +++++++++++++++++++--------------------------- src/client.ts | 25 ++++++------ src/format.test.ts | 10 +---- src/format.ts | 7 +++- 4 files changed, 59 insertions(+), 78 deletions(-) diff --git a/src/client.test.ts b/src/client.test.ts index 61b07ca..b49d7a3 100644 --- a/src/client.test.ts +++ b/src/client.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, beforeEach, afterEach } from "bun:test"; +import { describe, test, expect } from "bun:test"; import { parseClickHouseUrl, resolveConnectionConfig } from "./client"; describe("parseClickHouseUrl", () => { @@ -36,40 +36,11 @@ describe("parseClickHouseUrl", () => { }); describe("resolveConnectionConfig", () => { - const saved: Record = {}; - const envKeys = [ - "CLICKHOUSE_URL", - "CLICKHOUSE_HOST", - "CLICKHOUSE_PORT", - "CLICKHOUSE_SECURE", - "CLICKHOUSE_USER", - "CLICKHOUSE_USERNAME", - "CLICKHOUSE_PASSWORD", - "CLICKHOUSE_DATABASE", - "CLICKHOUSE_DB", - ]; - - beforeEach(() => { - for (const key of envKeys) { - saved[key] = process.env[key]; - delete process.env[key]; - } - }); - - afterEach(() => { - for (const key of envKeys) { - if (saved[key] !== undefined) { - process.env[key] = saved[key]; - } else { - delete process.env[key]; - } - } - }); - const emptyConfig = {} as Parameters[0]; + const emptyEnv = {}; test("uses defaults when no env vars or flags set", () => { - const result = resolveConnectionConfig(emptyConfig); + const result = resolveConnectionConfig(emptyConfig, emptyEnv); expect(result).toEqual({ url: "http://localhost:8123", username: "default", @@ -79,62 +50,72 @@ describe("resolveConnectionConfig", () => { }); test("CLICKHOUSE_URL sets host, port, secure", () => { - process.env.CLICKHOUSE_URL = "https://ch.prod.com:8443"; - const result = resolveConnectionConfig(emptyConfig); + const result = resolveConnectionConfig(emptyConfig, { + CLICKHOUSE_URL: "https://ch.prod.com:8443", + }); expect(result.url).toBe("https://ch.prod.com:8443"); }); test("CLICKHOUSE_URL password is used as fallback", () => { - process.env.CLICKHOUSE_URL = "https://user:urlpass@ch.prod.com:8443"; - const result = resolveConnectionConfig(emptyConfig); + const result = resolveConnectionConfig(emptyConfig, { + CLICKHOUSE_URL: "https://user:urlpass@ch.prod.com:8443", + }); expect(result.password).toBe("urlpass"); }); test("CLICKHOUSE_PASSWORD takes precedence over URL password", () => { - process.env.CLICKHOUSE_URL = "https://user:urlpass@ch.prod.com:8443"; - process.env.CLICKHOUSE_PASSWORD = "envpass"; - const result = resolveConnectionConfig(emptyConfig); + const result = resolveConnectionConfig(emptyConfig, { + CLICKHOUSE_URL: "https://user:urlpass@ch.prod.com:8443", + CLICKHOUSE_PASSWORD: "envpass", + }); expect(result.password).toBe("envpass"); }); test("CLICKHOUSE_HOST takes precedence over CLICKHOUSE_URL", () => { - process.env.CLICKHOUSE_URL = "https://from-url.com:8443"; - process.env.CLICKHOUSE_HOST = "from-host-env.com"; - const result = resolveConnectionConfig(emptyConfig); + const result = resolveConnectionConfig(emptyConfig, { + CLICKHOUSE_URL: "https://from-url.com:8443", + CLICKHOUSE_HOST: "from-host-env.com", + }); expect(result.url).toBe("https://from-host-env.com:8443"); }); test("CLICKHOUSE_USERNAME is used when CLICKHOUSE_USER is not set", () => { - process.env.CLICKHOUSE_USERNAME = "doppler_user"; - const result = resolveConnectionConfig(emptyConfig); + const result = resolveConnectionConfig(emptyConfig, { + CLICKHOUSE_USERNAME: "doppler_user", + }); expect(result.username).toBe("doppler_user"); }); test("CLICKHOUSE_USER takes precedence over CLICKHOUSE_USERNAME", () => { - process.env.CLICKHOUSE_USER = "primary"; - process.env.CLICKHOUSE_USERNAME = "fallback"; - const result = resolveConnectionConfig(emptyConfig); + const result = resolveConnectionConfig(emptyConfig, { + CLICKHOUSE_USER: "primary", + CLICKHOUSE_USERNAME: "fallback", + }); expect(result.username).toBe("primary"); }); test("CLICKHOUSE_DB is used when CLICKHOUSE_DATABASE is not set", () => { - process.env.CLICKHOUSE_DB = "doppler_db"; - const result = resolveConnectionConfig(emptyConfig); + const result = resolveConnectionConfig(emptyConfig, { + CLICKHOUSE_DB: "doppler_db", + }); expect(result.database).toBe("doppler_db"); }); test("CLICKHOUSE_DATABASE takes precedence over CLICKHOUSE_DB", () => { - process.env.CLICKHOUSE_DATABASE = "primary"; - process.env.CLICKHOUSE_DB = "fallback"; - const result = resolveConnectionConfig(emptyConfig); + const result = resolveConnectionConfig(emptyConfig, { + CLICKHOUSE_DATABASE: "primary", + CLICKHOUSE_DB: "fallback", + }); expect(result.database).toBe("primary"); }); test("CLI flags take precedence over all env vars", () => { - process.env.CLICKHOUSE_URL = "https://from-url.com:9999"; - process.env.CLICKHOUSE_USER = "env_user"; - process.env.CLICKHOUSE_DATABASE = "env_db"; - process.env.CLICKHOUSE_PASSWORD = "env_pass"; + const env = { + CLICKHOUSE_URL: "https://from-url.com:9999", + CLICKHOUSE_USER: "env_user", + CLICKHOUSE_DATABASE: "env_db", + CLICKHOUSE_PASSWORD: "env_pass", + }; const config = { host: "flag-host", @@ -145,7 +126,7 @@ describe("resolveConnectionConfig", () => { secure: true, } as Parameters[0]; - const result = resolveConnectionConfig(config); + const result = resolveConnectionConfig(config, env); expect(result).toEqual({ url: "https://flag-host:1234", username: "flag_user", diff --git a/src/client.ts b/src/client.ts index 2a98937..e0aa7b1 100644 --- a/src/client.ts +++ b/src/client.ts @@ -11,18 +11,21 @@ export function parseClickHouseUrl(raw: string) { }; } -export function resolveConnectionConfig(config: CliConfig) { - const parsed = process.env.CLICKHOUSE_URL - ? parseClickHouseUrl(process.env.CLICKHOUSE_URL) +export function resolveConnectionConfig( + config: CliConfig, + env: Record = process.env, +) { + const parsed = env.CLICKHOUSE_URL + ? parseClickHouseUrl(env.CLICKHOUSE_URL) : undefined; const host = - config.host || process.env.CLICKHOUSE_HOST || parsed?.host || "localhost"; + config.host || env.CLICKHOUSE_HOST || parsed?.host || "localhost"; const port = - config.port || process.env.CLICKHOUSE_PORT || parsed?.port || "8123"; + config.port || env.CLICKHOUSE_PORT || parsed?.port || "8123"; const secure = config.secure || - process.env.CLICKHOUSE_SECURE === "true" || + env.CLICKHOUSE_SECURE === "true" || (parsed?.secure ?? false); const protocol = secure ? "https" : "http"; @@ -30,18 +33,18 @@ export function resolveConnectionConfig(config: CliConfig) { url: `${protocol}://${host}:${port}`, username: config.user || - process.env.CLICKHOUSE_USER || - process.env.CLICKHOUSE_USERNAME || + env.CLICKHOUSE_USER || + env.CLICKHOUSE_USERNAME || "default", password: config.password || - process.env.CLICKHOUSE_PASSWORD || + env.CLICKHOUSE_PASSWORD || parsed?.password || "", database: config.database || - process.env.CLICKHOUSE_DATABASE || - process.env.CLICKHOUSE_DB || + env.CLICKHOUSE_DATABASE || + env.CLICKHOUSE_DB || "default", }; } diff --git a/src/format.test.ts b/src/format.test.ts index df41fb3..bdde974 100644 --- a/src/format.test.ts +++ b/src/format.test.ts @@ -28,16 +28,10 @@ describe("resolveFormat", () => { }); test("defaults to PrettyCompactMonoBlock when stdout is a TTY", () => { - const orig = process.stdout.isTTY; - Object.defineProperty(process.stdout, "isTTY", { value: true, writable: true }); - expect(resolveFormat(undefined)).toBe("PrettyCompactMonoBlock"); - Object.defineProperty(process.stdout, "isTTY", { value: orig, writable: true }); + expect(resolveFormat(undefined, true)).toBe("PrettyCompactMonoBlock"); }); test("defaults to TabSeparatedWithNames when stdout is not a TTY", () => { - const orig = process.stdout.isTTY; - Object.defineProperty(process.stdout, "isTTY", { value: undefined, writable: true }); - expect(resolveFormat(undefined)).toBe("TabSeparatedWithNames"); - Object.defineProperty(process.stdout, "isTTY", { value: orig, writable: true }); + expect(resolveFormat(undefined, false)).toBe("TabSeparatedWithNames"); }); }); diff --git a/src/format.ts b/src/format.ts index 5942a07..6262d7d 100644 --- a/src/format.ts +++ b/src/format.ts @@ -11,9 +11,12 @@ const FORMAT_ALIASES: Record = { sql: "SQLInsert", }; -export function resolveFormat(userFormat: string | undefined): string { +export function resolveFormat( + userFormat: string | undefined, + isTTY = !!process.stdout.isTTY, +): string { if (userFormat) { return FORMAT_ALIASES[userFormat.toLowerCase()] ?? userFormat; } - return process.stdout.isTTY ? "PrettyCompactMonoBlock" : "TabSeparatedWithNames"; + return isTTY ? "PrettyCompactMonoBlock" : "TabSeparatedWithNames"; }