Skip to content

Commit

Permalink
feat: make git pushes more robust (#156)
Browse files Browse the repository at this point in the history
  • Loading branch information
kormide authored Mar 18, 2024
1 parent ffd6f39 commit b3e277c
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 1 deletion.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"axios": "^1.4.0",
"axios-retry": "^4.0.0",
"diff": "^5.1.0",
"exponential-backoff": "3.1.1",
"extract-zip": "^2.0.1",
"gcp-metadata": "^6.0.0",
"nodemailer": "^6.7.8",
Expand Down
54 changes: 54 additions & 0 deletions src/domain/create-entry.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createTwoFilesPatch } from "diff";
import { BackoffOptions, backOff } from "exponential-backoff";
import { Mocked, mocked } from "jest-mock";
import { randomUUID } from "node:crypto";
import fs, { PathLike } from "node:fs";
Expand Down Expand Up @@ -37,6 +38,9 @@ jest.mock("../infrastructure/github");
jest.mock("./integrity-hash");
jest.mock("./release-archive");
jest.mock("node:fs");
jest.mock("exponential-backoff");

const realExponentialBackoff = jest.requireActual("exponential-backoff");

const mockedFileReads: { [path: string]: string } = {};
const EXTRACTED_MODULE_PATH = "/fake/path/to/MODULE.bazel";
Expand Down Expand Up @@ -1061,6 +1065,17 @@ describe("commitEntryToNewBranch", () => {
});

describe("pushEntryToFork", () => {
beforeEach(() => {
// Reduce the exponential-backoff delay to 0 for tests
(backOff as unknown as jest.SpyInstance).mockImplementation(
(request: () => Promise<void>, options?: BackoffOptions) =>
realExponentialBackoff.backOff(request, {
...options,
startingDelay: 0,
})
);
});

test("acquires an authenticated remote url for the bcr fork", async () => {
const bcrRepo = CANONICAL_BCR;
const bcrForkRepo = new Repository("bazel-central-registry", "aspect");
Expand Down Expand Up @@ -1136,6 +1151,45 @@ describe("pushEntryToFork", () => {
branchName
);
});

test("retries 5 times if it fails", async () => {
const bcrRepo = CANONICAL_BCR;
const bcrForkRepo = new Repository("bazel-central-registry", "aspect");
const branchName = `repo/[email protected]`;

(backOff as unknown as jest.SpyInstance).mockImplementation(
(request: () => Promise<any>, options?: BackoffOptions) => {
return realExponentialBackoff.backOff(request, {
...options,
startingDelay: 0,
});
}
);

mockGitClient.push
.mockRejectedValueOnce(new Error("failed push"))
.mockRejectedValueOnce(new Error("failed push"))
.mockRejectedValueOnce(new Error("failed push"))
.mockRejectedValueOnce(new Error("failed push"))
.mockResolvedValueOnce(undefined);

await createEntryService.pushEntryToFork(bcrForkRepo, bcrRepo, branchName);

expect(mockGitClient.push).toHaveBeenCalledTimes(5);
});

test("fails after the 5th retry", async () => {
const bcrRepo = CANONICAL_BCR;
const bcrForkRepo = new Repository("bazel-central-registry", "aspect");
const branchName = `repo/[email protected]`;

mockGitClient.push.mockRejectedValue(new Error("failed push"));

await expect(
createEntryService.pushEntryToFork(bcrForkRepo, bcrRepo, branchName)
).rejects.toThrow();
expect(mockGitClient.push).toHaveBeenCalledTimes(5);
});
});

function mockRulesetFiles(
Expand Down
9 changes: 8 additions & 1 deletion src/domain/create-entry.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createTwoFilesPatch, parsePatch } from "diff";
import { backOff } from "exponential-backoff";
import { randomBytes } from "node:crypto";
import fs, { readFileSync } from "node:fs";
import path from "node:path";
Expand Down Expand Up @@ -170,7 +171,13 @@ export class CreateEntryService {
await this.gitClient.push(bcr.diskPath, "origin", branch);
return;
}
await this.gitClient.push(bcr.diskPath, "authed-fork", branch);

await backOff(
() => this.gitClient.push(bcr.diskPath, "authed-fork", branch),
{
numOfAttempts: 5,
}
);
}

private addPatches(
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2408,6 +2408,11 @@ expect@^28.0.0, expect@^28.1.3:
jest-message-util "^28.1.3"
jest-util "^28.1.3"

[email protected]:
version "3.1.1"
resolved "https://registry.yarnpkg.com/exponential-backoff/-/exponential-backoff-3.1.1.tgz#64ac7526fe341ab18a39016cd22c787d01e00bf6"
integrity sha512-dX7e/LHVJ6W3DE1MHWi9S1EYzDESENfLrYohG2G++ovZrYOkm4Knwa0mc1cn84xJOR4KEU0WSchhLbd0UklbHw==

express@^4.14.0, express@^4.16.4:
version "4.18.2"
resolved "https://registry.yarnpkg.com/express/-/express-4.18.2.tgz#3fabe08296e930c796c19e3c516979386ba9fd59"
Expand Down

0 comments on commit b3e277c

Please sign in to comment.