Remove test flakiness via dependency injection

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<string, string | undefined> = {};
-  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<typeof resolveConnectionConfig>[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<typeof resolveConnectionConfig>[0];
 
-    const result = resolveConnectionConfig(config);
+    const result = resolveConnectionConfig(config, env);
     expect(result).toEqual({
       url: "https://flag-host:1234",
       username: "flag_user",

src/client.ts

@@ -11,37 +11,40 @@ 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<string, string | undefined> = 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";
 
   return {
     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",
   };
 }

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");
   });
 });

src/format.ts

@@ -11,9 +11,12 @@ const FORMAT_ALIASES: Record<string, string> = {
   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";
 }