Skip to content

Commit

Permalink
Add unit tests and force flag for directory overwrite fix
Browse files Browse the repository at this point in the history
  • Loading branch information
ajkpersonal committed Nov 27, 2024
1 parent d7209f4 commit ac96a58
Show file tree
Hide file tree
Showing 6 changed files with 194 additions and 22 deletions.
124 changes: 124 additions & 0 deletions packages/cli/cli/src/__test__/checkOutputDirectory.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
import { AbsoluteFilePath, join, RelativeFilePath } from "@fern-api/fs-utils";
import { mkdir, writeFile } from "fs/promises";
import tmp from "tmp-promise";
import { checkOutputDirectory } from "../commands/generate/checkOutputDirectory";
import { getOutputDirectories } from "../persistence/getOutputDirectories";
import { storeOutputDirectories } from "../persistence/storeOutputDirectories";
import { describe, it, expect, beforeEach, vi, Mock } from "vitest";

describe("checkOutputDirectory", () => {
let mockCliContext: {
confirmPrompt: Mock;
};

beforeEach(() => {
mockCliContext = {
confirmPrompt: vi.fn()
};
});

it("doesn't prompt if directory doesn't exist", async () => {
const tmpDir = await tmp.dir();
const nonExistentPath = join(AbsoluteFilePath.of(tmpDir.path), RelativeFilePath.of("non-existent"));

const result = await checkOutputDirectory(nonExistentPath, mockCliContext as any, false);

Check warning on line 24 in packages/cli/cli/src/__test__/checkOutputDirectory.test.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected any. Specify a different type

expect(result).toEqual({
shouldProceed: true
});
expect(mockCliContext.confirmPrompt).not.toHaveBeenCalled();
});

it("doesn't prompt if directory is empty", async () => {
const tmpDir = await tmp.dir();
const emptyDir = join(AbsoluteFilePath.of(tmpDir.path), RelativeFilePath.of("empty"));
await mkdir(emptyDir);

const result = await checkOutputDirectory(emptyDir, mockCliContext as any, false);

Check warning on line 37 in packages/cli/cli/src/__test__/checkOutputDirectory.test.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected any. Specify a different type

expect(result).toEqual({
shouldProceed: true
});
expect(mockCliContext.confirmPrompt).not.toHaveBeenCalled();
});

it("prompts for confirmation if directory has files and not in safelist", async () => {
const tmpDir = await tmp.dir();
const dirWithFiles = join(AbsoluteFilePath.of(tmpDir.path), RelativeFilePath.of("with-files"));
await mkdir(dirWithFiles);
await writeFile(join(dirWithFiles, RelativeFilePath.of("test.txt")), "test");

mockCliContext.confirmPrompt.mockResolvedValueOnce(true);

const result = await checkOutputDirectory(dirWithFiles, mockCliContext as any, false);

Check warning on line 53 in packages/cli/cli/src/__test__/checkOutputDirectory.test.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected any. Specify a different type

expect(result).toEqual({
shouldProceed: true
});
expect(mockCliContext.confirmPrompt).toHaveBeenCalledTimes(1);
});

it("doesn't prompt if directory is in safelist", async () => {
const tmpDir = await tmp.dir();
const safelistedDir = join(AbsoluteFilePath.of(tmpDir.path), RelativeFilePath.of("safelisted"));
await mkdir(safelistedDir);
await writeFile(join(safelistedDir, RelativeFilePath.of("test.txt")), "test");

// Add to safelist
await storeOutputDirectories([safelistedDir]);

const result = await checkOutputDirectory(safelistedDir, mockCliContext as any, false);

Check warning on line 70 in packages/cli/cli/src/__test__/checkOutputDirectory.test.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected any. Specify a different type

expect(result).toEqual({
shouldProceed: true
});
expect(mockCliContext.confirmPrompt).not.toHaveBeenCalled();
});

it("saves directory to safelist when requested", async () => {
const tmpDir = await tmp.dir();
const dirToSafelist = join(AbsoluteFilePath.of(tmpDir.path), RelativeFilePath.of("to-safelist"));
await mkdir(dirToSafelist);
await writeFile(join(dirToSafelist, RelativeFilePath.of("test.txt")), "test");

mockCliContext.confirmPrompt.mockResolvedValueOnce(true);

const result = await checkOutputDirectory(dirToSafelist, mockCliContext as any, false);

Check warning on line 86 in packages/cli/cli/src/__test__/checkOutputDirectory.test.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected any. Specify a different type

expect(result).toEqual({
shouldProceed: true
});

// Verify directory was added to safelist
const savedDirectories = await getOutputDirectories();
expect(savedDirectories).toContain(dirToSafelist);
});

it("doesn't proceed if user declines overwrite", async () => {
const tmpDir = await tmp.dir();
const dirWithFiles = join(AbsoluteFilePath.of(tmpDir.path), RelativeFilePath.of("with-files"));
await mkdir(dirWithFiles);
await writeFile(join(dirWithFiles, RelativeFilePath.of("test.txt")), "test");

mockCliContext.confirmPrompt.mockResolvedValueOnce(false); // overwrite prompt

const result = await checkOutputDirectory(dirWithFiles, mockCliContext as any, false);

Check warning on line 105 in packages/cli/cli/src/__test__/checkOutputDirectory.test.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected any. Specify a different type

expect(result).toEqual({
shouldProceed: false
});
expect(mockCliContext.confirmPrompt).toHaveBeenCalledTimes(1);
});

it("doesn't prompt if force is true", async () => {
const tmpDir = await tmp.dir();
const dirWithFiles = join(AbsoluteFilePath.of(tmpDir.path), RelativeFilePath.of("with-files"));
await mkdir(dirWithFiles);
await writeFile(join(dirWithFiles, RelativeFilePath.of("test.txt")), "test");

const result = await checkOutputDirectory(dirWithFiles, mockCliContext as any, true);

Check warning on line 119 in packages/cli/cli/src/__test__/checkOutputDirectory.test.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected any. Specify a different type

expect(result).toEqual({ shouldProceed: true });
expect(mockCliContext.confirmPrompt).not.toHaveBeenCalled();
});
});
42 changes: 42 additions & 0 deletions packages/cli/cli/src/__test__/checkOutputDirectoryCI.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { AbsoluteFilePath, join, RelativeFilePath } from "@fern-api/fs-utils";
import { mkdir, writeFile } from "fs/promises";
import tmp from "tmp-promise";
import { checkOutputDirectory } from "../commands/generate/checkOutputDirectory";
import { describe, it, expect, beforeEach, afterEach, vi, Mock } from "vitest";

