Skip to content

Gracefully handle ignored symlinks #35

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.

Sign up for GitHub

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

Closed
s0 wants to merge 5 commits into from
Closed

Gracefully handle ignored symlinks #35

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f99e98d
tests: add a symlink to changes
h3rmanj May 7, 2025
c2c5699
tests: Add more tests for symlinks
s0 May 7, 2025
030f74f
fix: check isIgnored first
h3rmanj May 7, 2025
e98f619
fix: Add more useful error for non-ignored symlinks
s0 May 7, 2025
1e78dfb
chore: add changeset
h3rmanj May 7, 2025
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
Prev Previous commit
Next Next commit
tests: Add more tests for symlinks
  • Loading branch information
@s0
s0 committed May 7, 2025
commit c2c569918096e886968104bd3f5270ca30efd0e5
240 changes: 182 additions & 58 deletions src/test/integration/git.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,14 @@ const expectParentHasOid = async ({
expect(commit.parents.nodes).toEqual([{ oid }]);
};

const makeFileChanges = async (repoDirectory: string) => {
const makeFileChanges = async (
repoDirectory: string,
changegroup:
| "standard"
| "with-ignored-symlink"
| "with-included-valid-symlink"
| "with-included-invalid-symlink",
) => {
// Update an existing file
await fs.promises.writeFile(
path.join(repoDirectory, "LICENSE"),
Expand Down Expand Up @@ -113,11 +120,34 @@ const makeFileChanges = async (repoDirectory: string) => {
path.join(repoDirectory, "coverage", "foo", "bar"),
"This file should be ignored",
);
// Add a symlink in ignored directory
await fs.promises.symlink(
path.join(repoDirectory, "nested"),
path.join(repoDirectory, "node_modules", "nested")
);
if (changegroup === "with-ignored-symlink") {
// node_modules is ignored in this repo
await fs.promises.mkdir(path.join(repoDirectory, "node_modules"), {
recursive: true,
});
await fs.promises.symlink(
path.join(repoDirectory, "non-existent"),
path.join(repoDirectory, "node_modules", "nested"),
);
}
if (changegroup === "with-included-valid-symlink") {
await fs.promises.mkdir(path.join(repoDirectory, "some-dir"), {
recursive: true,
});
await fs.promises.symlink(
path.join(repoDirectory, "README.md"),
path.join(repoDirectory, "some-dir", "nested"),
);
}
if (changegroup === "with-included-invalid-symlink") {
await fs.promises.mkdir(path.join(repoDirectory, "some-dir"), {
recursive: true,
});
await fs.promises.symlink(
path.join(repoDirectory, "non-existent"),
path.join(repoDirectory, "some-dir", "nested"),
);
}
};

const makeFileChangeAssertions = async (branch: string) => {
Expand Down Expand Up @@ -159,62 +189,156 @@ describe("git", () => {
describe("commitChangesFromRepo", () => {
const testDir = path.join(ROOT_TEMP_DIRECTORY, "commitChangesFromRepo");

it("should correctly commit all changes", async () => {
const branch = `${TEST_BRANCH_PREFIX}-multiple-changes`;
branches.push(branch);

await fs.promises.mkdir(testDir, { recursive: true });
const repoDirectory = path.join(testDir, "repo-1");

// Clone the git repo locally using the git cli and child-process
await new Promise<void>((resolve, reject) => {
const p = execFile(
"git",
["clone", process.cwd(), "repo-1"],
{ cwd: testDir },
(error) => {
if (error) {
reject(error);
} else {
resolve();
}
for (const group of ["standard", "with-ignored-symlink"] as const) {
it(`should correctly commit all changes for group: ${group}`, async () => {
const branch = `${TEST_BRANCH_PREFIX}-multiple-changes-${group}`;
branches.push(branch);

await fs.promises.mkdir(testDir, { recursive: true });
const repoDirectory = path.join(testDir, `repo-1-${group}`);

// Clone the git repo locally using the git cli and child-process
await new Promise<void>((resolve, reject) => {
const p = execFile(
"git",
["clone", process.cwd(), `repo-1-${group}`],
{ cwd: testDir },
(error) => {
if (error) {
reject(error);
} else {
resolve();
}
},
);
p.stdout?.pipe(process.stdout);
p.stderr?.pipe(process.stderr);
});

await makeFileChanges(repoDirectory, group);

// Push the changes
await commitChangesFromRepo({
octokit,
...REPO,
branch,
message: {
headline: "Test commit",
body: "This is a test commit",
},
repoDirectory,
log,
});

await waitForGitHubToBeReady();

await makeFileChangeAssertions(branch);

// Expect the OID to be the HEAD commit
const oid =
(
await git.log({
fs,
dir: repoDirectory,
ref: "HEAD",
depth: 1,
})
)[0]?.oid ?? "NO_OID";

await expectParentHasOid({ branch, oid });
});
}

describe(`should throw appropriate error when symlink is present`, () => {
it(`and file does not exist`, async () => {
const branch = `${TEST_BRANCH_PREFIX}-invalid-symlink-error`;
branches.push(branch);

await fs.promises.mkdir(testDir, { recursive: true });
const repoDirectory = path.join(testDir, `repo-invalid-symlink`);

// Clone the git repo locally using the git cli and child-process
await new Promise<void>((resolve, reject) => {
const p = execFile(
"git",
["clone", process.cwd(), `repo-invalid-symlink`],
{ cwd: testDir },
(error) => {
if (error) {
reject(error);
} else {
resolve();
}
},
);
p.stdout?.pipe(process.stdout);
p.stderr?.pipe(process.stderr);
});

await makeFileChanges(repoDirectory, "with-included-invalid-symlink");

// Push the changes
await expect(() =>
commitChangesFromRepo({
octokit,
...REPO,
branch,
message: {
headline: "Test commit",
body: "This is a test commit",
},
repoDirectory,
log,
}),
).rejects.toThrow(
"Unexpected symlink at some-dir/nested, GitHub API only supports files and directories. You may need to add this file to .gitignore",
);
p.stdout?.pipe(process.stdout);
p.stderr?.pipe(process.stderr);
});

await makeFileChanges(repoDirectory);

// Push the changes
await commitChangesFromRepo({
octokit,
...REPO,
branch,
message: {
headline: "Test commit",
body: "This is a test commit",
},
repoDirectory,
log,
it(`and file exists`, async () => {
const branch = `${TEST_BRANCH_PREFIX}-valid-symlink-error`;
branches.push(branch);

await fs.promises.mkdir(testDir, { recursive: true });
const repoDirectory = path.join(testDir, `repo-valid-symlink`);

// Clone the git repo locally using the git cli and child-process
await new Promise<void>((resolve, reject) => {
const p = execFile(
"git",
["clone", process.cwd(), `repo-valid-symlink`],
{ cwd: testDir },
(error) => {
if (error) {
reject(error);
} else {
resolve();
}
},
);
p.stdout?.pipe(process.stdout);
p.stderr?.pipe(process.stderr);
});

await makeFileChanges(repoDirectory, "with-included-valid-symlink");

// Push the changes
await expect(() =>
commitChangesFromRepo({
octokit,
...REPO,
branch,
message: {
headline: "Test commit",
body: "This is a test commit",
},
repoDirectory,
log,
}),
).rejects.toThrow(
"Unexpected symlink at some-dir/nested, GitHub API only supports files and directories. You may need to add this file to .gitignore",
);
});

await waitForGitHubToBeReady();

await makeFileChangeAssertions(branch);

// Expect the OID to be the HEAD commit
const oid =
(
await git.log({
fs,
dir: repoDirectory,
ref: "HEAD",
depth: 1,
})
)[0]?.oid ?? "NO_OID";

await expectParentHasOid({ branch, oid });
});

it("should correctly be able to base changes off specific commit", async () => {
Expand Down Expand Up @@ -242,7 +366,7 @@ describe("git", () => {
p.stderr?.pipe(process.stderr);
});

makeFileChanges(repoDirectory);
makeFileChanges(repoDirectory, "standard");

// Determine the previous commit hash
const gitLog = await git.log({
Expand Down