Skip to content

Commit

Permalink
test(secondary-limit): replace 'abuse-limit' with 'secondary-limit'
Browse files Browse the repository at this point in the history
  • Loading branch information
@oscard0m
oscard0m committed Feb 13, 2022
1 parent 7b7a534 commit ee84d9e
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 148 deletions.
257 changes: 134 additions & 123 deletions test/events.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { TestOctokit } from "./octokit";
import util from "util";

describe("Events", function () {
it("Should support non-limit 403s", async function () {
const octokit = new TestOctokit({
throttle: { onAbuseLimit: () => 1, onRateLimit: () => 1 },
throttle: { onSecondaryRateLimit: () => 1, onRateLimit: () => 1 },
});
let caught = false;

Expand Down Expand Up @@ -32,144 +33,154 @@ describe("Events", function () {
]);
});

describe("'abuse-limit'", function () {
it("Should detect abuse limit and broadcast event", async function () {
let eventCount = 0;
const octokit = new TestOctokit({
throttle: {
onAbuseLimit: (
retryAfter: number,
options: object,
octokitFromOptions: InstanceType<typeof TestOctokit>
) => {
expect(octokit).toBe(octokitFromOptions);
expect(retryAfter).toEqual(60);
expect(options).toMatchObject({
method: "GET",
url: "/route2",
request: { retryCount: 0 },
});
eventCount++;
describe.each(["onSecondaryRateLimit", "onAbuseLimit"])(
"secondary-limit",
function (eventHandlerName) {
it("Should detect SecondaryRate limit and broadcast event", async function () {
let eventCount = 0;
jest.spyOn(util, "deprecate");
const octokit = new TestOctokit({
throttle: {
[eventHandlerName]: (
retryAfter: number,
options: object,
octokitFromOptions: InstanceType<typeof TestOctokit>
) => {
expect(octokit).toBe(octokitFromOptions);
expect(retryAfter).toEqual(60);
expect(options).toMatchObject({
method: "GET",
url: "/route2",
request: { retryCount: 0 },
});
if (eventHandlerName === "onAbuseLimit") {
expect(util.deprecate).toHaveBeenCalledWith(
expect.any(Function),
"'onAbuseLimit' is deprecated, use 'onSecondaryRateLimit' instead"
);
}
eventCount++;
},
onRateLimit: () => 1,
},
onRateLimit: () => 1,
},
});
});

await octokit.request("GET /route1", {
request: {
responses: [{ status: 201, headers: {}, data: {} }],
},
});
try {
await octokit.request("GET /route2", {
await octokit.request("GET /route1", {
request: {
responses: [
{
status: 403,
headers: { "retry-after": "60" },
data: {
message: "You have exceeded a secondary rate limit",
},
},
],
responses: [{ status: 201, headers: {}, data: {} }],
},
});
throw new Error("Should not reach this point");
} catch (error: any) {
expect(error.status).toEqual(403);
}

expect(eventCount).toEqual(1);
});
try {
await octokit.request("GET /route2", {
request: {
responses: [
{
status: 403,
headers: { "retry-after": "60" },
data: {
message: "You have exceeded a secondary rate limit",
},
},
],
},
});
throw new Error("Should not reach this point");
} catch (error: any) {
expect(error.status).toEqual(403);
}

expect(eventCount).toEqual(1);
});

it("Should ensure retryAfter is a minimum of 5s", async function () {
let eventCount = 0;
const octokit = new TestOctokit({
throttle: {
onAbuseLimit: (retryAfter: number, options: object) => {
expect(retryAfter).toEqual(5);
expect(options).toMatchObject({
method: "GET",
url: "/route2",
request: { retryCount: 0 },
});
eventCount++;
it("Should ensure retryAfter is a minimum of 5s", async function () {
let eventCount = 0;
const octokit = new TestOctokit({
throttle: {
[eventHandlerName]: (retryAfter: number, options: object) => {
expect(retryAfter).toEqual(5);
expect(options).toMatchObject({
method: "GET",
url: "/route2",
request: { retryCount: 0 },
});
eventCount++;
},
onRateLimit: () => 1,
},
onRateLimit: () => 1,
},
});
});

await octokit.request("GET /route1", {
request: {
responses: [{ status: 201, headers: {}, data: {} }],
},
});
try {
await octokit.request("GET /route2", {
await octokit.request("GET /route1", {
request: {
responses: [
{
status: 403,
headers: { "retry-after": "2" },
data: {
message: "You have exceeded a secondary rate limit",
},
},
],
responses: [{ status: 201, headers: {}, data: {} }],
},
});
throw new Error("Should not reach this point");
} catch (error: any) {
expect(error.status).toEqual(403);
}

expect(eventCount).toEqual(1);
});
try {
await octokit.request("GET /route2", {
request: {
responses: [
{
status: 403,
headers: { "retry-after": "2" },
data: {
message: "You have exceeded a secondary rate limit",
},
},
],
},
});
throw new Error("Should not reach this point");
} catch (error: any) {
expect(error.status).toEqual(403);
}

expect(eventCount).toEqual(1);
});

it("Should broadcast retryAfter of 5s even when the header is missing", async function () {
let eventCount = 0;
const octokit = new TestOctokit({
throttle: {
onAbuseLimit: (retryAfter: number, options: object) => {
expect(retryAfter).toEqual(5);
expect(options).toMatchObject({
method: "GET",
url: "/route2",
request: { retryCount: 0 },
});
eventCount++;
it("Should broadcast retryAfter of 5s even when the header is missing", async function () {
let eventCount = 0;
const octokit = new TestOctokit({
throttle: {
[eventHandlerName]: (retryAfter: number, options: object) => {
expect(retryAfter).toEqual(5);
expect(options).toMatchObject({
method: "GET",
url: "/route2",
request: { retryCount: 0 },
});
eventCount++;
},
onRateLimit: () => 1,
},
onRateLimit: () => 1,
},
});
});

await octokit.request("GET /route1", {
request: {
responses: [{ status: 201, headers: {}, data: {} }],
},
});
try {
await octokit.request("GET /route2", {
await octokit.request("GET /route1", {
request: {
responses: [
{
status: 403,
headers: {},
data: {
message: "You have exceeded a secondary rate limit",
},
},
],
responses: [{ status: 201, headers: {}, data: {} }],
},
});
throw new Error("Should not reach this point");
} catch (error: any) {
expect(error.status).toEqual(403);
}

expect(eventCount).toEqual(1);
});
});
try {
await octokit.request("GET /route2", {
request: {
responses: [
{
status: 403,
headers: {},
data: {
message: "You have exceeded a secondary rate limit",
},
},
],
},
});
throw new Error("Should not reach this point");
} catch (error: any) {
expect(error.status).toEqual(403);
}

expect(eventCount).toEqual(1);
});
}
);

describe("'rate-limit'", function () {
it("Should detect rate limit exceeded and broadcast event", async function () {
Expand All @@ -191,7 +202,7 @@ describe("Events", function () {
});
eventCount++;
},
onAbuseLimit: () => 1,
onSecondaryRateLimit: () => 1,
},
});
const t0 = Date.now();
Expand Down
25 changes: 15 additions & 10 deletions test/plugin.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,27 +37,32 @@ describe("General", function () {

it("Should require the user to pass both limit handlers", function () {
const message =
"You must pass the onAbuseLimit and onRateLimit error handlers";
"You must pass the onSecondaryRateLimit and onRateLimit error handlers";

expect(() => new TestOctokit()).toThrow(message);
expect(() => new TestOctokit({ throttle: {} })).toThrow(message);
expect(
() => new TestOctokit({ throttle: { onAbuseLimit: 5, onRateLimit: 5 } })
() =>
new TestOctokit({
throttle: { onSecondaryRateLimit: 5, onRateLimit: 5 },
})
).toThrow(message);
expect(
() =>
new TestOctokit({ throttle: { onAbuseLimit: 5, onRateLimit: () => 1 } })
new TestOctokit({
throttle: { onSecondaryRateLimit: 5, onRateLimit: () => 1 },
})
).toThrow(message);
expect(
() => new TestOctokit({ throttle: { onAbuseLimit: () => 1 } })
() => new TestOctokit({ throttle: { onSecondaryRateLimit: () => 1 } })
).toThrow(message);
expect(
() => new TestOctokit({ throttle: { onRateLimit: () => 1 } })
).toThrow(message);
expect(
() =>
new TestOctokit({
throttle: { onAbuseLimit: () => 1, onRateLimit: () => 1 },
throttle: { onSecondaryRateLimit: () => 1, onRateLimit: () => 1 },
})
).not.toThrow();
});
Expand All @@ -66,7 +71,7 @@ describe("General", function () {
describe("Github API best practices", function () {
it("Should linearize requests", async function () {
const octokit = new TestOctokit({
throttle: { onAbuseLimit: () => 1, onRateLimit: () => 1 },
throttle: { onSecondaryRateLimit: () => 1, onRateLimit: () => 1 },
});
const req1 = octokit.request("GET /route1", {
request: {
Expand Down Expand Up @@ -101,7 +106,7 @@ describe("Github API best practices", function () {
const octokit = new TestOctokit({
throttle: {
write: new Bottleneck.Group({ minTime: 50 }),
onAbuseLimit: () => 1,
onSecondaryRateLimit: () => 1,
onRateLimit: () => 1,
},
});
Expand Down Expand Up @@ -151,7 +156,7 @@ describe("Github API best practices", function () {
throttle: {
write: new Bottleneck.Group({ minTime: 50 }),
notifications: new Bottleneck.Group({ minTime: 100 }),
onAbuseLimit: () => 1,
onSecondaryRateLimit: () => 1,
onRateLimit: () => 1,
},
});
Expand Down Expand Up @@ -227,7 +232,7 @@ describe("Github API best practices", function () {
const octokit = new TestOctokit({
throttle: {
search: new Bottleneck.Group({ minTime: 50 }),
onAbuseLimit: () => 1,
onSecondaryRateLimit: () => 1,
onRateLimit: () => 1,
},
});
Expand Down Expand Up @@ -281,7 +286,7 @@ describe("Github API best practices", function () {
throttle: {
write: new Bottleneck.Group({ minTime: 50 }),
notifications: new Bottleneck.Group({ minTime: 150 }),
onAbuseLimit: () => 1,
onSecondaryRateLimit: () => 1,
onRateLimit: () => 1,
},
});
Expand Down
Loading

0 comments on commit ee84d9e

Please sign in to comment.