fix(ci): preserve release proposal overflow notes
This commit is contained in:
1 parent
26c3e9e413
commit
2cb29693c3
5 files changed
+152
-1
No files matched your search
@@ -81,6 +81,51 @@
|
||||
// In this case, there is no squashed merge commit. This could be a simple
|
||||
// merge commit, a rebase merge commit, or a direct commit to the branch.
|
||||
// Fallback to fetching the list of commits from the REST API. In the future
|
||||
@@ -1154,15 +1154,40 @@
|
||||
* @returns {string} HTML URL of the new file
|
||||
*/
|
||||
async createFileOnNewBranch(filename, contents, newBranchName, baseBranchName) {
|
||||
- // create or update new branch to match base branch
|
||||
- await this.forkBranch(newBranchName, baseBranchName);
|
||||
- // use the single file upload API
|
||||
+ // Preserve existing notes until the replacement file is committed.
|
||||
+ // A failed write must not leave an open proposal pointing at a missing file.
|
||||
+ if (!(await this.getBranchSha(newBranchName))) {
|
||||
+ const baseSha = await this.getBranchSha(baseBranchName);
|
||||
+ if (!baseSha) {
|
||||
+ throw new Error(`Unable to find base branch: ${baseBranchName}`);
|
||||
+ }
|
||||
+ await this.createNewBranch(newBranchName, baseSha);
|
||||
+ }
|
||||
+ let existingSha;
|
||||
+ try {
|
||||
+ const { data } = await this.octokit.repos.getContent({
|
||||
+ owner: this.repository.owner,
|
||||
+ repo: this.repository.repo,
|
||||
+ path: filename,
|
||||
+ ref: newBranchName,
|
||||
+ });
|
||||
+ if (Array.isArray(data) || data.type !== 'file' || !data.sha) {
|
||||
+ throw new Error(`Expected a file at ${filename} on branch: ${newBranchName}`);
|
||||
+ }
|
||||
+ existingSha = data.sha;
|
||||
+ }
|
||||
+ catch (error) {
|
||||
+ if (!(error instanceof request_error_1.RequestError && error.status === 404)) {
|
||||
+ throw error;
|
||||
+ }
|
||||
+ }
|
||||
const { data: { content }, } = await this.octokit.repos.createOrUpdateFileContents({
|
||||
owner: this.repository.owner,
|
||||
repo: this.repository.repo,
|
||||
path: filename,
|
||||
// contents need to be base64 encoded
|
||||
- content: Buffer.from(contents, 'binary').toString('base64'),
|
||||
+ content: Buffer.from(contents, 'utf8').toString('base64'),
|
||||
+ ...(existingSha ? { sha: existingSha } : {}),
|
||||
message: 'Saving release notes',
|
||||
branch: newBranchName,
|
||||
});
|
||||
--- a/build/src/github.d.ts
|
||||
+++ b/build/src/github.d.ts
|
||||
@@ -47,6 +47,8 @@
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
export function applyReleasePleasePatch(directory?: string): Promise<void>;
|
||||
|
||||
export function describeReleaseProposalError(error: unknown): string;
|
||||
|
||||
export function releaseProposalSearchDepth(
|
||||
run: (command: string, args: string[]) => Promise<string>,
|
||||
sourceSha: string,
|
||||
|
||||
@@ -13,6 +13,15 @@ const patch = fileURLToPath(
|
||||
new URL("../../patches/release-please+17.3.0.patch", import.meta.url),
|
||||
);
|
||||
|
||||
export function describeReleaseProposalError(error) {
|
||||
if (!(error instanceof Error)) return String(error);
|
||||
const description = error.message || error.name || "Unknown error";
|
||||
const status = error.status;
|
||||
return Number.isInteger(status) && status >= 400 && status <= 599
|
||||
? `${description} (HTTP ${status})`
|
||||
: description;
|
||||
}
|
||||
|
||||
export async function applyReleasePleasePatch(
|
||||
directory = dirname(require.resolve("release-please/package.json")),
|
||||
) {
|
||||
|
||||
@@ -4,6 +4,7 @@ import { appendFile } from "node:fs/promises";
|
||||
import { parseArgs, promisify } from "node:util";
|
||||
import {
|
||||
applyReleasePleasePatch,
|
||||
describeReleaseProposalError,
|
||||
releaseProposalSearchDepth,
|
||||
} from "./lib/release-proposal.mjs";
|
||||
|
||||
@@ -63,7 +64,7 @@ async function main() {
|
||||
|
||||
await main().catch((error) => {
|
||||
process.stderr.write(
|
||||
`Release proposal failed: ${error instanceof Error ? error.message : String(error)}\n`,
|
||||
`Release proposal failed: ${describeReleaseProposalError(error)}\n`,
|
||||
);
|
||||
process.exitCode = 1;
|
||||
});
|
||||
@@ -10,11 +10,13 @@ import { createRequire } from "node:module";
|
||||
import { dirname, join } from "node:path";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
import { promisify } from "node:util";
|
||||
import { RequestError } from "@octokit/request-error";
|
||||
import { afterEach, beforeAll, expect, it, vi } from "vitest";
|
||||
import { z } from "zod";
|
||||
|
||||
import {
|
||||
applyReleasePleasePatch,
|
||||
describeReleaseProposalError,
|
||||
releaseProposalSearchDepth,
|
||||
} from "../../../scripts/lib/release-proposal.mjs";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
@@ -33,6 +35,16 @@ const silent = {
|
||||
trace() {},
|
||||
};
|
||||
|
||||
it("reports an HTTP status when a release proposal error has no message", () => {
|
||||
const failure = Object.assign(new Error(""), {
|
||||
name: "HttpError",
|
||||
status: 504,
|
||||
request: { headers: { authorization: "secret" } },
|
||||
});
|
||||
expect(describeReleaseProposalError(failure)).toBe("HttpError (HTTP 504)");
|
||||
expect(describeReleaseProposalError(new Error("broken"))).toBe("broken");
|
||||
});
|
||||
|
||||
beforeAll(async () => {
|
||||
upstream = await import("release-please");
|
||||
githubRuntime = require("release-please/build/src/github.js");
|
||||
@@ -40,6 +52,88 @@ beforeAll(async () => {
|
||||
|
||||
afterEach(() => vi.restoreAllMocks());
|
||||
|
||||
it("preserves the existing overflow notes if their replacement fails", async () => {
|
||||
const write = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new Error("upload failed"))
|
||||
.mockResolvedValueOnce({
|
||||
data: { content: { html_url: "https://example.test/notes" } },
|
||||
});
|
||||
const getContent = vi.fn().mockResolvedValue({
|
||||
data: { type: "file", sha: "existing-blob" },
|
||||
});
|
||||
const createNewBranch = vi.fn();
|
||||
const forkBranch = vi.fn();
|
||||
const github = {
|
||||
repository: { owner: "fixture", repo: "rea" },
|
||||
getBranchSha: vi.fn().mockResolvedValue("existing-branch"),
|
||||
createNewBranch,
|
||||
forkBranch,
|
||||
octokit: { repos: { getContent, createOrUpdateFileContents: write } },
|
||||
} as unknown as Awaited<ReturnType<typeof upstream.GitHub.create>>;
|
||||
const update = () =>
|
||||
upstream.GitHub.prototype.createFileOnNewBranch.call(
|
||||
github,
|
||||
"release-notes.md",
|
||||
"Notes: café",
|
||||
"notes",
|
||||
"main",
|
||||
);
|
||||
await expect(update()).rejects.toThrow("upload failed");
|
||||
expect(forkBranch).not.toHaveBeenCalled();
|
||||
expect(createNewBranch).not.toHaveBeenCalled();
|
||||
await expect(update()).resolves.toBe("https://example.test/notes");
|
||||
expect(write).toHaveBeenLastCalledWith(
|
||||
expect.objectContaining({
|
||||
branch: "notes",
|
||||
sha: "existing-blob",
|
||||
content: Buffer.from("Notes: café", "utf8").toString("base64"),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("creates overflow notes on a new branch without requiring an existing file", async () => {
|
||||
const write = vi.fn().mockResolvedValue({
|
||||
data: { content: { html_url: "https://example.test/notes" } },
|
||||
});
|
||||
const createNewBranch = vi.fn();
|
||||
const github = {
|
||||
repository: { owner: "fixture", repo: "rea" },
|
||||
getBranchSha: vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(undefined)
|
||||
.mockResolvedValueOnce("base-sha"),
|
||||
createNewBranch,
|
||||
octokit: {
|
||||
repos: {
|
||||
getContent: vi.fn().mockRejectedValue(
|
||||
new RequestError("missing", 404, {
|
||||
request: {
|
||||
method: "GET",
|
||||
url: "https://example.test/notes",
|
||||
headers: {},
|
||||
},
|
||||
}),
|
||||
),
|
||||
createOrUpdateFileContents: write,
|
||||
},
|
||||
},
|
||||
} as unknown as Awaited<ReturnType<typeof upstream.GitHub.create>>;
|
||||
await expect(
|
||||
upstream.GitHub.prototype.createFileOnNewBranch.call(
|
||||
github,
|
||||
"release-notes.md",
|
||||
"Notes",
|
||||
"notes",
|
||||
"main",
|
||||
),
|
||||
).resolves.toBe("https://example.test/notes");
|
||||
expect(createNewBranch).toHaveBeenCalledWith("notes", "base-sha");
|
||||
expect(write).toHaveBeenCalledWith(
|
||||
expect.not.objectContaining({ sha: expect.anything() }),
|
||||
);
|
||||
});
|
||||
|
||||
function respond(value: unknown, status = 200) {
|
||||
return new Response(JSON.stringify(value), {
|
||||
status,
|
||||
|
||||
Reference in new issue
Block a user