Skip to content

Commit 311201d

Browse files
committed
Replace the tautological session-id re-creation test with a real one
The prior test called createTelemetry() twice but asserted against getSessionId(), which never reads from a Telemetry instance — it would have passed even if createTelemetry() were never called. The replacement drives the actual toggle.ts enable/disable/enable cycle and asserts session_id stays constant across the captured payloads.
1 parent 3f26458 commit 311201d

2 files changed

Lines changed: 37 additions & 9 deletions

File tree

tests/unit/telemetry-toggle.test.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { test, expect } from "bun:test";
22
import { createTelemetryToggleHandler, type TelemetryToggleDeps } from "../../src/telemetry/toggle.js";
3-
import { createTelemetry } from "../../src/telemetry/index.js";
3+
import { createTelemetry, getSessionId } from "../../src/telemetry/index.js";
44
import type { Settings } from "../../src/config/settings.js";
55
import type { Telemetry } from "../../src/telemetry/index.js";
66

@@ -178,3 +178,39 @@ test("toggle on re-enables after settings load/save resolve", async () => {
178178
expect(saved?.telemetry?.enabled).toBe(true);
179179
expect(getInstance().enabled).toBe(true);
180180
});
181+
182+
test("session_id on captured payloads stays constant across an enable/disable/enable toggle cycle", async () => {
183+
const capturedBodies: { properties: Record<string, unknown> }[] = [];
184+
const fetchFn = ((_url: string, init: RequestInit) => {
185+
capturedBodies.push(JSON.parse(init.body as string));
186+
return Promise.resolve(new Response("1", { status: 200 }));
187+
}) as unknown as typeof fetch;
188+
const { deps, getInstance } = fakeDeps({
189+
createTelemetry: (opts) =>
190+
createTelemetry({ ...opts, env: opts.env ?? {}, apiKey: opts.apiKey ?? "test-key", fetchFn }),
191+
});
192+
const handler = createTelemetryToggleHandler("/fake/path", deps);
193+
194+
// Re-enable once up front so the captured instance is one built through
195+
// deps.createTelemetry (and thus fetchFn) rather than fakeDeps' bootstrap
196+
// instance, which is wired to its own separate fetch counter.
197+
handler(true);
198+
await new Promise((resolve) => setTimeout(resolve, 10));
199+
getInstance().capture("cli_start");
200+
201+
handler(false);
202+
await new Promise((resolve) => setTimeout(resolve, 10));
203+
getInstance().capture("cli_start"); // disabled: no fetch, but proves the swapped instance is live
204+
205+
handler(true);
206+
await new Promise((resolve) => setTimeout(resolve, 10));
207+
getInstance().capture("cli_start");
208+
209+
await new Promise((resolve) => setTimeout(resolve, 0));
210+
expect(capturedBodies.length).toBe(2);
211+
const sessionId = capturedBodies[0].properties.session_id;
212+
expect(typeof sessionId).toBe("string");
213+
expect((sessionId as string).length).toBeGreaterThan(0);
214+
expect(capturedBodies[1].properties.session_id).toBe(sessionId);
215+
expect(sessionId).toBe(getSessionId());
216+
});

tests/unit/telemetry.test.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -269,14 +269,6 @@ test("capture attaches the same session_id across multiple events in one process
269269
expect(sessionId).toBe(getSessionId());
270270
});
271271

272-
test("getSessionId is stable across separate createTelemetry instances in the same process", () => {
273-
const first = createTelemetry({ settings: settingsWith("id"), env: {}, apiKey: "" });
274-
const second = createTelemetry({ settings: settingsWith("id"), env: {}, apiKey: "" });
275-
void first;
276-
void second;
277-
expect(getSessionId()).toBe(getSessionId());
278-
});
279-
280272
test("ensureTelemetrySettings called twice keeps installationId and enabled flag unchanged", async () => {
281273
const dir = await mkdtemp(join(tmpdir(), "corbits-telemetry-settings-"));
282274
const path = join(dir, "settings.json");

0 commit comments

Comments
 (0)