Skip to content

Commit 911fdcb

Browse files
fmenezesfmenezes
committed
fix: tests
1 parent 95b0b06 commit 911fdcb

File tree

3 files changed

+87
-103
lines changed

3 files changed

+87
-103
lines changed

‎src/telemetry/telemetry.ts

Lines changed: 21 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,6 @@ import nodeMachineId from "node-machine-id";
99
import { getDeviceId } from "@mongodb-js/device-id";
1010
import fs from "fs/promises";
1111

12-
type EventResult = {
13-
success: boolean;
14-
error?: Error;
15-
};
16-
1712
async function fileExists(filePath: string): Promise<boolean> {
1813
try {
1914
await fs.stat(filePath);
@@ -60,7 +55,6 @@ export class Telemetry {
6055
private constructor(
6156
private readonly session: Session,
6257
private readonly userConfig: UserConfig,
63-
private readonly commonProperties: CommonProperties,
6458
{
6559
eventCache,
6660
getRawMachineId,
@@ -80,18 +74,16 @@ export class Telemetry {
8074
session: Session,
8175
userConfig: UserConfig,
8276
{
83-
commonProperties = { ...MACHINE_METADATA },
8477
eventCache = EventCache.getInstance(),
8578
getRawMachineId = () => nodeMachineId.machineId(true),
8679
getContainerEnv = isContainerized,
8780
}: {
88-
commonProperties?: CommonProperties;
8981
eventCache?: EventCache;
9082
getRawMachineId?: () => Promise<string>;
9183
getContainerEnv?: () => Promise<boolean>;
9284
} = {}
9385
): Telemetry {
94-
const instance = new Telemetry(session, userConfig, commonProperties, {
86+
const instance = new Telemetry(session, userConfig, {
9587
eventCache,
9688
getRawMachineId,
9789
getContainerEnv,
@@ -156,9 +148,9 @@ export class Telemetry {
156148
* Gets the common properties for events
157149
* @returns Object containing common properties for all events
158150
*/
159-
public async getCommonProperties(): Promise<CommonProperties> {
151+
private async getCommonProperties(): Promise<CommonProperties> {
160152
return {
161-
...this.commonProperties,
153+
...MACHINE_METADATA,
162154
mcp_client_version: this.session.agentRunner?.version,
163155
mcp_client_name: this.session.agentRunner?.name,
164156
session_id: this.session.sessionId,
@@ -186,7 +178,7 @@ export class Telemetry {
186178
return !doNotTrack;
187179
}
188180

189-
public hasPendingPromises(): boolean {
181+
private hasPendingPromises(): boolean {
190182
return this.pendingPromises > 0;
191183
}
192184

@@ -209,43 +201,34 @@ export class Telemetry {
209201
`Attempting to send ${allEvents.length} events (${cachedEvents.length} cached)`
210202
);
211203

212-
const result = await this.sendEvents(this.session.apiClient, allEvents);
213-
if (result.success) {
204+
try {
205+
await this.sendEvents(this.session.apiClient, allEvents);
214206
this.eventCache.clearEvents();
215207
logger.debug(
216208
LogId.telemetryEmitSuccess,
217209
"telemetry",
218210
`Sent ${allEvents.length} events successfully: ${JSON.stringify(allEvents, null, 2)}`
219211
);
220-
return;
212+
} catch (error: unknown) {
213+
logger.debug(
214+
LogId.telemetryEmitFailure,
215+
"telemetry",
216+
`Error sending event to client: ${error instanceof Error ? error.message : String(error)}`
217+
);
218+
this.eventCache.appendEvents(events);
221219
}
222-
223-
logger.debug(
224-
LogId.telemetryEmitFailure,
225-
"telemetry",
226-
`Error sending event to client: ${result.error instanceof Error ? result.error.message : String(result.error)}`
227-
);
228-
this.eventCache.appendEvents(events);
229220
}
230221

231222
/**
232223
* Attempts to send events through the provided API client
233224
*/
234-
private async sendEvents(client: ApiClient, events: BaseEvent[]): Promise<EventResult> {
235-
try {
236-
const commonProperties = await this.getCommonProperties();
237-
await client.sendEvents(
238-
events.map((event) => ({
239-
...event,
240-
properties: { ...commonProperties, ...event.properties },
241-
}))
242-
);
243-
return { success: true };
244-
} catch (error) {
245-
return {
246-
success: false,
247-
error: error instanceof Error ? error : new Error(String(error)),
248-
};
249-
}
225+
private async sendEvents(client: ApiClient, events: BaseEvent[]): Promise<void> {
226+
const commonProperties = await this.getCommonProperties();
227+
await client.sendEvents(
228+
events.map((event) => ({
229+
...event,
230+
properties: { ...commonProperties, ...event.properties },
231+
}))
232+
);
250233
}
251234
}

‎tests/integration/telemetry.test.ts

Lines changed: 0 additions & 22 deletions
This file was deleted.

‎tests/unit/telemetry.test.ts

Lines changed: 66 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ const MockApiClient = ApiClient as jest.MockedClass<typeof ApiClient>;
1616
jest.mock("../../src/telemetry/eventCache.js");
1717
const MockEventCache = EventCache as jest.MockedClass<typeof EventCache>;
1818

19+
const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
20+
1921
describe("Telemetry", () => {
2022
const machineId = "test-machine-id";
2123
const hashedMachineId = createHmac("sha256", machineId.toUpperCase()).update("atlascli").digest("hex");
@@ -24,6 +26,11 @@ describe("Telemetry", () => {
2426
let mockEventCache: jest.Mocked<EventCache>;
2527
let session: Session;
2628
let telemetry: Telemetry;
29+
let telemetryConfig: {
30+
eventCache: EventCache;
31+
getRawMachineId: () => Promise<string>;
32+
getContainerEnv: () => Promise<boolean>;
33+
};
2734

2835
// Helper function to create properly typed test events
2936
function createTestEvent(options?: {
@@ -55,7 +62,7 @@ describe("Telemetry", () => {
5562
}
5663

5764
// Helper function to verify mock calls to reduce duplication
58-
async function verifyMockCalls({
65+
function verifyMockCalls({
5966
sendEventsCalls = 0,
6067
clearEventsCalls = 0,
6168
appendEventsCalls = 0,
@@ -77,13 +84,10 @@ describe("Telemetry", () => {
7784
expect(appendEvents.length).toBe(appendEventsCalls);
7885

7986
if (sendEventsCalledWith) {
80-
const commonProps = await telemetry.getCommonProperties();
81-
82-
expect(sendEvents[0]?.[0]).toEqual(
87+
expect(sendEvents[0]?.[0]).toMatchObject(
8388
sendEventsCalledWith.map((event) => ({
8489
...event,
8590
properties: {
86-
...commonProps,
8791
...event.properties,
8892
},
8993
}))
@@ -127,11 +131,13 @@ describe("Telemetry", () => {
127131
setAgentRunner: jest.fn().mockResolvedValue(undefined),
128132
} as unknown as Session;
129133

130-
telemetry = Telemetry.create(session, config, {
134+
telemetryConfig = {
131135
eventCache: mockEventCache,
132136
getRawMachineId: () => Promise.resolve(machineId),
133137
getContainerEnv: () => Promise.resolve(false),
134-
});
138+
};
139+
140+
telemetry = Telemetry.create(session, config, telemetryConfig);
135141

136142
config.telemetry = "enabled";
137143
});
@@ -143,7 +149,7 @@ describe("Telemetry", () => {
143149

144150
await telemetry.emitEvents([testEvent]);
145151

146-
await verifyMockCalls({
152+
verifyMockCalls({
147153
sendEventsCalls: 1,
148154
clearEventsCalls: 1,
149155
sendEventsCalledWith: [testEvent],
@@ -157,7 +163,7 @@ describe("Telemetry", () => {
157163

158164
await telemetry.emitEvents([testEvent]);
159165

160-
await verifyMockCalls({
166+
verifyMockCalls({
161167
sendEventsCalls: 1,
162168
appendEventsCalls: 1,
163169
appendEventsCalledWith: [testEvent],
@@ -180,7 +186,7 @@ describe("Telemetry", () => {
180186

181187
await telemetry.emitEvents([newEvent]);
182188

183-
await verifyMockCalls({
189+
verifyMockCalls({
184190
sendEventsCalls: 1,
185191
clearEventsCalls: 1,
186192
sendEventsCalledWith: [cachedEvent, newEvent],
@@ -198,46 +204,57 @@ describe("Telemetry", () => {
198204
device_id: hashedMachineId,
199205
};
200206

201-
const commonProps = await telemetry.getCommonProperties();
207+
const testEvent = createTestEvent();
202208

203-
expect(commonProps).toMatchObject(expectedProps);
204-
});
209+
await telemetry.emitEvents([testEvent]);
205210

206-
describe("machine ID resolution", () => {
207-
beforeEach(() => {
208-
jest.clearAllMocks();
209-
jest.useFakeTimers();
210-
});
211+
const checkEvent = {
212+
...testEvent,
213+
properties: {
214+
...testEvent.properties,
215+
...expectedProps,
216+
},
217+
};
211218

212-
afterEach(() => {
213-
jest.clearAllMocks();
214-
jest.useRealTimers();
219+
verifyMockCalls({
220+
sendEventsCalls: 1,
221+
clearEventsCalls: 1,
222+
sendEventsCalledWith: [checkEvent],
215223
});
224+
});
216225

226+
describe("machine ID resolution", () => {
217227
it("should successfully resolve the machine ID", async () => {
218-
telemetry = Telemetry.create(session, config, {
219-
getRawMachineId: () => Promise.resolve(machineId),
220-
getContainerEnv: () => Promise.resolve(false),
228+
const testEvent = createTestEvent();
229+
230+
await telemetry.emitEvents([testEvent]);
231+
232+
const checkEvent = {
233+
...testEvent,
234+
properties: {
235+
...testEvent.properties,
236+
device_id: hashedMachineId,
237+
},
238+
};
239+
240+
verifyMockCalls({
241+
sendEventsCalls: 1,
242+
clearEventsCalls: 1,
243+
sendEventsCalledWith: [checkEvent],
221244
});
222-
223-
expect(telemetry.hasPendingPromises()).toBe(true);
224-
const commonProps = await telemetry.getCommonProperties();
225-
expect(telemetry.hasPendingPromises()).toBe(false);
226-
expect(commonProps.device_id).toBe(hashedMachineId);
227245
});
228246

229247
it("should handle machine ID resolution failure", async () => {
230248
const loggerSpy = jest.spyOn(logger, "debug");
231249

232250
telemetry = Telemetry.create(session, config, {
251+
...telemetryConfig,
233252
getRawMachineId: () => Promise.reject(new Error("Failed to get device ID")),
234-
getContainerEnv: () => Promise.resolve(false),
235253
});
236254

237-
expect(telemetry.hasPendingPromises()).toBe(true);
238-
const commonProps = await telemetry.getCommonProperties();
239-
expect(telemetry.hasPendingPromises()).toBe(false);
240-
expect(commonProps.device_id).toBe("unknown");
255+
const testEvent = createTestEvent();
256+
257+
await telemetry.emitEvents([testEvent]);
241258

242259
expect(loggerSpy).toHaveBeenCalledWith(
243260
LogId.telemetryDeviceIdFailure,
@@ -247,17 +264,19 @@ describe("Telemetry", () => {
247264
});
248265

249266
it("should timeout if machine ID resolution takes too long", async () => {
250-
const DEVICE_ID_TIMEOUT = 3000;
251267
const loggerSpy = jest.spyOn(logger, "debug");
252268

253269
telemetry = Telemetry.create(session, config, {
254-
getRawMachineId: () => new Promise(() => {}),
255-
getContainerEnv: () => Promise.resolve(false),
270+
...telemetryConfig,
271+
getRawMachineId: () => new Promise(() => {}), // Never resolves
256272
});
257-
expect(telemetry.hasPendingPromises()).toBe(true);
258-
jest.advanceTimersByTime(DEVICE_ID_TIMEOUT);
259-
const commonProps = await telemetry.getCommonProperties();
260-
expect(commonProps.device_id).toBe("unknown");
273+
274+
const testEvent = createTestEvent();
275+
276+
await telemetry.emitEvents([testEvent]);
277+
278+
await delay(5000); // Wait for timeout
279+
261280
expect(loggerSpy).toHaveBeenCalledWith(
262281
LogId.telemetryDeviceIdTimeout,
263282
"telemetry",
@@ -281,7 +300,9 @@ describe("Telemetry", () => {
281300

282301
await telemetry.emitEvents([testEvent]);
283302

284-
await verifyMockCalls();
303+
verifyMockCalls({
304+
sendEventsCalls: 0,
305+
});
285306
});
286307
});
287308

@@ -306,7 +327,9 @@ describe("Telemetry", () => {
306327

307328
await telemetry.emitEvents([testEvent]);
308329

309-
await verifyMockCalls();
330+
verifyMockCalls({
331+
sendEventsCalls: 0,
332+
});
310333
});
311334
});
312335
});

0 commit comments

Comments
 (0)