Skip to content

Commit 0b4e1a4

Browse files
committed
Review comments
1 parent 07adc3e commit 0b4e1a4

File tree

5 files changed

+119
-73
lines changed

5 files changed

+119
-73
lines changed

test/unit/api/coderApi.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ describe("CoderApi", () => {
125125
expect(thrownError.x509Err).toBeDefined();
126126
});
127127

128-
it("applies headers in correct precedence order (command > config > axios default)", async () => {
128+
it("applies headers in correct precedence order (command overrides config overrides axios default)", async () => {
129129
const api = createApi(CODER_URL, AXIOS_TOKEN);
130130

131131
// Test 1: Headers from config, default token from API creation
@@ -225,7 +225,7 @@ describe("CoderApi", () => {
225225
});
226226
});
227227

228-
it("applies headers in correct precedence order (command > config > axios default)", async () => {
228+
it("applies headers in correct precedence order (command overrides config overrides axios default)", async () => {
229229
// Test 1: Default token from API creation
230230
await api.watchBuildLogsByBuildId(BUILD_ID, []);
231231

test/unit/api/streamingFetchAdapter.test.ts

Lines changed: 35 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,17 @@
1-
import { type AxiosInstance } from "axios";
2-
import { EventEmitter } from "events";
1+
import { type AxiosInstance, type AxiosResponse } from "axios";
32
import { type ReaderLike } from "eventsource";
4-
import { type IncomingMessage } from "http";
5-
import { describe, it, expect, vi, beforeEach } from "vitest";
3+
import { EventEmitter } from "node:events";
4+
import { type IncomingMessage } from "node:http";
5+
import { describe, it, expect, vi } from "vitest";
66

77
import { createStreamingFetchAdapter } from "@/api/streamingFetchAdapter";
88

99
const TEST_URL = "https://example.com/api";
1010

1111
describe("createStreamingFetchAdapter", () => {
12-
let mockAxios: AxiosInstance;
13-
14-
beforeEach(() => {
15-
vi.resetAllMocks();
16-
mockAxios = {
17-
request: vi.fn(),
18-
} as unknown as AxiosInstance;
19-
});
20-
2112
describe("Request Handling", () => {
2213
it("passes URL, signal, and responseType to axios", async () => {
14+
const mockAxios = createAxiosMock();
2315
const mockStream = createMockStream();
2416
setupAxiosResponse(mockAxios, 200, {}, mockStream);
2517

@@ -37,7 +29,8 @@ describe("createStreamingFetchAdapter", () => {
3729
});
3830
});
3931

40-
it("applies headers in correct precedence order (config > init)", async () => {
32+
it("applies headers in correct precedence order (config overrides init)", async () => {
33+
const mockAxios = createAxiosMock();
4134
const mockStream = createMockStream();
4235
setupAxiosResponse(mockAxios, 200, {}, mockStream);
4336

@@ -88,6 +81,7 @@ describe("createStreamingFetchAdapter", () => {
8881

8982
describe("Response Properties", () => {
9083
it("returns response with correct properties", async () => {
84+
const mockAxios = createAxiosMock();
9185
const mockStream = createMockStream();
9286
setupAxiosResponse(
9387
mockAxios,
@@ -102,11 +96,13 @@ describe("createStreamingFetchAdapter", () => {
10296
expect(response.url).toBe(TEST_URL);
10397
expect(response.status).toBe(200);
10498
expect(response.headers.get("content-type")).toBe("text/event-stream");
99+
// Headers are lowercased when we retrieve them
105100
expect(response.headers.get("CoNtEnT-TyPe")).toBe("text/event-stream");
106101
expect(response.body?.getReader).toBeDefined();
107102
});
108103

109104
it("detects redirected requests", async () => {
105+
const mockAxios = createAxiosMock();
110106
const mockStream = createMockStream();
111107
const mockResponse = {
112108
status: 200,
@@ -117,7 +113,7 @@ describe("createStreamingFetchAdapter", () => {
117113
responseUrl: "https://redirect.com/api",
118114
},
119115
},
120-
};
116+
} as AxiosResponse<IncomingMessage>;
121117
vi.mocked(mockAxios.request).mockResolvedValue(mockResponse);
122118

123119
const adapter = createStreamingFetchAdapter(mockAxios);
@@ -178,22 +174,14 @@ describe("createStreamingFetchAdapter", () => {
178174
expect(mockStream.destroy).toHaveBeenCalled();
179175
});
180176
});
181-
182-
async function setupReaderTest(): Promise<{
183-
mockStream: IncomingMessage;
184-
reader: ReaderLike | ReadableStreamDefaultReader<Uint8Array<ArrayBuffer>>;
185-
}> {
186-
const mockStream = createMockStream();
187-
setupAxiosResponse(mockAxios, 200, {}, mockStream);
188-
189-
const adapter = createStreamingFetchAdapter(mockAxios);
190-
const response = await adapter(TEST_URL);
191-
const reader = response.body!.getReader();
192-
193-
return { mockStream, reader };
194-
}
195177
});
196178

179+
function createAxiosMock(): AxiosInstance {
180+
return {
181+
request: vi.fn(),
182+
} as unknown as AxiosInstance;
183+
}
184+
197185
function createMockStream(): IncomingMessage {
198186
const stream = new EventEmitter() as IncomingMessage;
199187
stream.destroy = vi.fn();
@@ -212,3 +200,21 @@ function setupAxiosResponse(
212200
data: stream,
213201
});
214202
}
203+
204+
async function setupReaderTest(): Promise<{
205+
mockStream: IncomingMessage;
206+
reader: ReaderLike | ReadableStreamDefaultReader<Uint8Array<ArrayBuffer>>;
207+
}> {
208+
const mockAxios = createAxiosMock();
209+
const mockStream = createMockStream();
210+
setupAxiosResponse(mockAxios, 200, {}, mockStream);
211+
212+
const adapter = createStreamingFetchAdapter(mockAxios);
213+
const response = await adapter(TEST_URL);
214+
const reader = response.body?.getReader();
215+
if (reader === undefined) {
216+
throw new Error("Reader is undefined");
217+
}
218+
219+
return { mockStream, reader };
220+
}

test/unit/core/cliManager.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,8 @@ describe("CliManager", () => {
546546
expect(files.find((file) => file.includes(".asc"))).toBeUndefined();
547547
});
548548

549-
it.each([
549+
type SignatureErrorTestCase = [status: number, message: string];
550+
it.each<SignatureErrorTestCase>([
550551
[404, "Signature not found"],
551552
[500, "Failed to download signature"],
552553
])("allows skipping verification on %i", async (status, message) => {
@@ -558,7 +559,7 @@ describe("CliManager", () => {
558559
expect(pgp.verifySignature).not.toHaveBeenCalled();
559560
});
560561

561-
it.each([
562+
it.each<SignatureErrorTestCase>([
562563
[404, "Signature not found"],
563564
[500, "Failed to download signature"],
564565
])(

test/unit/logging/utils.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ describe("Logging utils", () => {
2323
});
2424

2525
describe("sizeOf", () => {
26-
it.each([
26+
type SizeOfTestCase = [data: unknown, bytes: number | undefined];
27+
it.each<SizeOfTestCase>([
2728
// Primitives return a fixed value
2829
[null, 0],
2930
[undefined, 0],

0 commit comments

Comments
 (0)