From ac96a588e777dc87ad19a2b4674c1cd6af401ea8 Mon Sep 17 00:00:00 2001 From: Aman Javeri Kadri Date: Wed, 27 Nov 2024 16:15:11 +0530 Subject: [PATCH] Add unit tests and force flag for directory overwrite fix --- .../src/__test__/checkOutputDirectory.test.ts | 124 ++++++++++++++++++ .../__test__/checkOutputDirectoryCI.test.ts | 42 ++++++ packages/cli/cli/src/cli.ts | 11 +- .../commands/generate/checkOutputDirectory.ts | 6 +- .../generate/generateAPIWorkspaces.ts | 26 ++-- pnpm-lock.yaml | 7 +- 6 files changed, 194 insertions(+), 22 deletions(-) create mode 100644 packages/cli/cli/src/__test__/checkOutputDirectory.test.ts create mode 100644 packages/cli/cli/src/__test__/checkOutputDirectoryCI.test.ts diff --git a/packages/cli/cli/src/__test__/checkOutputDirectory.test.ts b/packages/cli/cli/src/__test__/checkOutputDirectory.test.ts new file mode 100644 index 00000000000..66dfd102232 --- /dev/null +++ b/packages/cli/cli/src/__test__/checkOutputDirectory.test.ts @@ -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); + + 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); + + 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); + + 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); + + 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); + + 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); + + 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); + + expect(result).toEqual({ shouldProceed: true }); + expect(mockCliContext.confirmPrompt).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/cli/src/__test__/checkOutputDirectoryCI.test.ts b/packages/cli/cli/src/__test__/checkOutputDirectoryCI.test.ts new file mode 100644 index 00000000000..44e4572d47e --- /dev/null +++ b/packages/cli/cli/src/__test__/checkOutputDirectoryCI.test.ts @@ -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); + + expect(result).toEqual({ + shouldProceed: true + }); + expect(mockCliContext.confirmPrompt).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/cli/src/cli.ts b/packages/cli/cli/src/cli.ts index cf73175b2fb..117553640f0 100644 --- a/packages/cli/cli/src/cli.ts +++ b/packages/cli/cli/src/cli.ts @@ -380,6 +380,11 @@ function addGenerateCommand(cli: Argv, 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) { @@ -401,7 +406,8 @@ function addGenerateCommand(cli: Argv, cliContext: CliContext) keepDocker: argv.keepDocker, useLocalDocker: argv.local, preview: argv.preview, - mode: argv.mode + mode: argv.mode, + force: argv.force }); } if (argv.docs != null) { @@ -438,7 +444,8 @@ function addGenerateCommand(cli: Argv, cliContext: CliContext) keepDocker: argv.keepDocker, useLocalDocker: argv.local, preview: argv.preview, - mode: argv.mode + mode: argv.mode, + force: argv.force }); } ); diff --git a/packages/cli/cli/src/commands/generate/checkOutputDirectory.ts b/packages/cli/cli/src/commands/generate/checkOutputDirectory.ts index c06ecb46611..425a443a7d8 100644 --- a/packages/cli/cli/src/commands/generate/checkOutputDirectory.ts +++ b/packages/cli/cli/src/commands/generate/checkOutputDirectory.ts @@ -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; @@ -16,9 +17,10 @@ export interface CheckOutputDirectoryResult { */ export async function checkOutputDirectory( outputPath: AbsoluteFilePath | undefined, - cliContext: CliContext + cliContext: CliContext, + force: boolean ): Promise { - if (!outputPath) { + if (!outputPath || isCI() || force) { return { shouldProceed: true }; diff --git a/packages/cli/cli/src/commands/generate/generateAPIWorkspaces.ts b/packages/cli/cli/src/commands/generate/generateAPIWorkspaces.ts index 7baf31e152c..3ec5ad37a0f 100644 --- a/packages/cli/cli/src/commands/generate/generateAPIWorkspaces.ts +++ b/packages/cli/cli/src/commands/generate/generateAPIWorkspaces.ts @@ -24,7 +24,8 @@ export async function generateAPIWorkspaces({ keepDocker, useLocalDocker, preview, - mode + mode, + force }: { project: Project; cliContext: CliContext; @@ -35,6 +36,7 @@ export async function generateAPIWorkspaces({ keepDocker: boolean; preview: boolean; mode: GenerationMode | undefined; + force: boolean; }): Promise { let token: FernToken | undefined = undefined; @@ -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"); } } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 651acfb8737..0fc192dce73 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3759,10 +3759,6 @@ importers: version: 2.1.4(@types/node@18.7.18)(jsdom@20.0.3)(sass@1.72.0)(terser@5.31.5) packages/cli/cli: - dependencies: - '@inquirer/prompts': - specifier: ^7.1.0 - version: 7.1.0(@types/node@18.7.18) devDependencies: '@fern-api/api-workspace-commons': specifier: workspace:* @@ -3881,6 +3877,9 @@ importers: '@fern-typescript/fetcher': specifier: workspace:* version: link:../../../generators/typescript/utils/core-utilities/fetcher + '@inquirer/prompts': + specifier: ^7.1.0 + version: 7.1.0(@types/node@18.7.18) '@types/axios': specifier: ^0.14.0 version: 0.14.0