diff --git a/AGENTS.md b/AGENTS.md index 85e27c83..48ec3ae5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -589,6 +589,31 @@ Use traditional unit tests only when: - Testing error messages or specific output formatting - Integration with external systems (E2E tests) +### Avoiding Unit/Property Test Duplication + +When a `*.property.test.ts` file exists for a module, **do not add unit tests that re-check the same invariants** with hardcoded examples. Before adding a unit test, check whether the companion property file already generates random inputs for that invariant. + +**Unit tests that belong alongside property tests:** +- Edge cases outside the property generator's range (e.g., self-hosted DSNs when the arbitrary only produces SaaS ones) +- Specific output format documentation (exact strings, column layouts, rendered vs plain mode) +- Concurrency/timing behavior that property tests cannot express +- Integration tests exercising multiple functions together (e.g., `writeJsonList` envelope shape) + +**Unit tests to avoid when property tests exist:** +- "returns true for valid input" / "returns false for invalid input" — the property test already covers this with random inputs +- Basic round-trip assertions — property tests check `decode(encode(x)) === x` for all `x` +- Hardcoded examples of invariants like idempotency, symmetry, or subset relationships + +When adding property tests for a function that already has unit tests, **remove the unit tests that become redundant**. Add a header comment to the unit test file noting which invariants live in the property file: + +```typescript +/** + * Note: Core invariants (round-trips, validation, ordering) are tested via + * property-based tests in foo.property.test.ts. These tests focus on edge + * cases and specific output formatting not covered by property generators. + */ +``` + ```typescript import { describe, expect, test, mock } from "bun:test"; diff --git a/test/lib/bspatch.test.ts b/test/lib/bspatch.test.ts index 6f842fe6..e72435e7 100644 --- a/test/lib/bspatch.test.ts +++ b/test/lib/bspatch.test.ts @@ -1,6 +1,11 @@ /** * Unit Tests for TRDIFF10 Binary Patch Application * + * Note: Core invariants (offtin sign/magnitude, parsePatchHeader non-negative + * values) are tested via property-based tests in bspatch.property.test.ts. + * These tests focus on fixture-based patch application, negative-value rejection + * (property generator clears sign bits), and error messages. + * * Tests the bspatch implementation against real TRDIFF10 patches * generated by zig-bsdiff v0.1.19 (stored as test fixtures). */ @@ -9,87 +14,11 @@ import { describe, expect, test } from "bun:test"; import { unlinkSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { applyPatch, offtin, parsePatchHeader } from "../../src/lib/bspatch.js"; +import { applyPatch, parsePatchHeader } from "../../src/lib/bspatch.js"; const FIXTURES_DIR = join(import.meta.dir, "../fixtures/patches"); -describe("offtin", () => { - test("reads zero", () => { - const buf = new Uint8Array(8); - expect(offtin(buf, 0)).toBe(0); - }); - - test("reads positive value 1", () => { - const buf = new Uint8Array(8); - buf[0] = 1; // LE: least significant byte - expect(offtin(buf, 0)).toBe(1); - }); - - test("reads positive value 256", () => { - const buf = new Uint8Array(8); - buf[1] = 1; // 256 in LE - expect(offtin(buf, 0)).toBe(256); - }); - - test("reads negative value -1", () => { - // zig-bsdiff sign-magnitude: magnitude=1, sign bit set in byte 7 - const buf = new Uint8Array(8); - buf[0] = 1; - buf[7] = 0x80; // Sign bit - expect(offtin(buf, 0)).toBe(-1); - }); - - test("reads negative value -256", () => { - const buf = new Uint8Array(8); - buf[1] = 1; // magnitude = 256 - buf[7] = 0x80; - expect(offtin(buf, 0)).toBe(-256); - }); - - test("reads from offset", () => { - const buf = new Uint8Array(16); - buf[8] = 42; - expect(offtin(buf, 8)).toBe(42); - }); - - test("reads large positive value", () => { - const buf = new Uint8Array(8); - // 0x01_00_00_00 = 16,777,216 in LE: byte 3 = 0x01 - buf[3] = 1; - expect(offtin(buf, 0)).toBe(16_777_216); - }); - - test("reads value spanning both 32-bit halves", () => { - const buf = new Uint8Array(8); - // Low 32 bits: 0 - // High 32 bits: 1 → value = 1 * 2^32 = 4,294,967,296 - buf[4] = 1; - expect(offtin(buf, 0)).toBe(4_294_967_296); - }); -}); - -describe("parsePatchHeader", () => { - test("rejects too-small buffer", () => { - expect(() => parsePatchHeader(new Uint8Array(31))).toThrow( - "Patch too small" - ); - }); - - test("rejects invalid magic", () => { - const buf = new Uint8Array(32); - buf.set(new TextEncoder().encode("BSDIFF40")); - expect(() => parsePatchHeader(buf)).toThrow("Invalid patch format"); - }); - - test("rejects negative header values", () => { - const buf = new Uint8Array(32); - buf.set(new TextEncoder().encode("TRDIFF10")); - // Set controlLen to -1 (sign bit in byte 15) - buf[8] = 1; - buf[15] = 0x80; - expect(() => parsePatchHeader(buf)).toThrow("negative length"); - }); - +describe("parsePatchHeader: fixtures", () => { test("parses valid small fixture header", async () => { const patchData = new Uint8Array( await Bun.file(join(FIXTURES_DIR, "small.trdiff10")).arrayBuffer() @@ -109,6 +38,26 @@ describe("parsePatchHeader", () => { expect(header.diffLen).toBeGreaterThanOrEqual(0); expect(header.newSize).toBe(65_536); }); + + test("rejects truncated patch (header claims more data than exists)", () => { + const truncated = new Uint8Array(32); + truncated.set(new TextEncoder().encode("TRDIFF10")); + truncated[8] = 100; // controlLen = 100 + expect(() => parsePatchHeader(truncated)).toThrow("exceed file size"); + }); + + test("rejects negative header values", () => { + // Property generator clears sign bits (patched[15] %= 128) so never + // exercises the negative-length error path — this test fills that gap + const buf = new Uint8Array(64); + buf.set(new TextEncoder().encode("TRDIFF10")); + // offtin reads sign from bit 7 of byte 7 within each 8-byte field. + // controlLen is at offset 8, so sign bit is byte 15. + // Need non-zero magnitude + sign bit for a negative result. + buf[8] = 1; // non-zero low byte → magnitude > 0 + buf[15] = 0x80; // Set sign bit → negative controlLen + expect(() => parsePatchHeader(buf)).toThrow("negative"); + }); }); describe("applyPatch", () => { @@ -126,14 +75,12 @@ describe("applyPatch", () => { try { const sha256 = await applyPatch(oldPath, patchData, destPath); - // Verify output matches expected new file const expected = await Bun.file( join(FIXTURES_DIR, "small-new.bin") ).arrayBuffer(); const actual = await Bun.file(destPath).arrayBuffer(); expect(new Uint8Array(actual)).toEqual(new Uint8Array(expected)); - // Verify SHA-256 matches const expectedHash = new Bun.CryptoHasher("sha256") .update(new Uint8Array(expected)) .digest("hex"); @@ -185,7 +132,6 @@ describe("applyPatch", () => { try { const sha256 = await applyPatch(oldPath, patchData, destPath); - // Known SHA-256 of "Hello, World! This is the new file content. Version 2." expect(sha256).toMatch(/^[0-9a-f]{64}$/); expect(sha256.length).toBe(64); } finally { @@ -197,15 +143,6 @@ describe("applyPatch", () => { } }); - test("rejects truncated patch (header claims more data than exists)", () => { - // Header says controlLen=100 but patch is only 32 bytes - const truncated = new Uint8Array(32); - truncated.set(new TextEncoder().encode("TRDIFF10")); - truncated[8] = 100; // controlLen = 100 - - expect(() => parsePatchHeader(truncated)).toThrow("exceed file size"); - }); - test("rejects non-TRDIFF10 magic", async () => { const oldPath = join(FIXTURES_DIR, "small-old.bin"); const buf = new Uint8Array(64); diff --git a/test/lib/db/auth.test.ts b/test/lib/db/auth.test.ts index 25f96b97..87732a5a 100644 --- a/test/lib/db/auth.test.ts +++ b/test/lib/db/auth.test.ts @@ -1,13 +1,14 @@ /** * Auth Environment Variable Tests * - * Tests for SENTRY_AUTH_TOKEN and SENTRY_TOKEN env var support. - * Verifies priority, source tracking, and interaction with stored tokens. + * Note: Core invariants (priority, source tracking, refresh skip, isEnvTokenActive) + * are tested via property-based tests in auth.property.test.ts. These tests focus on + * edge cases (whitespace, empty strings), shape assertions, and functions not covered + * by property tests (isAuthenticated, getActiveEnvVarName). */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { - type AuthSource, getActiveEnvVarName, getAuthConfig, getAuthToken, @@ -20,7 +21,6 @@ import { useTestConfigDir } from "../../helpers.js"; useTestConfigDir("auth-env-"); -/** Save and restore env vars around each test */ let savedAuthToken: string | undefined; let savedSentryToken: string | undefined; @@ -32,7 +32,6 @@ beforeEach(() => { }); afterEach(() => { - // Restore — never leave env vars dangling if (savedAuthToken !== undefined) { process.env.SENTRY_AUTH_TOKEN = savedAuthToken; } else { @@ -45,34 +44,7 @@ afterEach(() => { } }); -describe("env var auth: getAuthToken", () => { - test("returns SENTRY_AUTH_TOKEN when set", () => { - process.env.SENTRY_AUTH_TOKEN = "sntrys_token_abc"; - expect(getAuthToken()).toBe("sntrys_token_abc"); - }); - - test("returns SENTRY_TOKEN when set", () => { - process.env.SENTRY_TOKEN = "sntrys_token_xyz"; - expect(getAuthToken()).toBe("sntrys_token_xyz"); - }); - - test("SENTRY_AUTH_TOKEN takes priority over SENTRY_TOKEN", () => { - process.env.SENTRY_AUTH_TOKEN = "auth_token"; - process.env.SENTRY_TOKEN = "sentry_token"; - expect(getAuthToken()).toBe("auth_token"); - }); - - test("env var takes priority over stored token", () => { - setAuthToken("stored_token"); - process.env.SENTRY_AUTH_TOKEN = "env_token"; - expect(getAuthToken()).toBe("env_token"); - }); - - test("falls back to stored token when no env var", () => { - setAuthToken("stored_token"); - expect(getAuthToken()).toBe("stored_token"); - }); - +describe("env var auth: getAuthToken edge cases", () => { test("ignores empty SENTRY_AUTH_TOKEN", () => { process.env.SENTRY_AUTH_TOKEN = ""; setAuthToken("stored_token"); @@ -91,30 +63,7 @@ describe("env var auth: getAuthToken", () => { }); }); -describe("env var auth: getAuthConfig", () => { - test("returns env source for SENTRY_AUTH_TOKEN", () => { - process.env.SENTRY_AUTH_TOKEN = "test_token"; - const config = getAuthConfig(); - expect(config).toBeDefined(); - expect(config?.token).toBe("test_token"); - expect(config?.source).toBe("env:SENTRY_AUTH_TOKEN" satisfies AuthSource); - }); - - test("returns env source for SENTRY_TOKEN", () => { - process.env.SENTRY_TOKEN = "test_token"; - const config = getAuthConfig(); - expect(config).toBeDefined(); - expect(config?.token).toBe("test_token"); - expect(config?.source).toBe("env:SENTRY_TOKEN" satisfies AuthSource); - }); - - test("returns oauth source for stored token", () => { - setAuthToken("stored_token"); - const config = getAuthConfig(); - expect(config).toBeDefined(); - expect(config?.source).toBe("oauth" satisfies AuthSource); - }); - +describe("env var auth: getAuthConfig shape", () => { test("env config has no refreshToken or expiry", () => { process.env.SENTRY_AUTH_TOKEN = "test_token"; const config = getAuthConfig(); @@ -135,21 +84,7 @@ describe("env var auth: isAuthenticated", () => { }); }); -describe("env var auth: isEnvTokenActive", () => { - test("returns true for SENTRY_AUTH_TOKEN", () => { - process.env.SENTRY_AUTH_TOKEN = "test_token"; - expect(isEnvTokenActive()).toBe(true); - }); - - test("returns true for SENTRY_TOKEN", () => { - process.env.SENTRY_TOKEN = "test_token"; - expect(isEnvTokenActive()).toBe(true); - }); - - test("returns false when no env var set", () => { - expect(isEnvTokenActive()).toBe(false); - }); - +describe("env var auth: isEnvTokenActive edge case", () => { test("returns false for empty env var", () => { process.env.SENTRY_AUTH_TOKEN = ""; expect(isEnvTokenActive()).toBe(false); @@ -178,23 +113,8 @@ describe("env var auth: getActiveEnvVarName", () => { }); }); -describe("env var auth: refreshToken", () => { - test("returns env token without refreshing", async () => { - process.env.SENTRY_AUTH_TOKEN = "env_token"; - const result = await refreshToken(); - expect(result.token).toBe("env_token"); - expect(result.refreshed).toBe(false); - }); - - test("returns env token even with force=true", async () => { - process.env.SENTRY_AUTH_TOKEN = "env_token"; - const result = await refreshToken({ force: true }); - expect(result.token).toBe("env_token"); - expect(result.refreshed).toBe(false); - }); - +describe("env var auth: refreshToken edge cases", () => { test("env token skips stored token entirely", async () => { - // Store a token that would require refresh (expired) setAuthToken("stored_token", -1, "refresh_token"); process.env.SENTRY_AUTH_TOKEN = "env_token"; const result = await refreshToken(); diff --git a/test/lib/dsn.test.ts b/test/lib/dsn.test.ts index 49cd93a9..ff2f8fc8 100644 --- a/test/lib/dsn.test.ts +++ b/test/lib/dsn.test.ts @@ -1,7 +1,9 @@ /** * DSN Parsing Tests * - * Unit tests for DSN parsing and validation logic. + * Note: Core invariants (parsing, validation, round-trips, fingerprint ordering/dedup) + * are tested via property-based tests in dsn.property.test.ts. These tests focus on + * edge cases and self-hosted DSN behavior that property generators don't cover. */ import { describe, expect, test } from "bun:test"; @@ -9,170 +11,35 @@ import type { DetectedDsn } from "../../src/lib/dsn/index.js"; import { createDetectedDsn, createDsnFingerprint, - extractOrgIdFromHost, inferPackagePath, - isValidDsn, parseDsn, } from "../../src/lib/dsn/index.js"; -describe("extractOrgIdFromHost", () => { - test("extracts org ID from US ingest host", () => { - const result = extractOrgIdFromHost("o1169445.ingest.us.sentry.io"); - expect(result).toBe("1169445"); - }); - - test("extracts org ID from EU ingest host", () => { - const result = extractOrgIdFromHost("o123.ingest.de.sentry.io"); - expect(result).toBe("123"); - }); - - test("extracts org ID from default ingest host", () => { - const result = extractOrgIdFromHost("o999.ingest.sentry.io"); - expect(result).toBe("999"); - }); - - test("returns null for self-hosted host", () => { - const result = extractOrgIdFromHost("sentry.mycompany.com"); - expect(result).toBeNull(); - }); - - test("returns null for non-Sentry host", () => { - const result = extractOrgIdFromHost("example.com"); - expect(result).toBeNull(); - }); -}); - -describe("parseDsn", () => { - test("parses valid SaaS DSN with org ID", () => { - const dsn = "https://abc123@o1169445.ingest.us.sentry.io/4505229541441536"; - const result = parseDsn(dsn); - - expect(result).toEqual({ - protocol: "https", - publicKey: "abc123", - host: "o1169445.ingest.us.sentry.io", - projectId: "4505229541441536", - orgId: "1169445", - }); - }); - - test("parses valid self-hosted DSN without org ID", () => { - const dsn = "https://abc123@sentry.mycompany.com/1"; - const result = parseDsn(dsn); - - expect(result).toEqual({ - protocol: "https", - publicKey: "abc123", - host: "sentry.mycompany.com", - projectId: "1", - orgId: undefined, - }); - }); - - test("parses DSN with http protocol", () => { - const dsn = "http://key@o123.ingest.sentry.io/456"; - const result = parseDsn(dsn); - - expect(result).toEqual({ - protocol: "http", - publicKey: "key", - host: "o123.ingest.sentry.io", - projectId: "456", - orgId: "123", - }); - }); - - test("returns null for DSN without public key", () => { - const dsn = "https://sentry.io/123"; - const result = parseDsn(dsn); - expect(result).toBeNull(); - }); - - test("returns null for DSN without project ID", () => { +describe("parseDsn edge cases", () => { + test("returns null for DSN without project ID (trailing slash)", () => { const dsn = "https://key@o123.ingest.sentry.io/"; const result = parseDsn(dsn); expect(result).toBeNull(); }); - test("returns null for invalid URL", () => { - const dsn = "not-a-url"; - const result = parseDsn(dsn); - expect(result).toBeNull(); - }); - - test("returns null for empty string", () => { - const result = parseDsn(""); - expect(result).toBeNull(); - }); - test("handles DSN with path segments", () => { const dsn = "https://key@o123.ingest.sentry.io/api/456"; const result = parseDsn(dsn); - expect(result?.projectId).toBe("456"); }); }); -describe("isValidDsn", () => { - test("returns true for valid SaaS DSN", () => { - const dsn = "https://abc123@o1169445.ingest.us.sentry.io/4505229541441536"; - expect(isValidDsn(dsn)).toBe(true); - }); - - test("returns true for valid self-hosted DSN", () => { - const dsn = "https://abc123@sentry.mycompany.com/1"; - expect(isValidDsn(dsn)).toBe(true); - }); - - test("returns false for invalid DSN", () => { - const dsn = "https://sentry.io/123"; - expect(isValidDsn(dsn)).toBe(false); - }); - - test("returns false for non-URL string", () => { - const dsn = "not-a-url"; - expect(isValidDsn(dsn)).toBe(false); - }); -}); - -describe("createDsnFingerprint", () => { - /** Helper to create a minimal DetectedDsn for testing */ - function makeDsn(orgId: string, projectId: string): DetectedDsn { - return { - raw: `https://key@o${orgId}.ingest.sentry.io/${projectId}`, +describe("createDsnFingerprint: self-hosted DSNs", () => { + test("includes DSNs without orgId using host as prefix", () => { + const saas: DetectedDsn = { + raw: "https://key@o123.ingest.sentry.io/456", protocol: "https", publicKey: "key", - host: `o${orgId}.ingest.sentry.io`, - projectId, - orgId, + host: "o123.ingest.sentry.io", + projectId: "456", + orgId: "123", source: "env", }; - } - - test("creates fingerprint from multiple DSNs", () => { - const dsns = [makeDsn("123", "456"), makeDsn("123", "789")]; - const result = createDsnFingerprint(dsns); - expect(result).toBe("123:456,123:789"); - }); - - test("sorts fingerprint alphabetically", () => { - const dsns = [makeDsn("999", "111"), makeDsn("123", "456")]; - const result = createDsnFingerprint(dsns); - expect(result).toBe("123:456,999:111"); - }); - - test("deduplicates same DSN from multiple sources", () => { - const dsn1 = makeDsn("123", "456"); - dsn1.source = "env"; - const dsn2 = makeDsn("123", "456"); - dsn2.source = "file"; - - const result = createDsnFingerprint([dsn1, dsn2]); - expect(result).toBe("123:456"); - }); - - test("includes DSNs without orgId using host as prefix (self-hosted)", () => { - const saas = makeDsn("123", "456"); const selfHosted: DetectedDsn = { raw: "https://key@sentry.mycompany.com/1", protocol: "https", @@ -184,15 +51,9 @@ describe("createDsnFingerprint", () => { }; const result = createDsnFingerprint([saas, selfHosted]); - // Self-hosted uses host:projectId, SaaS uses orgId:projectId expect(result).toBe("123:456,sentry.mycompany.com:1"); }); - test("returns empty string for empty array", () => { - const result = createDsnFingerprint([]); - expect(result).toBe(""); - }); - test("returns host-based fingerprint for self-hosted DSNs", () => { const selfHosted: DetectedDsn = { raw: "https://key@sentry.mycompany.com/1", @@ -209,36 +70,7 @@ describe("createDsnFingerprint", () => { }); }); -describe("createDetectedDsn", () => { - test("creates DetectedDsn from valid DSN string", () => { - const result = createDetectedDsn( - "https://abc123@o1169445.ingest.us.sentry.io/4505229541441536", - "env" - ); - - expect(result).toEqual({ - raw: "https://abc123@o1169445.ingest.us.sentry.io/4505229541441536", - protocol: "https", - publicKey: "abc123", - host: "o1169445.ingest.us.sentry.io", - projectId: "4505229541441536", - orgId: "1169445", - source: "env", - sourcePath: undefined, - packagePath: undefined, - }); - }); - - test("includes sourcePath when provided", () => { - const result = createDetectedDsn( - "https://abc123@o123.ingest.sentry.io/456", - "code", - "src/config.ts" - ); - - expect(result?.sourcePath).toBe("src/config.ts"); - }); - +describe("createDetectedDsn edge cases", () => { test("includes packagePath when provided", () => { const result = createDetectedDsn( "https://abc123@o123.ingest.sentry.io/456", @@ -246,59 +78,31 @@ describe("createDetectedDsn", () => { "packages/web/src/config.ts", "packages/web" ); - expect(result?.packagePath).toBe("packages/web"); }); - - test("returns null for invalid DSN", () => { - const result = createDetectedDsn("not-a-valid-dsn", "env"); - expect(result).toBeNull(); - }); - - test("returns null for DSN without public key", () => { - const result = createDetectedDsn("https://sentry.io/123", "env"); - expect(result).toBeNull(); - }); }); describe("inferPackagePath", () => { test("infers package path from packages/ directory", () => { - const result = inferPackagePath("packages/frontend/src/index.ts"); - expect(result).toBe("packages/frontend"); + expect(inferPackagePath("packages/frontend/src/index.ts")).toBe( + "packages/frontend" + ); }); test("infers package path from apps/ directory", () => { - const result = inferPackagePath("apps/web/.env"); - expect(result).toBe("apps/web"); - }); - - test("infers package path from libs/ directory", () => { - const result = inferPackagePath("libs/shared/config.ts"); - expect(result).toBe("libs/shared"); - }); - - test("infers package path from modules/ directory", () => { - const result = inferPackagePath("modules/auth/index.ts"); - expect(result).toBe("modules/auth"); + expect(inferPackagePath("apps/web/.env")).toBe("apps/web"); }); test("infers package path from services/ directory", () => { - const result = inferPackagePath("services/api/server.ts"); - expect(result).toBe("services/api"); + expect(inferPackagePath("services/api/server.ts")).toBe("services/api"); }); - test("returns undefined for root project files", () => { - const result = inferPackagePath("src/index.ts"); - expect(result).toBeUndefined(); - }); - - test("returns undefined for top-level files", () => { - const result = inferPackagePath(".env"); - expect(result).toBeUndefined(); + test("infers package path from modules/ directory", () => { + // Property generator only uses packages/apps/services/libs — modules/ is not covered + expect(inferPackagePath("modules/auth/index.ts")).toBe("modules/auth"); }); test("returns undefined for non-monorepo directories", () => { - const result = inferPackagePath("other/path/file.ts"); - expect(result).toBeUndefined(); + expect(inferPackagePath("other/path/file.ts")).toBeUndefined(); }); }); diff --git a/test/lib/formatters/json.test.ts b/test/lib/formatters/json.test.ts index b72dc4ba..da77b52c 100644 --- a/test/lib/formatters/json.test.ts +++ b/test/lib/formatters/json.test.ts @@ -1,5 +1,11 @@ /** * Unit tests for JSON formatting and field filtering. + * + * Note: Core invariants (idempotency, subset property, missing field handling, + * primitive pass-through, array element filtering, deduplication, whitespace + * tolerance) are tested via property-based tests in json.property.test.ts. + * These tests focus on specific output documentation, edge cases, and the + * writeJson/writeJsonList/formatJson APIs not covered by property tests. */ import { describe, expect, test } from "bun:test"; @@ -19,23 +25,7 @@ function filter(data: unknown, fields: string[]): unknown { return filterFields(data, fields); } -// --------------------------------------------------------------------------- -// filterFields — unit tests for specific behaviors -// --------------------------------------------------------------------------- - -describe("filterFields", () => { - test("picks top-level fields", () => { - expect( - filter({ id: 1, title: "bug", status: "open", count: 42 }, [ - "id", - "title", - ]) - ).toEqual({ - id: 1, - title: "bug", - }); - }); - +describe("filterFields edge cases", () => { test("picks nested fields via dot-notation", () => { const data = { id: 1, @@ -51,18 +41,6 @@ describe("filterFields", () => { }); }); - test("silently skips missing fields", () => { - expect( - filter({ id: 1, title: "bug" }, ["id", "nonexistent", "also.missing"]) - ).toEqual({ - id: 1, - }); - }); - - test("handles empty fields list", () => { - expect(filter({ a: 1, b: 2 }, [])).toEqual({}); - }); - test("preserves null values", () => { expect(filter({ id: 1, value: null }, ["id", "value"])).toEqual({ id: 1, @@ -77,21 +55,6 @@ describe("filterFields", () => { }); }); - test("handles arrays of objects", () => { - const data = [ - { id: 1, title: "first", extra: true }, - { id: 2, title: "second", extra: false }, - ]; - expect(filter(data, ["id", "title"])).toEqual([ - { id: 1, title: "first" }, - { id: 2, title: "second" }, - ]); - }); - - test("handles empty array", () => { - expect(filter([], ["id"])).toEqual([]); - }); - test("stops at null intermediate in dot-path", () => { expect(filter({ a: { b: null } }, ["a.b.c"])).toEqual({}); }); @@ -100,22 +63,6 @@ describe("filterFields", () => { expect(filter({ a: { b: 42 } }, ["a.b.c"])).toEqual({}); }); - test("passes through null data unchanged", () => { - expect(filter(null, ["id"])).toBeNull(); - }); - - test("passes through undefined data unchanged", () => { - expect(filter(undefined, ["id"])).toBeUndefined(); - }); - - test("passes through string data unchanged", () => { - expect(filter("hello", ["length"])).toBe("hello"); - }); - - test("passes through number data unchanged", () => { - expect(filter(42, ["id"])).toBe(42); - }); - test("merges nested paths into shared parents", () => { expect( filter({ user: { name: "Alice", email: "alice@example.com", age: 30 } }, [ @@ -152,10 +99,6 @@ describe("filterFields", () => { }); }); -// --------------------------------------------------------------------------- -// parseFieldsList — unit tests -// --------------------------------------------------------------------------- - describe("parseFieldsList", () => { test("parses comma-separated fields", () => { expect(parseFieldsList("id,title,status")).toEqual([ @@ -179,14 +122,6 @@ describe("parseFieldsList", () => { ); }); - test("deduplicates fields", () => { - expect(parseFieldsList("id,title,id,title")).toEqual(["id", "title"]); - }); - - test("filters empty segments from double commas", () => { - expect(parseFieldsList("id,,title")).toEqual(["id", "title"]); - }); - test("handles single field", () => { expect(parseFieldsList("id")).toEqual(["id"]); }); @@ -194,22 +129,9 @@ describe("parseFieldsList", () => { test("returns empty array for empty string", () => { expect(parseFieldsList("")).toEqual([]); }); - - test("returns empty array for whitespace-only", () => { - expect(parseFieldsList(" ")).toEqual([]); - }); - - test("returns empty array for commas-only", () => { - expect(parseFieldsList(",,,")).toEqual([]); - }); }); -// --------------------------------------------------------------------------- -// writeJson with fields — integration tests -// --------------------------------------------------------------------------- - describe("writeJson with fields", () => { - /** Capture stdout writes */ function capture(): { writer: { write: (s: string) => void }; output: () => string; @@ -288,10 +210,6 @@ describe("writeJson with fields", () => { }); }); -// --------------------------------------------------------------------------- -// formatJson — basic sanity -// --------------------------------------------------------------------------- - describe("formatJson", () => { test("pretty-prints with 2-space indentation", () => { expect(formatJson({ a: 1 })).toBe('{\n "a": 1\n}'); @@ -306,12 +224,7 @@ describe("formatJson", () => { }); }); -// --------------------------------------------------------------------------- -// writeJsonList — paginated list output with per-element field filtering -// --------------------------------------------------------------------------- - describe("writeJsonList", () => { - /** Capture stdout writes and parse the result as JSON */ function capture(): { writer: { write: (s: string) => void }; output: () => string; @@ -417,7 +330,6 @@ describe("writeJsonList", () => { { id: 1, title: "Bug" }, { id: 2, title: "Feature" }, ]); - // Wrapper metadata is always preserved expect(result.hasMore).toBe(true); }); @@ -429,7 +341,6 @@ describe("writeJsonList", () => { fields: ["id"], }); const result = parsed() as Record; - // Only array elements are filtered — wrapper keys preserved expect(result.data).toEqual([{ id: 1 }]); expect(result.hasMore).toBe(true); expect(result.nextCursor).toBe("xyz"); @@ -487,9 +398,7 @@ describe("writeJsonList", () => { extra: { hint: "use --cursor" }, }); const result = parsed() as Record; - // Items are filtered expect(result.data).toEqual([{ id: 1 }]); - // Extra is in wrapper, not filtered expect(result.hint).toBe("use --cursor"); }); diff --git a/test/lib/formatters/trace.test.ts b/test/lib/formatters/trace.test.ts index dcfcc6a1..b4116ea6 100644 --- a/test/lib/formatters/trace.test.ts +++ b/test/lib/formatters/trace.test.ts @@ -1,9 +1,10 @@ /** * Unit Tests for Trace Formatters * - * Tests for formatTraceDuration, formatTraceTable, - formatTracesHeader, formatTraceRow, - * computeTraceSummary, and formatTraceSummary. + * Note: Core invariants (duration formatting, trace ID containment, row newline + * termination, determinism, span counting) are tested via property-based tests + * in trace.property.test.ts. These tests focus on specific format output values, + * rendered vs plain mode behavior, header newline termination, and edge cases. */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; @@ -138,20 +139,14 @@ describe("formatTracesHeader (rendered mode)", () => { }); test("ends with newline", () => { - const header = formatTracesHeader(); - expect(header.endsWith("\n")).toBe(true); + // Property test only checks Row newlines, not Header + expect(formatTracesHeader().endsWith("\n")).toBe(true); }); }); describe("formatTraceRow (rendered mode)", () => { useRenderedMode(); - test("includes trace ID", () => { - const traceId = "a".repeat(32); - const row = formatTraceRow(makeTransaction({ trace: traceId })); - expect(row).toContain(traceId); - }); - test("includes transaction name", () => { const row = formatTraceRow( makeTransaction({ transaction: "POST /api/data" }) @@ -169,7 +164,6 @@ describe("formatTraceRow (rendered mode)", () => { test("includes full transaction name in markdown row", () => { const longName = "A".repeat(50); const row = formatTraceRow(makeTransaction({ transaction: longName })); - // Full name preserved in markdown table cell expect(row).toContain(longName); }); @@ -177,11 +171,6 @@ describe("formatTraceRow (rendered mode)", () => { const row = formatTraceRow(makeTransaction({ transaction: "" })); expect(row).toContain("unknown"); }); - - test("ends with newline", () => { - const row = formatTraceRow(makeTransaction()); - expect(row.endsWith("\n")).toBe(true); - }); }); describe("formatTracesHeader (plain mode)", () => { @@ -189,7 +178,6 @@ describe("formatTracesHeader (plain mode)", () => { test("emits markdown table header and separator", () => { const result = formatTracesHeader(); - // Plain mode produces mdTableHeader output (no bold markup), with separator expect(result).toContain("| Trace ID | Transaction | Duration | When |"); // Duration column is right-aligned (`:` suffix in TRACE_TABLE_COLS) expect(result).toContain("| --- | --- | ---: | --- |"); @@ -208,12 +196,6 @@ describe("formatTraceRow (plain mode)", () => { expect(row).toMatch(/^\|.+\|.+\|.+\|.+\|\n$/); }); - test("includes trace ID", () => { - const traceId = "a".repeat(32); - const row = formatTraceRow(makeTransaction({ trace: traceId })); - expect(row).toContain(traceId); - }); - test("includes transaction name", () => { const row = formatTraceRow( makeTransaction({ transaction: "POST /api/data" }) @@ -243,11 +225,6 @@ describe("formatTraceRow (plain mode)", () => { const row = formatTraceRow(makeTransaction({ transaction: "" })); expect(row).toContain("unknown"); }); - - test("ends with newline", () => { - const row = formatTraceRow(makeTransaction()); - expect(row).toEndWith("\n"); - }); }); describe("computeTraceSummary", () => { @@ -316,6 +293,13 @@ describe("computeTraceSummary", () => { expect(Number.isNaN(summary.duration)).toBe(true); }); + test("returns NaN duration for empty spans array", () => { + // Property generator uses minLength:1, so empty array is never tested there + const summary = computeTraceSummary("trace-id", []); + expect(Number.isNaN(summary.duration)).toBe(true); + expect(summary.spanCount).toBe(0); + }); + test("ignores zero timestamps in min/max calculations", () => { const spans: TraceSpan[] = [ makeSpan({ start_timestamp: 0, timestamp: 0 }), @@ -326,12 +310,6 @@ describe("computeTraceSummary", () => { expect(summary.duration).toBe(2000); }); - test("returns NaN duration for empty spans array", () => { - const summary = computeTraceSummary("trace-id", []); - expect(Number.isNaN(summary.duration)).toBe(true); - expect(summary.spanCount).toBe(0); - }); - test("falls back to timestamp when end_timestamp is 0", () => { // end_timestamp: 0 should be treated as missing, falling back to timestamp const spans: TraceSpan[] = [ @@ -426,11 +404,6 @@ describe("formatTraceSummary", () => { }); describe("formatTraceTable", () => { - test("returns a string", () => { - const result = formatTraceTable([makeTransaction()]); - expect(typeof result).toBe("string"); - }); - test("includes all transaction names", () => { const items = [ makeTransaction({ transaction: "GET /api/users" }), diff --git a/test/lib/promises.test.ts b/test/lib/promises.test.ts index 1a9fda72..fdf16bd0 100644 --- a/test/lib/promises.test.ts +++ b/test/lib/promises.test.ts @@ -1,75 +1,15 @@ /** * Promise Utilities Tests + * + * Note: Core behavior invariants (true/false results, error handling, empty arrays) + * are tested via property-based tests in promises.property.test.ts. These tests + * focus on concurrency and timing behavior that property tests cannot easily verify. */ import { describe, expect, test } from "bun:test"; import { anyTrue } from "../../src/lib/promises.js"; -describe("anyTrue", () => { - test("returns false for empty array", async () => { - const result = await anyTrue([], async () => true); - expect(result).toBe(false); - }); - - test("returns true when single item passes", async () => { - const result = await anyTrue([1], async () => true); - expect(result).toBe(true); - }); - - test("returns false when single item fails", async () => { - const result = await anyTrue([1], async () => false); - expect(result).toBe(false); - }); - - test("returns true when first item passes quickly", async () => { - const calls: number[] = []; - - const result = await anyTrue([1, 2, 3], async (n) => { - calls.push(n); - if (n === 1) { - return true; // First item passes immediately - } - await Bun.sleep(100); // Others are slow - return false; - }); - - expect(result).toBe(true); - // First item should have been called - expect(calls).toContain(1); - }); - - test("returns true when last item passes", async () => { - const result = await anyTrue([1, 2, 3], async (n) => { - return n === 3; // Only last item passes - }); - - expect(result).toBe(true); - }); - - test("returns false when all items fail", async () => { - const result = await anyTrue([1, 2, 3], async () => false); - expect(result).toBe(false); - }); - - test("treats errors as false", async () => { - const result = await anyTrue([1, 2, 3], async (n) => { - if (n === 1) { - throw new Error("test error"); - } - return n === 2; // Second item passes - }); - - expect(result).toBe(true); - }); - - test("returns false when all predicates error", async () => { - const result = await anyTrue([1, 2, 3], async () => { - throw new Error("test error"); - }); - - expect(result).toBe(false); - }); - +describe("anyTrue concurrency", () => { test("starts all predicates concurrently", async () => { const startTimes: number[] = []; const startTime = Date.now(); @@ -104,18 +44,6 @@ describe("anyTrue", () => { expect(resolveCount).toBe(1); }); - test("works with complex async predicates", async () => { - const files = ["exists.txt", "missing.txt", "also-missing.txt"]; - - const result = await anyTrue(files, async (filename) => { - // Simulate async file check - await Bun.sleep(5); - return filename === "exists.txt"; - }); - - expect(result).toBe(true); - }); - test("does not wait for slow false predicates after finding true", async () => { const startTime = Date.now();