-
Notifications
You must be signed in to change notification settings - Fork 163
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(cli): Adds check for overwriting files in generate output (#5278)
* Adds check for overrwriting files in generate output * Adds check for CI env for directory overwrite prompt * Updates from PR comments * Add unit tests and force flag for directory overwrite fix * Adds e2e tests for CI env and --force flag * Fixes lint errors and tests in CI env * Add changelog and create directory for output location persistence files * chore: update changelog * chore: update changelog * Update versions.yml * chore: update changelog --------- Co-authored-by: ajkpersonal <[email protected]> Co-authored-by: Darwin Ding <[email protected]> Co-authored-by: dubwub <[email protected]> Co-authored-by: Deep Singhvi <[email protected]> Co-authored-by: dsinghvi <[email protected]>
- Loading branch information
6 people
authored
Dec 9, 2024
1 parent
0002a9a
commit f448b33
Showing
19 changed files
with
597 additions
and
10 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| ## 0.45.4-rc0 | ||
| **`(fix):`** The CLI prompts the user to confirm output directory overwrites on fern generate. | ||
|
|
||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| ## 1.6.0 | ||
| **`(fix):`** The FastAPI generator now supports a new mode for pydantic model generation, | ||
| to use pydantic v1 on v2 versions. | ||
|
|
||
| ```yml generators.yml | ||
| generators: | ||
| server: | ||
| - name: fernapi/fern-fastapi-server | ||
| version: 1.6.0 | ||
| config: | ||
| pydantic: | ||
| version: "v1_on_v2" | ||
| ```` | ||
|
|
||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| ## 4.3.9 | ||
| **`(fix):`** Fix indentation in generated README.md sections to ensure proper formatting and readability. | ||
|
|
||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
134 changes: 134 additions & 0 deletions
134
packages/cli/cli/src/__test__/checkOutputDirectory.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| 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/output-directories/getOutputDirectories"; | ||
| import { storeOutputDirectories } from "../persistence/output-directories/storeOutputDirectories"; | ||
| import { describe, it, expect, beforeEach, vi, Mock, afterEach } from "vitest"; | ||
| import { CliContext } from "../cli-context/CliContext"; | ||
| import { isCI } from "../utils/isCI"; | ||
|
|
||
| vi.mock("../utils/isCI", () => ({ | ||
| isCI: vi.fn().mockReturnValue(false) | ||
| })); | ||
|
|
||
| describe("checkOutputDirectory", () => { | ||
| let mockCliContext: { | ||
| confirmPrompt: Mock & ((message: string, defaultValue?: boolean) => Promise<boolean>); | ||
| }; | ||
|
|
||
| beforeEach(() => { | ||
| mockCliContext = { | ||
| confirmPrompt: vi.fn().mockImplementation(async () => true) | ||
| }; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| 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 unknown as CliContext, 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 unknown as CliContext, 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 unknown as CliContext, 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 unknown as CliContext, 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 unknown as CliContext, 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 unknown as CliContext, 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 unknown as CliContext, true); | ||
|
|
||
| expect(result).toEqual({ shouldProceed: true }); | ||
| expect(mockCliContext.confirmPrompt).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
39 changes: 39 additions & 0 deletions
39
packages/cli/cli/src/__test__/checkOutputDirectoryCI.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| 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 } from "vitest"; | ||
| import { CliContext } from "../cli-context/CliContext"; | ||
| import { isCI } from "../utils/isCI"; | ||
|
|
||
| vi.mock("../utils/isCI", () => ({ | ||
| isCI: vi.fn().mockReturnValue(true) | ||
| })); | ||
|
|
||
| describe("checkOutputDirectory in CI", () => { | ||
| let mockCliContext: Partial<CliContext>; | ||
|
|
||
| beforeEach(() => { | ||
| mockCliContext = { | ||
| confirmPrompt: vi.fn() | ||
| }; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| 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 CliContext, false); | ||
|
|
||
| expect(result).toEqual({ | ||
| shouldProceed: true | ||
| }); | ||
| expect(mockCliContext.confirmPrompt).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
69 changes: 69 additions & 0 deletions
69
packages/cli/cli/src/commands/generate/checkOutputDirectory.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import { AbsoluteFilePath, doesPathExist } from "@fern-api/fs-utils"; | ||
| import { readdir } from "fs/promises"; | ||
| import { CliContext } from "../../cli-context/CliContext"; | ||
| import { getOutputDirectories } from "../../persistence/output-directories/getOutputDirectories"; | ||
| import { storeOutputDirectories } from "../../persistence/output-directories/storeOutputDirectories"; | ||
| import { isCI } from "../../utils/isCI"; | ||
|
|
||
| export interface CheckOutputDirectoryResult { | ||
| shouldProceed: boolean; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if an output directory is safe to write to and handles user confirmations | ||
| * @param outputPath The path to check | ||
| * @param cliContext The CLI context for prompting | ||
| * @returns Object containing whether to proceed and if directory was saved | ||
| */ | ||
| export async function checkOutputDirectory( | ||
| outputPath: AbsoluteFilePath | undefined, | ||
| cliContext: CliContext, | ||
| force: boolean | ||
| ): Promise<CheckOutputDirectoryResult> { | ||
| if (!outputPath || isCI() || force) { | ||
| return { | ||
| shouldProceed: true | ||
| }; | ||
| } | ||
|
|
||
| // First check if this is already a saved output directory | ||
| const savedDirectories = await getOutputDirectories(); | ||
| if (savedDirectories?.includes(outputPath)) { | ||
| return { | ||
| shouldProceed: true | ||
| }; | ||
| } | ||
|
|
||
| // Check if directory exists and has files | ||
| const doesExist = await doesPathExist(outputPath); | ||
| if (!doesExist) { | ||
| return { | ||
| shouldProceed: true | ||
| }; | ||
| } | ||
|
|
||
| const files = await readdir(outputPath); | ||
| if (files.length === 0) { | ||
| return { | ||
| shouldProceed: true | ||
| }; | ||
| } | ||
|
|
||
| // Prompt user for confirmation since directory has files | ||
| const shouldOverwrite = await cliContext.confirmPrompt( | ||
| `Directory ${outputPath} contains existing files that may be overwritten. Continue?`, | ||
| false | ||
| ); | ||
|
|
||
| if (!shouldOverwrite) { | ||
| return { | ||
| shouldProceed: false | ||
| }; | ||
| } | ||
|
|
||
| await storeOutputDirectories([...(savedDirectories ?? []), outputPath]); | ||
|
|
||
| return { | ||
| shouldProceed: true | ||
| }; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.