Add tests

Author: EhabY

src/logging/formatters.ts

@@ -3,7 +3,7 @@ import prettyBytes from "pretty-bytes";
 
 import { sizeOf } from "./utils";
 
-import type { InternalAxiosRequestConfig } from "axios";
+import type { AxiosRequestConfig } from "axios";
 
 const SENSITIVE_HEADERS = ["Coder-Session-Token", "Proxy-Authorization"];
 
@@ -21,7 +21,7 @@ export function formatTime(ms: number): string {
 }
 
 export function formatMethod(method: string | undefined): string {
-	return (method ?? "GET").toUpperCase();
+	return (method ? method : "GET").toUpperCase();
 }
 
 /**
@@ -55,9 +55,7 @@ export function formatContentLength(
 	return "(? B)";
 }
 
-export function formatUri(
-	config: InternalAxiosRequestConfig | undefined,
-): string {
+export function formatUri(config: AxiosRequestConfig | undefined): string {
 	return config?.url || "<no url>";
 }
 

src/logging/utils.ts

@@ -5,6 +5,10 @@ export function shortId(id: string): string {
 	return id.slice(0, 8);
 }
 
+/**
+ * Returns the byte size of the data if it can be determined from the data's intrinsic properties,
+ * otherwise returns undefined (e.g., for plain objects and arrays that would require serialization).
+ */
 export function sizeOf(data: unknown): number | undefined {
 	if (data === null || data === undefined) {
 		return 0;

src/logging/wsLogger.ts

@@ -77,6 +77,6 @@ export class WsLogger {
 
 	private formatBytes(): string {
 		const bytes = prettyBytes(this.byteCount);
-		return this.unknownByteCount ? `>=${bytes}` : bytes;
+		return this.unknownByteCount ? `>= ${bytes}` : bytes;
 	}
 }

test/mocks/testHelpers.ts

@@ -1,6 +1,8 @@
 import { vi } from "vitest";
 import * as vscode from "vscode";
 
+import { type Logger } from "@/logging/logger";
+
 /**
  * Mock configuration provider that integrates with the vscode workspace configuration mock.
  * Use this to set configuration values that will be returned by vscode.workspace.getConfiguration().
@@ -286,3 +288,13 @@ export class InMemorySecretStorage implements vscode.SecretStorage {
 		this.listeners.forEach((listener) => listener(event));
 	}
 }
+
+export function createMockLogger(): Logger {
+	return {
+		trace: vi.fn(),
+		debug: vi.fn(),
+		info: vi.fn(),
+		warn: vi.fn(),
+		error: vi.fn(),
+	};
+}

test/unit/core/cliManager.test.ts

@@ -12,10 +12,10 @@ import * as vscode from "vscode";
 import { CliManager } from "@/core/cliManager";
 import * as cliUtils from "@/core/cliUtils";
 import { PathResolver } from "@/core/pathResolver";
-import { type Logger } from "@/logging/logger";
 import * as pgp from "@/pgp";
 
 import {
+	createMockLogger,
 	MockConfigurationProvider,
 	MockProgressReporter,
 	MockUserInteraction,
@@ -625,16 +625,6 @@ describe("CliManager", () => {
 		});
 	});
 
-	function createMockLogger(): Logger {
-		return {
-			trace: vi.fn(),
-			debug: vi.fn(),
-			info: vi.fn(),
-			warn: vi.fn(),
-			error: vi.fn(),
-		};
-	}
-
 	function createMockApi(version: string, url: string): Api {
 		const axios = {
 			defaults: { baseURL: url },

test/unit/logging/formatters.test.ts

@@ -0,0 +1,148 @@
+import { describe, expect, it } from "vitest";
+
+import {
+	formatBody,
+	formatContentLength,
+	formatHeaders,
+	formatMethod,
+	formatTime,
+	formatUri,
+} from "@/logging/formatters";
+
+describe("Logging formatters", () => {
+	it("formats time in appropriate units", () => {
+		expect(formatTime(500)).toBe("500ms");
+		expect(formatTime(1000)).toBe("1.00s");
+		expect(formatTime(5500)).toBe("5.50s");
+		expect(formatTime(60000)).toBe("1.00m");
+		expect(formatTime(150000)).toBe("2.50m");
+		expect(formatTime(3600000)).toBe("1.00h");
+		expect(formatTime(7255000)).toBe("2.02h");
+	});
+
+	describe("formatMethod", () => {
+		it("normalizes HTTP methods to uppercase", () => {
+			expect(formatMethod("get")).toBe("GET");
+			expect(formatMethod("post")).toBe("POST");
+			expect(formatMethod("PUT")).toBe("PUT");
+			expect(formatMethod("delete")).toBe("DELETE");
+		});
+
+		it("defaults to GET for falsy values", () => {
+			expect(formatMethod(undefined)).toBe("GET");
+			expect(formatMethod("")).toBe("GET");
+		});
+	});
+
+	describe("formatContentLength", () => {
+		it("uses content-length header when available", () => {
+			const result = formatContentLength({ "content-length": "1024" }, null);
+			expect(result).toContain("1.02 kB");
+		});
+
+		it("handles invalid content-length header", () => {
+			const result = formatContentLength({ "content-length": "invalid" }, null);
+			expect(result).toContain("?");
+		});
+
+		it.each([
+			["hello", 5],
+			[Buffer.from("test"), 4],
+			[123, 8],
+			[false, 8],
+			[BigInt(1234), 4],
+			[null, 0],
+			[undefined, 0],
+		])("calculates size for %s", (data: unknown, bytes: number) => {
+			const result = formatContentLength({}, data);
+			expect(result).toContain(`${bytes} B`);
+		});
+
+		it("estimates size for objects", () => {
+			const result = formatContentLength({}, { foo: "bar" });
+			expect(result).toMatch(/~\d+/);
+		});
+
+		it("handles circular references safely", () => {
+			const circular: Record<string, unknown> = { a: 1 };
+			circular.self = circular;
+			const result = formatContentLength({}, circular);
+			expect(result).toMatch(/~\d+/);
+		});
+	});
+
+	describe("formatUri", () => {
+		it("returns URL when present", () => {
+			expect(formatUri({ url: "https://example.com/api" })).toBe(
+				"https://example.com/api",
+			);
+			expect(formatUri({ url: "/relative/path" })).toBe("/relative/path");
+		});
+
+		it("returns placeholder for missing URL", () => {
+			expect(formatUri(undefined)).toContain("no url");
+			expect(formatUri({})).toContain("no url");
+			expect(formatUri({ url: "" })).toContain("no url");
+		});
+	});
+
+	describe("formatHeaders", () => {
+		it("formats headers as key-value pairs", () => {
+			const headers = {
+				"content-type": "application/json",
+				accept: "text/html",
+			};
+			const result = formatHeaders(headers);
+			expect(result).toContain("content-type: application/json");
+			expect(result).toContain("accept: text/html");
+		});
+
+		it("redacts sensitive headers", () => {
+			const sensitiveHeaders = ["Coder-Session-Token", "Proxy-Authorization"];
+
+			sensitiveHeaders.forEach((header) => {
+				const result = formatHeaders({ [header]: "secret-value" });
+				expect(result).toContain(`${header}: <redacted>`);
+				expect(result).not.toContain("secret-value");
+			});
+		});
+
+		it("returns placeholder for empty headers", () => {
+			expect(formatHeaders({})).toBe("<no headers>");
+		});
+	});
+
+	describe("formatBody", () => {
+		it("formats various body types", () => {
+			expect(formatBody({ key: "value" })).toContain("key: 'value'");
+			expect(formatBody("plain text")).toContain("plain text");
+			expect(formatBody([1, 2, 3])).toContain("1");
+			expect(formatBody(123)).toContain("123");
+			expect(formatBody(true)).toContain("true");
+		});
+
+		it("handles circular references gracefully", () => {
+			const circular: Record<string, unknown> = { a: 1 };
+			circular.self = circular;
+			const result = formatBody(circular);
+			expect(result).toBeTruthy();
+			expect(result).not.toContain("invalid body");
+			expect(result).toContain("a: 1");
+		});
+
+		it("handles deep nesting", () => {
+			const deep = {
+				level1: { level2: { level3: { level4: { value: "deep" } } } },
+			};
+			const result = formatBody(deep);
+			expect(result).toContain("level4: { value: 'deep' }");
+		});
+
+		it("returns placeholder for empty values", () => {
+			const emptyValues = [null, undefined, "", 0, false];
+			emptyValues.forEach((value) => {
+				expect(formatBody(value)).toContain("no body");
+			});
+		});
+	});
+});

test/unit/logging/httpLogger.test.ts

@@ -0,0 +1,112 @@
+import { AxiosError, type AxiosHeaders, type AxiosResponse } from "axios";
+import { describe, expect, it, vi } from "vitest";
+
+import {
+	createRequestMeta,
+	logError,
+	logRequest,
+	logResponse,
+} from "@/logging/httpLogger";
+import {
+	HttpClientLogLevel,
+	type RequestConfigWithMeta,
+} from "@/logging/types";
+
+import { createMockLogger } from "../../mocks/testHelpers";
+
+describe("REST HTTP Logger", () => {
+	describe("log level behavior", () => {
+		const config = {
+			method: "POST",
+			url: "https://api.example.com/endpoint",
+			headers: {
+				"content-type": "application/json",
+			} as unknown as AxiosHeaders,
+			data: { key: "value" },
+			metadata: createRequestMeta(),
+		} as RequestConfigWithMeta;
+
+		it("respects NONE level for trace logs", () => {
+			const logger = createMockLogger();
+
+			logRequest(logger, config, HttpClientLogLevel.NONE);
+			logResponse(
+				logger,
+				{ status: 200 } as AxiosResponse,
+				HttpClientLogLevel.NONE,
+			);
+			logError(logger, new Error("test"), HttpClientLogLevel.NONE);
+
+			expect(logger.trace).not.toHaveBeenCalled();
+			expect(logger.error).toHaveBeenCalled(); // always log errors
+		});
+
+		it("includes headers at HEADERS level but not at BASIC", () => {
+			const logger = createMockLogger();
+
+			logRequest(logger, config, HttpClientLogLevel.BASIC);
+			expect(logger.trace).not.toHaveBeenCalledWith(
+				expect.stringContaining("content-type"),
+			);
+
+			vi.clearAllMocks();
+			logRequest(logger, config, HttpClientLogLevel.HEADERS);
+			expect(logger.trace).toHaveBeenCalledWith(
+				expect.stringContaining("content-type"),
+			);
+		});
+
+		it("includes body at BODY level but not at HEADERS", () => {
+			const logger = createMockLogger();
+
+			logRequest(logger, config, HttpClientLogLevel.HEADERS);
+			expect(logger.trace).not.toHaveBeenCalledWith(
+				expect.stringContaining("key: 'value'"),
+			);
+
+			vi.clearAllMocks();
+			logRequest(logger, config, HttpClientLogLevel.BODY);
+			expect(logger.trace).toHaveBeenCalledWith(
+				expect.stringContaining("key: 'value'"),
+			);
+		});
+	});
+
+	describe("error handling", () => {
+		it("distinguishes between network errors and response errors", () => {
+			const logger = createMockLogger();
+
+			const networkError = new AxiosError("Some Network Error", "ECONNREFUSED");
+			networkError.config = {
+				metadata: createRequestMeta(),
+			} as RequestConfigWithMeta;
+
+			logError(logger, networkError, HttpClientLogLevel.BASIC);
+			expect(logger.error).toHaveBeenCalledWith(
+				expect.stringContaining("Some Network Error"),
+			);
+
+			// Response error (4xx/5xx)
+			vi.clearAllMocks();
+			const responseError = new AxiosError("Bad Request");
+			responseError.config = {
+				metadata: createRequestMeta(),
+			} as RequestConfigWithMeta;
+			responseError.response = { status: 400 } as AxiosResponse;
+
+			logError(logger, responseError, HttpClientLogLevel.BASIC);
+			expect(logger.error).toHaveBeenCalledWith(expect.stringContaining("400"));
+			expect(logger.error).toHaveBeenCalledWith(
+				expect.stringContaining("Bad Request"),
+			);
+		});
+
+		it("handles non-Axios errors", () => {
+			const logger = createMockLogger();
+			const error = new Error("Generic error");
+
+			logError(logger, error, HttpClientLogLevel.BASIC);
+			expect(logger.error).toHaveBeenCalledWith("Request error", error);
+		});
+	});
+});