describe("checkOutputDirectory in CI", () => {
let originalEnv: NodeJS.ProcessEnv;
let mockCliContext: {
confirmPrompt: Mock;
};

beforeEach(() => {
originalEnv = process.env;
process.env = {
...process.env,
CI: "true"
};

mockCliContext = {
confirmPrompt: vi.fn()
};
});

afterEach(() => {
process.env = originalEnv;
});

it("doesn't prompt in CI environment even with files present", async () => {
const tmpDir = await tmp.dir();
const dirWithFiles = join(AbsoluteFilePath.of(tmpDir.path), RelativeFilePath.of("with-files"));
await mkdir(dirWithFiles);
await writeFile(join(dirWithFiles, RelativeFilePath.of("test.txt")), "test");

const result = await checkOutputDirectory(dirWithFiles, mockCliContext as any, false);

Check warning on line 35 in packages/cli/cli/src/__test__/checkOutputDirectoryCI.test.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected any. Specify a different type

expect(result).toEqual({
shouldProceed: true
});
expect(mockCliContext.confirmPrompt).not.toHaveBeenCalled();
});
});
11 changes: 9 additions & 2 deletions packages/cli/cli/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,11 @@ function addGenerateCommand(cli: Argv<GlobalCliOptions>, cliContext: CliContext)
boolean: true,
default: false,
description: "Prevent auto-deletion of the Docker containers."
})
.option("force", {
boolean: true,
default: false,
description: "Ignore prompts to confirm generation, defaults to false"
}),
async (argv) => {
if (argv.api != null && argv.docs != null) {
Expand All @@ -401,7 +406,8 @@ function addGenerateCommand(cli: Argv<GlobalCliOptions>, cliContext: CliContext)
keepDocker: argv.keepDocker,
useLocalDocker: argv.local,
preview: argv.preview,
mode: argv.mode
mode: argv.mode,
force: argv.force
});
}
if (argv.docs != null) {
Expand Down Expand Up @@ -438,7 +444,8 @@ function addGenerateCommand(cli: Argv<GlobalCliOptions>, cliContext: CliContext)
keepDocker: argv.keepDocker,
useLocalDocker: argv.local,
preview: argv.preview,
mode: argv.mode
mode: argv.mode,
force: argv.force
});
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { readdir } from "fs/promises";
import { CliContext } from "../../cli-context/CliContext";
import { getOutputDirectories } from "../../persistence/getOutputDirectories";
import { storeOutputDirectories } from "../../persistence/storeOutputDirectories";
import { isCI } from "../../utils/isCI";

export interface CheckOutputDirectoryResult {
shouldProceed: boolean;
Expand All @@ -16,9 +17,10 @@ export interface CheckOutputDirectoryResult {
*/
export async function checkOutputDirectory(
outputPath: AbsoluteFilePath | undefined,
cliContext: CliContext
cliContext: CliContext,
force: boolean
): Promise<CheckOutputDirectoryResult> {
if (!outputPath) {
if (!outputPath || isCI() || force) {
return {
shouldProceed: true
};
Expand Down
26 changes: 12 additions & 14 deletions packages/cli/cli/src/commands/generate/generateAPIWorkspaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export async function generateAPIWorkspaces({
keepDocker,
useLocalDocker,
preview,
mode
mode,
force
}: {
project: Project;
cliContext: CliContext;
Expand All @@ -35,6 +36,7 @@ export async function generateAPIWorkspaces({
keepDocker: boolean;
preview: boolean;
mode: GenerationMode | undefined;
force: boolean;
}): Promise<void> {
let token: FernToken | undefined = undefined;

Expand All @@ -54,19 +56,15 @@ export async function generateAPIWorkspaces({
token = currentToken;
}

if (!isCI()) {
for (const workspace of project.apiWorkspaces) {
for (const generator of workspace.generatorsConfiguration?.groups.flatMap((group) => group.generators) ??
[]) {
if (generator.absolutePathToLocalOutput) {
const { shouldProceed } = await checkOutputDirectory(
generator.absolutePathToLocalOutput,
cliContext
);
if (!shouldProceed) {
cliContext.failAndThrow("Generation cancelled");
}
}
for (const workspace of project.apiWorkspaces) {
for (const generator of workspace.generatorsConfiguration?.groups.flatMap((group) => group.generators) ?? []) {
const { shouldProceed } = await checkOutputDirectory(
generator.absolutePathToLocalOutput,
cliContext,
force
);
if (!shouldProceed) {
cliContext.failAndThrow("Generation cancelled");
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit ac96a58

Please sign in to comment.