Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: v5 #563

Merged
merged 30 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
11d6862
ci: stop testing against NodeJS v14, v16 (#561)
wolfy1339 May 21, 2023
41cbcdc
Merge branch 'main' into beta
nickfloyd May 25, 2023
0122b69
fix(deps): update dependency @octokit/request-error to v4 (#565)
renovate[bot] Jun 3, 2023
819646a
fix: bump `@octokit/request-error` to `v4.0.1`
wolfy1339 Jun 3, 2023
6fb43c4
Merge branch 'main' into beta
wolfy1339 Jun 7, 2023
5dca843
Merge branch 'main' into beta
wolfy1339 Jun 14, 2023
5a5776d
fix(deps): update dependency @octokit/types to v10 (#571)
renovate[bot] Jun 14, 2023
7627a33
Merge branch 'main' into beta
wolfy1339 Jun 16, 2023
db76bdb
fix(deps): bump Octokit deps
wolfy1339 Jun 16, 2023
d267580
fix(deps): upgrade `@octokit/graphql` to beta
wolfy1339 Jun 16, 2023
5f043aa
fix: bump `@octokit/graphql`
wolfy1339 Jun 18, 2023
6ce4d38
build(lockfile): upgrade `@octokit/endpoint` to v8.0.1
wolfy1339 Jun 18, 2023
4311080
test: replace `@octokit/auth` with proper packages
wolfy1339 Jun 18, 2023
6aeab0a
test: use graphql endpoint for previews
wolfy1339 Jun 18, 2023
4fa71b4
test: fix last commit
wolfy1339 Jun 18, 2023
8b0dc00
test: fix preview test
wolfy1339 Jun 18, 2023
1c89fd5
test: STATE is not passed to the request for oauth app auth
wolfy1339 Jun 18, 2023
d5e1222
test: don't pass `state` argument
wolfy1339 Jun 19, 2023
fcb43d3
build(deps): switch to `@gr2m/fetch-mock`
wolfy1339 Jun 19, 2023
61e4c36
test: use `JSON.stringify()` as the returned data is not strictly equ…
wolfy1339 Jun 19, 2023
d4b89b7
build(package): lock file
gr2m Jun 20, 2023
2ffb8a3
test: remove unused `STATE` variable
gr2m Jun 20, 2023
a2aa1f3
Bump to v11 of types
kfcampbell Jul 7, 2023
260568f
Merge branch 'main' into beta
kfcampbell Jul 7, 2023
57a7303
Bump octokit/request version
kfcampbell Jul 7, 2023
4e934d2
fix(deps): upgrade `@octokit` packages
wolfy1339 Jul 7, 2023
6974fd6
style: prettier
wolfy1339 Jul 7, 2023
29f04aa
test: skip agent tests
wolfy1339 Jul 7, 2023
e91e8a4
test: add fixme comments
wolfy1339 Jul 7, 2023
71ca7a8
build(deps): upgrade Octokit deps
wolfy1339 Jul 8, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ jobs:
strategy:
matrix:
node_version:
- 14
- 16
- 18
- 20
steps:
- uses: actions/checkout@v3
- name: Use Node.js ${{ matrix.node_version }}
Expand Down
26,805 changes: 8,178 additions & 18,627 deletions package-lock.json

Large diffs are not rendered by default.

18 changes: 10 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,25 @@
"author": "Gregor Martynus (https://github.com/gr2m)",
"license": "MIT",
"dependencies": {
"@octokit/auth-token": "^3.0.0",
"@octokit/graphql": "^5.0.0",
"@octokit/request": "^6.0.0",
"@octokit/request-error": "^3.0.0",
"@octokit/types": "^9.0.0",
"@octokit/auth-token": "^4.0.0",
"@octokit/graphql": "^7.0.0",
"@octokit/request": "^8.0.2",
"@octokit/request-error": "^5.0.0",
"@octokit/types": "^11.0.0",
"before-after-hook": "^2.2.0",
"universal-user-agent": "^6.0.0"
},
"devDependencies": {
"@octokit/auth": "^3.0.1",
"@octokit/auth-action": "^4.0.0",
"@octokit/auth-app": "^6.0.0",
"@octokit/auth-oauth-app": "^7.0.0",
"@octokit/tsconfig": "^2.0.0",
"@types/fetch-mock": "^7.3.1",
"@types/jest": "^29.0.0",
"@types/lolex": "^5.1.0",
"@types/node": "^18.0.0",
"esbuild": "^0.18.0",
"fetch-mock": "^9.0.0",
"fetch-mock": "npm:@gr2m/[email protected]",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to see this get merged into fetch-mock - any thoughts on what might be holding that up (since Sept 2022)?

The alias is fine, but it does seem like the project isn't even in maintenance mode - given the last commit to master was almost 2 years ago. Are there any alternatives to this mocking framework that would make for an easy migration and get us the functionality that we need here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know of any alternatives at the moment.

"glob": "^10.2.5",
"http-proxy-agent": "^7.0.0",
"jest": "^29.0.0",
Expand Down Expand Up @@ -103,6 +105,6 @@
]
},
"engines": {
"node": ">= 14"
"node": ">= 18"
}
}
2 changes: 1 addition & 1 deletion scripts/build.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ async function main() {
outdir: "pkg/dist-node",
bundle: true,
platform: "node",
target: "node14",
target: "node18",
format: "cjs",
...sharedOptions,
}),
Expand Down
3 changes: 2 additions & 1 deletion test/agent-ca/agent-ca-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import { resolve } from "path";
const { Octokit } = require("../../src");
const ca = readFileSync(resolve(__dirname, "./ca.crt"));

