From 21b64060534504dfda9b11efc95037965547c805 Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Fri, 8 May 2026 00:37:21 +0000 Subject: [PATCH 1/2] [Tests] Replace mocks with behavior-based testing in fs.test.ts Refactored `packages/cli-kit/src/public/node/fs.test.ts` to remove brittle implementation-focused mocks and replace them with robust, behavior-based tests using real temporary directories. - Removed global mocks for `fast-glob`, `os`, and `array`. - Refactored `glob` tests to verify actual file discovery. - Refactored `generateRandomNameForSubdirectory` reroll logic tests using `vi.spyOn`. - Simplified `detectEOL` assertions. - Removed redundant `vi.doMock` for `fast-glob`. --- packages/cli-kit/src/public/node/fs.test.ts | 72 +++++++++++---------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/packages/cli-kit/src/public/node/fs.test.ts b/packages/cli-kit/src/public/node/fs.test.ts index a54b4d83f55..cea164f330c 100644 --- a/packages/cli-kit/src/public/node/fs.test.ts +++ b/packages/cli-kit/src/public/node/fs.test.ts @@ -24,19 +24,11 @@ import { fileRealPath, } from './fs.js' import {joinPath, normalizePath} from './path.js' -import {takeRandomFromArray} from '../common/array.js' -import {beforeEach, describe, expect, test, vi} from 'vitest' -import FastGlob from 'fast-glob' +import * as array from '../common/array.js' +import {describe, expect, test, vi} from 'vitest' import * as os from 'os' -vi.mock('../common/array.js') -vi.mock('fast-glob') -vi.mock('os', async (importOriginal) => { - const actual = await importOriginal() - return {...actual, EOL: actual.EOL} -}) - describe('inTemporaryDirectory', () => { test('ties the lifecycle of the temporary directory to the lifecycle of the callback', async () => { // Given @@ -223,10 +215,11 @@ describe('makeDirectoryWithRandomName', () => { test('rerolls the name if a directory exists with the same name', async () => { await inTemporaryDirectory(async (tmpDir) => { // Given - vi.mocked(takeRandomFromArray).mockReturnValueOnce('taken') - vi.mocked(takeRandomFromArray).mockReturnValueOnce('directory') - vi.mocked(takeRandomFromArray).mockReturnValueOnce('free') - vi.mocked(takeRandomFromArray).mockReturnValueOnce('directory') + const takeRandomFromArraySpy = vi.spyOn(array, 'takeRandomFromArray') + takeRandomFromArraySpy.mockReturnValueOnce('taken') + takeRandomFromArraySpy.mockReturnValueOnce('directory') + takeRandomFromArraySpy.mockReturnValueOnce('free') + takeRandomFromArraySpy.mockReturnValueOnce('directory') const content = 'test' const filePath = joinPath(tmpDir, 'taken-directory-app') @@ -237,7 +230,8 @@ describe('makeDirectoryWithRandomName', () => { // Then expect(got).toEqual('free-directory-app') - expect(takeRandomFromArray).toHaveBeenCalledTimes(4) + expect(takeRandomFromArraySpy).toHaveBeenCalledTimes(4) + takeRandomFromArraySpy.mockRestore() }) }) }) @@ -292,20 +286,38 @@ describe('createFileReadStream', () => { }) describe('glob', () => { - test('calls fastGlob with dot:true if no dot option is passed', async () => { - // When - await glob('pattern') + test('returns the files that match the pattern', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + const filePath = joinPath(tmpDir, 'test-file') + const dotFilePath = joinPath(tmpDir, '.dot-file') + await touchFile(filePath) + await touchFile(dotFilePath) - // Then - expect(FastGlob).toBeCalledWith('pattern', {dot: true}) + // When + const got = await glob(joinPath(tmpDir, '*')) + + // Then + expect(got.map((path) => normalizePath(path))).toContain(normalizePath(filePath)) + expect(got.map((path) => normalizePath(path))).toContain(normalizePath(dotFilePath)) + }) }) - test('calls fastGlob with dot option if passed', async () => { - // When - await glob('pattern', {dot: false}) + test('returns the files that match the pattern excluding dot files', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + const filePath = joinPath(tmpDir, 'test-file') + const dotFilePath = joinPath(tmpDir, '.dot-file') + await touchFile(filePath) + await touchFile(dotFilePath) - // Then - expect(FastGlob).toBeCalledWith('pattern', {dot: false}) + // When + const got = await glob(joinPath(tmpDir, '*'), {dot: false}) + + // Then + expect(got.map((path) => normalizePath(path))).toContain(normalizePath(filePath)) + expect(got.map((path) => normalizePath(path))).not.toContain(normalizePath(dotFilePath)) + }) }) }) @@ -335,24 +347,16 @@ describe('detectEOL', () => { test('returns the default EOL if no EOL is found', async () => { // Given const fileContent = 'testcontent' - vi.mocked(os).EOL = '\n' // When const eol = detectEOL(fileContent) // Then - expect(eol).toEqual('\n') + expect(eol).toEqual(os.EOL) }) }) describe('copyDirectoryContents', () => { - beforeEach(() => { - // restore fast-glob to its original implementation for the tests - vi.doMock('fast-glob', async () => { - return vi.importActual('fast-glob') - }) - }) - test('copies the contents of source directory to destination directory', async () => { // Given await inTemporaryDirectory(async (tmpDir) => { From 443aa2b2eb5bc5acc1bc4fc588fb209f53dfa368 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Fri, 8 May 2026 13:50:45 +0200 Subject: [PATCH 2/2] Decouple expectation from implementation --- packages/cli-kit/src/public/node/fs.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/cli-kit/src/public/node/fs.test.ts b/packages/cli-kit/src/public/node/fs.test.ts index cea164f330c..d30612b5b96 100644 --- a/packages/cli-kit/src/public/node/fs.test.ts +++ b/packages/cli-kit/src/public/node/fs.test.ts @@ -27,8 +27,6 @@ import {joinPath, normalizePath} from './path.js' import * as array from '../common/array.js' import {describe, expect, test, vi} from 'vitest' -import * as os from 'os' - describe('inTemporaryDirectory', () => { test('ties the lifecycle of the temporary directory to the lifecycle of the callback', async () => { // Given @@ -352,7 +350,7 @@ describe('detectEOL', () => { const eol = detectEOL(fileContent) // Then - expect(eol).toEqual(os.EOL) + expect(eol).toMatch(/^(\r\n|\n)$/) }) })