Skip to content

Commit 3fd4ef6

Browse files
renovate[bot]nickfloydwolfy1339kfcampbell
authoredJul 18, 2023
fix(deps): update octokit monorepo (major) (#513)
* fix(deps): update octokit monorepo * todo fix proxy test by replacing http agent/fetch mock with custom fetch * build: switch to `@gr2m/fetch-mock` * updates the plugin to use a custom fetch as suggested * moves undici to deps and removes https-proxy-agent * Serialization issue as seen in core * adds local server for the tests to call * Undo skip for array of smoke tests * Undo skip on another test * WIP checkpoint with all lines covered but one test broken * WIP checkpoint with tests passing * Use localhost for consistency * Remove extraneous console.log statements * Strongly type Server * Some test cleanup * WIP comment out spy * Refactor customFetch tests to own describe block * Comment fix * Remove redundant tests * Remove unnecessary tests * Revert "Remove unnecessary tests" This reverts commit 6458785 in order to increase code coverage. --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Nick Floyd <139819+nickfloyd@users.noreply.github.com> Co-authored-by: wolfy1339 <webmaster@wolfy1339.com> Co-authored-by: Keegan Campbell <me@kfcampbell.com>
·
v7.0.1v6.0.5
1 parent 8e12440 commit 3fd4ef6

File tree

4 files changed

+223
-201
lines changed

4 files changed

+223
-201
lines changed
 

‎package-lock.json

+110-145
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+7-7
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,19 @@
2424
"author": "Gregor Martynus (https://twitter.com/gr2m)",
2525
"license": "MIT",
2626
"dependencies": {
27-
"@octokit/auth-action": "^3.0.0",
28-
"@octokit/core": "^4.0.0",
29-
"@octokit/plugin-paginate-rest": "^7.0.0",
30-
"@octokit/plugin-rest-endpoint-methods": "^8.0.0",
31-
"@octokit/types": "^10.0.0",
32-
"https-proxy-agent": "^7.0.0"
27+
"@octokit/auth-action": "^4.0.0",
28+
"@octokit/core": "^5.0.0",
29+
"@octokit/plugin-paginate-rest": "^8.0.0",
30+
"@octokit/plugin-rest-endpoint-methods": "^9.0.0",
31+
"@octokit/types": "^11.0.0",
32+
"undici": "^5.0.0"
3333
},
3434
"devDependencies": {
3535
"@octokit/tsconfig": "^2.0.0",
3636
"@types/jest": "^29.0.0",
3737
"@types/node": "^18.0.0",
3838
"esbuild": "^0.18.0",
39-
"fetch-mock": "^9.0.0",
39+
"fetch-mock": "npm:@gr2m/fetch-mock@9.11.0-pull-request-644.1",
4040
"glob": "^10.2.6",
4141
"jest": "^29.0.0",
4242
"prettier": "3.0.0",

‎src/index.ts

+12-5
Original file line numberDiff line numberDiff line change
@@ -6,28 +6,35 @@ export type { RestEndpointMethodTypes } from "@octokit/plugin-rest-endpoint-meth
66

77
import { VERSION } from "./version";
88
import type { OctokitOptions } from "@octokit/core/dist-types/types";
9-
import { HttpsProxyAgent } from "https-proxy-agent";
9+
import { fetch as undiciFetch, ProxyAgent } from "undici";
1010

1111
const DEFAULTS = {
1212
authStrategy: createActionAuth,
1313
baseUrl: getApiBaseUrl(),
1414
userAgent: `octokit-action.js/${VERSION}`,
1515
};
1616

17-
function getProxyAgent() {
17+
export function getProxyAgent() {
1818
const httpProxy = process.env["HTTP_PROXY"] || process.env["http_proxy"];
1919
if (httpProxy) {
20-
return new HttpsProxyAgent(httpProxy);
20+
return new ProxyAgent(httpProxy);
2121
}
2222

2323
const httpsProxy = process.env["HTTPS_PROXY"] || process.env["https_proxy"];
2424
if (httpsProxy) {
25-
return new HttpsProxyAgent(httpsProxy);
25+
return new ProxyAgent(httpsProxy);
2626
}
2727

2828
return undefined;
2929
}
3030

31+
export const customFetch = async function (url: string, opts: any) {
32+
return await undiciFetch(url, {
33+
dispatcher: getProxyAgent(),
34+
...opts,
35+
});
36+
};
37+
3138
export const Octokit = Core.plugin(
3239
paginateRest,
3340
legacyRestEndpointMethods,
@@ -36,7 +43,7 @@ export const Octokit = Core.plugin(
3643
...DEFAULTS,
3744
...options,
3845
request: {
39-
agent: getProxyAgent(),
46+
fetch: customFetch,
4047
...options.request,
4148
},
4249
};

‎test/smoke.test.ts

+94-44
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,74 @@
11
import fetchMock from "fetch-mock";
2-
import { RequestOptions } from "https";
3-
import { HttpsProxyAgent } from "https-proxy-agent";
2+
import { createServer, type Server } from "https";
3+
import { Octokit, getProxyAgent, customFetch } from "../src";
4+
import { ProxyAgent } from "undici";
5+
6+
// mock undici such that we can substitute our own fetch implementation
7+
// but use the actual ProxyAgent implementation for most tests. the
8+
// exception is "should call undiciFetch with the correct dispatcher"
9+
// where we want to validate that a mocked ProxyAgent is passed through
10+
// to undici.fetch.
11+
jest.mock("undici", () => {
12+
return {
13+
fetch: jest.fn(),
14+
ProxyAgent: jest.requireActual("undici").ProxyAgent,
15+
};
16+
});
17+
const undici = jest.requireMock("undici");
418

5-
import { Octokit } from "../src";
19+
describe("Smoke test", () => {
20+
let server: Server;
621

7-
jest.mock("https-proxy-agent");
22+
beforeAll((done) => {
23+
server = createServer(
24+
{
25+
requestCert: false,
26+
rejectUnauthorized: false,
27+
},
28+
(request: any, response: any) => {
29+
expect(request.method).toEqual("GET");
30+
expect(request.url).toEqual("/");
31+
32+
response.writeHead(200);
33+
response.write("ok");
34+
response.end();
35+
},
36+
);
37+
38+
server.listen(0, done);
39+
});
840

9-
describe("Smoke test", () => {
1041
beforeEach(() => {
1142
delete process.env.GITHUB_TOKEN;
1243
delete process.env.INPUT_GITHUB_TOKEN;
1344
delete process.env.GITHUB_ACTION;
1445
delete process.env.GITHUB_API_URL;
1546
delete process.env.HTTPS_PROXY;
1647
delete process.env.https_proxy;
48+
delete process.env.HTTP_PROXY;
49+
delete process.env.http_proxy;
50+
});
51+
52+
afterAll((done) => {
53+
server.close(done);
54+
jest.unmock("undici");
55+
});
56+
57+
it("should return a ProxyAgent for the httpProxy environment variable", () => {
58+
process.env.HTTP_PROXY = "https://127.0.0.1";
59+
const agent = getProxyAgent();
60+
expect(agent).toBeInstanceOf(ProxyAgent);
61+
});
62+
63+
it("should return a ProxyAgent for the httpsProxy environment variable", () => {
64+
process.env.HTTPS_PROXY = "https://127.0.0.1";
65+
const agent = getProxyAgent();
66+
expect(agent).toBeInstanceOf(ProxyAgent);
67+
});
68+
69+
it("should return undefined if no proxy environment variables are set", () => {
70+
const agent = getProxyAgent();
71+
expect(agent).toBeUndefined();
1772
});
1873

1974
it("happy path with GITHUB_TOKEN", () => {
@@ -175,11 +230,12 @@ describe("Smoke test", () => {
175230
title: "My test issue",
176231
});
177232

178-
expect(data).toStrictEqual({ ok: true });
233+
// TODO: need a follow up issue to clean this up
234+
expect(JSON.stringify(data)).toStrictEqual(JSON.stringify({ ok: true }));
179235
});
180236

181237
it.each(["HTTPS_PROXY", "https_proxy", "HTTP_PROXY", "http_proxy"])(
182-
"Uses https-proxy-agent with %s env var",
238+
"Uses ProxyAgent with %s env var",
183239
async (https_proxy_env) => {
184240
process.env.GITHUB_TOKEN = "secret123";
185241
process.env.GITHUB_ACTION = "test";
@@ -209,52 +265,46 @@ describe("Smoke test", () => {
209265
title: "My test issue",
210266
});
211267

212-
expect(HttpsProxyAgent).toHaveBeenCalled();
213-
214268
const [call] = fetchSandbox.calls();
215269
expect(call[0]).toEqual(
216270
"https://api.github.com/repos/octocat/hello-world/issues",
217271
);
218-
expect((call[1] as RequestOptions).agent).toBeInstanceOf(HttpsProxyAgent);
219272
},
220273
);
274+
describe("customFetch", () => {
275+
afterAll(() => {
276+
delete process.env.HTTPS_PROXY;
277+
jest.clearAllMocks();
278+
});
221279

222-
it("Uses the explicitly provided request.agent value if it's provided", async () => {
223-
process.env.GITHUB_TOKEN = "secret123";
224-
process.env.GITHUB_ACTION = "test";
225-
process.env.HTTPS_PROXY = "https://127.0.0.1";
226-
227-
const fetchSandbox = fetchMock.sandbox();
228-
const mock = fetchSandbox.post(
229-
"path:/repos/octocat/hello-world/issues",
230-
{ id: 1 },
231-
{
232-
body: {
233-
title: "My test issue",
234-
},
235-
},
236-
);
280+
it("should call undiciFetch with the correct dispatcher", async () => {
281+
process.env.HTTPS_PROXY = "https://127.0.0.1";
282+
const expectedAgent = new ProxyAgent("https://127.0.0.1");
237283

238-
expect(Octokit).toBeInstanceOf(Function);
239-
const octokit = new Octokit({
240-
auth: "secret123",
241-
request: {
242-
fetch: mock,
243-
agent: null,
244-
},
245-
});
246-
await octokit.request("POST /repos/{owner}/{repo}/issues", {
247-
owner: "octocat",
248-
repo: "hello-world",
249-
title: "My test issue",
250-
});
284+
jest.mock("../src", () => {
285+
const actualModule = jest.requireActual("../src");
286+
return {
287+
...actualModule,
288+
getProxyAgent: jest.fn(() => expectedAgent),
289+
};
290+
});
291+
expect(JSON.stringify(getProxyAgent())).toBe(
292+
JSON.stringify(expectedAgent),
293+
);
251294

252-
expect(HttpsProxyAgent).toHaveBeenCalled();
295+
// mock undici.fetch to extract the `dispatcher` option passed in.
296+
// this allows us to verify that `customFetch` correctly sets
297+
// the dispatcher to `expectedAgent` when HTTPS_PROXY is set.
298+
let dispatcher: any;
299+
(undici.fetch as jest.Mock).mockImplementation(
300+
(_url: string, options: any) => {
301+
dispatcher = options.dispatcher;
253302

254-
const [call] = fetchSandbox.calls();
255-
expect(call[0]).toEqual(
256-
"https://api.github.com/repos/octocat/hello-world/issues",
257-
);
258-
expect((call[1] as RequestOptions).agent).toBeNull();
303+
return Promise.resolve(new Response());
304+
},
305+
);
306+
await customFetch("http://api.github.com", {});
307+
expect(JSON.stringify(dispatcher)).toEqual(JSON.stringify(expectedAgent));
308+
});
259309
});
260310
});

0 commit comments

Comments
 (0)
Please sign in to comment.