describe("custom client certificate", () => {
// TODO: rewrite tests to use fetch dispatchers
describe.skip("custom client certificate", () => {
let server: any;
beforeAll((done) => {
server = createServer(
Expand Down
3 changes: 2 additions & 1 deletion test/agent-proxy/agent-proxy-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ const { HttpProxyAgent } = require("http-proxy-agent");

import { Octokit } from "../../src";

describe("client proxy", () => {
// TODO: rewrite tests to use fetch dispatchers
describe.skip("client proxy", () => {
let proxy: any;
let proxyUrl: string;

Expand Down
19 changes: 5 additions & 14 deletions test/auth.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import { getUserAgent } from "universal-user-agent";
import fetchMock from "fetch-mock";
import {
createAppAuth,
createActionAuth,
createOAuthAppAuth,
} from "@octokit/auth";
import { createAppAuth } from "@octokit/auth-app";
import { createActionAuth } from "@octokit/auth-action";
import { createOAuthAppAuth } from "@octokit/auth-oauth-app";
import lolex, { type Clock } from "lolex";

import { Octokit } from "../src";
Expand Down Expand Up @@ -163,7 +161,6 @@ describe("Authentication", () => {
const CLIENT_ID = "0123";
const CLIENT_SECRET = "0123secret";
const CODE = "code123";
const STATE = "state123";

const mock = fetchMock.sandbox().postOnce(
"https://github.com/login/oauth/access_token",
Expand All @@ -177,7 +174,6 @@ describe("Authentication", () => {
client_id: CLIENT_ID,
client_secret: CLIENT_SECRET,
code: CODE,
state: STATE,
},
},
);
Expand All @@ -199,7 +195,6 @@ describe("Authentication", () => {
await octokit.auth({
type: "oauth-user",
code: CODE,
state: STATE,
});

expect(mock.done()).toBe(true);
Expand Down Expand Up @@ -231,7 +226,7 @@ describe("Authentication", () => {
{ id: 123 },
{
headers: {
accept: "application/vnd.github.machine-man-preview+json",
accept: "application/vnd.github.v3+json",
"user-agent": userAgent,
authorization: `bearer ${BEARER}`,
},
Expand All @@ -253,11 +248,7 @@ describe("Authentication", () => {
await octokit.request("GET /repos/octocat/hello-world");
await octokit.request("GET /repos/octocat/hello-world");

await octokit.request("GET /app", {
mediaType: {
previews: ["machine-man"],
},
});
await octokit.request("GET /app");

expect(mock.done()).toBe(true);
});
Expand Down
4 changes: 2 additions & 2 deletions test/constructor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import fetchMock from "fetch-mock";
describe("Smoke test", () => {
it("previews option", () => {
const mock = fetchMock.sandbox().getOnce(
"https://api.github.com/",
"https://api.github.com/graphql",
Comment on lines 6 to +7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we update the test to something real? At least we should use POST /graphql. I think the GraphQL API supports get requests technically, but we always use POST /graphql.

{ ok: true },
{
headers: {
Expand All @@ -25,7 +25,7 @@ describe("Smoke test", () => {
},
});

return octokit.request("/");
return octokit.request("/graphql");
});

it("timeZone option", () => {
Expand Down
2 changes: 1 addition & 1 deletion test/defaults.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fetchMock from "fetch-mock";
import { getUserAgent } from "universal-user-agent";
import { createActionAuth } from "@octokit/auth";
import { createActionAuth } from "@octokit/auth-action";

import { Octokit } from "../src";

Expand Down
8 changes: 4 additions & 4 deletions test/graphql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe("octokit.graphql()", () => {

const result = await octokit.graphql(query, { login: "octokit" });

expect(result).toStrictEqual(mockResult);
expect(JSON.stringify(result)).toStrictEqual(JSON.stringify(mockResult));
oscard0m marked this conversation as resolved.
Show resolved Hide resolved
});

it("GitHub Enterprise Server usage (with option.baseUrl)", async () => {
Expand Down Expand Up @@ -84,7 +84,7 @@ describe("octokit.graphql()", () => {

const result = await octokit.graphql(query, { login: "octokit" });

expect(result).toStrictEqual(mockResult);
expect(JSON.stringify(result)).toStrictEqual(JSON.stringify(mockResult));
});

it("custom headers: octokit.graphql({ query, headers })", async () => {
Expand Down Expand Up @@ -113,7 +113,7 @@ describe("octokit.graphql()", () => {
},
});

expect(result).toStrictEqual({ ok: true });
expect(JSON.stringify(result)).toStrictEqual(JSON.stringify({ ok: true }));
});

it("custom headers: octokit.graphql(query, { headers })", async () => {
Expand Down Expand Up @@ -145,6 +145,6 @@ describe("octokit.graphql()", () => {
foo: "bar",
});

expect(result).toStrictEqual({ ok: true });
expect(JSON.stringify(result)).toStrictEqual(JSON.stringify({ ok: true }));
});
});
30 changes: 18 additions & 12 deletions test/hook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe("octokit.hook", () => {
"x-foo": "bar",
},
mediaType: {
previews: ["octicon"],
previews: [],
format: "rad",
},
bar: "daz",
Expand All @@ -77,7 +77,7 @@ describe("octokit.hook", () => {
"x-foo": "bar",
},
mediaType: {
previews: ["octicon"],
previews: [],
format: "rad",
},
});
Expand Down Expand Up @@ -118,10 +118,12 @@ describe("octokit.hook", () => {

const { data } = await octokit.request("/");

expect(data).toStrictEqual({
ok: true,
afterAddition: "works",
});
expect(JSON.stringify(data)).toStrictEqual(
JSON.stringify({
ok: true,
afterAddition: "works",
}),
);
});

it("octokit.hook.error('request')", async () => {
Expand Down Expand Up @@ -159,9 +161,11 @@ describe("octokit.hook", () => {

const { data } = await octokit.request("/");

expect(data).toStrictEqual({
ok: true,
});
expect(JSON.stringify(data)).toStrictEqual(
JSON.stringify({
ok: true,
}),
);
});

it("octokit.hook.wrap('request')", async () => {
Expand Down Expand Up @@ -192,9 +196,11 @@ describe("octokit.hook", () => {

const { data } = await octokit.request("/");

expect(data).toStrictEqual({
ok: true,
});
expect(JSON.stringify(data)).toStrictEqual(
JSON.stringify({
ok: true,
}),
);
});

it("octokit.hook()", async () => {
Expand Down
4 changes: 3 additions & 1 deletion test/issues.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ describe("issues", () => {
const response = await octokit.request("/");

expect(response.status).toEqual(200);
expect(response.data).toStrictEqual({ ok: true });
expect(JSON.stringify(response.data)).toStrictEqual(
JSON.stringify({ ok: true }),
);
});
});
22 changes: 12 additions & 10 deletions test/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,40 +98,41 @@ describe("octokit.request()", () => {
const mock = fetchMock
.sandbox()
.getOnce(
"https://api.github.com/",
"https://api.github.com/graphql",
Comment on lines 100 to +101
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above, let's use POST

{},
{
headers: {
accept:
"application/vnd.github.foo-preview+json,application/vnd.github.bar-preview+json",
accept: "application/vnd.github.package-deletes-preview.json",
"user-agent": userAgent,
},
},
)
.getOnce(
"https://api.github.com/",
"https://api.github.com/graphql",
{},
{
headers: {
accept:
"application/vnd.github.foo-preview.raw,application/vnd.github.bar-preview.raw,application/vnd.github.baz-preview.raw",
accept: "application/vnd.github.v3.raw",
"user-agent": userAgent,
},
overwriteRoutes: false,
},
);

const octokit = new Octokit({
previews: ["foo", "bar-preview"],
request: {
fetch: mock,
},
});

await octokit.request("/");
await octokit.request("/", {
await octokit.request("/graphql", {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking more about it, maybe we should move these tests to test/graphql.test.ts now?

Copy link
Contributor

@nickfloyd nickfloyd Jul 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into hoisting them out once I get past the skips. We could also differ it to another change set so we can move forward here. Not like the change would be hard, it just introduces more into something we've been trying to get out the door.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, this can totally be a follow up, like all of my comments

mediaType: {
previews: ["package-deletes"],
format: "json",
},
});
await octokit.request("/graphql", {
mediaType: {
previews: ["bar", "baz-preview"],
format: "raw",
},
});
Expand Down Expand Up @@ -178,6 +179,7 @@ describe("octokit.request()", () => {
{
owner: "epmatsw",
repo: "example-repo",
// @ts-expect-error There is currently an issue with the types, null is an allowed value
milestone: null,
issue_number: 1,
},
Expand Down