From e8559c4577a84f652392c80bba1815fcb57d70f7 Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Sat, 16 May 2026 00:25:59 +0000 Subject: [PATCH] [Tests] Remove filesystem mocks in copy-source-entry.test.ts Refactor copy-source-entry.test.ts to use real filesystem operations via inTemporaryDirectory instead of mocking @shopify/cli-kit/node/fs. This improves test fidelity and reliability. --- .jules/tester.md | 0 .../include-assets/copy-source-entry.test.ts | 310 ++++++++++-------- 2 files changed, 182 insertions(+), 128 deletions(-) create mode 100644 .jules/tester.md diff --git a/.jules/tester.md b/.jules/tester.md new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.test.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.test.ts index 5c1436fc2d2..72c726d6963 100644 --- a/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.test.ts +++ b/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.test.ts @@ -1,8 +1,7 @@ import {copySourceEntry} from './copy-source-entry.js' import {describe, expect, test, vi, beforeEach} from 'vitest' -import * as fs from '@shopify/cli-kit/node/fs' - -vi.mock('@shopify/cli-kit/node/fs') +import {inTemporaryDirectory, writeFile, mkdir, fileExistsSync, readFile} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' describe('copySourceEntry', () => { let mockStdout: any @@ -12,149 +11,204 @@ describe('copySourceEntry', () => { }) test('throws when source path does not exist', async () => { - // Given - vi.mocked(fs.fileExists).mockResolvedValue(false) + await inTemporaryDirectory(async (tmpDir) => { + // Given + const baseDir = joinPath(tmpDir, 'ext') + const outputDir = joinPath(tmpDir, 'out') + const appDirectory = tmpDir + await mkdir(baseDir) + await mkdir(outputDir) + + // When / Then + await expect( + copySourceEntry( + { + source: 'missing/file.js', + destination: undefined, + baseDir, + outputDir, + appDirectory, + }, + {stdout: mockStdout}, + ), + ).rejects.toThrow(`Source does not exist: ${joinPath(baseDir, 'missing/file.js')}`) + }) + }) - // When / Then - await expect( - copySourceEntry( + test('copies file to explicit destination path', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + const baseDir = joinPath(tmpDir, 'ext') + const outputDir = joinPath(tmpDir, 'out') + const appDirectory = tmpDir + await mkdir(baseDir) + await mkdir(outputDir) + const sourceFile = joinPath(baseDir, 'src/icon.png') + await mkdir(joinPath(baseDir, 'src')) + await writeFile(sourceFile, 'icon-content') + + // When + const result = await copySourceEntry( { - source: 'missing/file.js', - destination: undefined, - baseDir: '/ext', - outputDir: '/out', - appDirectory: '/ext', + source: 'src/icon.png', + destination: 'assets/icon.png', + baseDir, + outputDir, + appDirectory, }, {stdout: mockStdout}, - ), - ).rejects.toThrow('Source does not exist: /ext/missing/file.js') - }) - - test('copies file to explicit destination path', async () => { - // Given - vi.mocked(fs.fileExists).mockResolvedValue(true) - vi.mocked(fs.isDirectory).mockResolvedValue(false) - vi.mocked(fs.mkdir).mockResolvedValue() - vi.mocked(fs.copyFile).mockResolvedValue() - - // When - const result = await copySourceEntry( - { - source: 'src/icon.png', - destination: 'assets/icon.png', - baseDir: '/ext', - outputDir: '/out', - appDirectory: '/ext', - }, - {stdout: mockStdout}, - ) - - // Then - expect(fs.copyFile).toHaveBeenCalledWith('/ext/src/icon.png', '/out/assets/icon.png') - expect(result.filesCopied).toBe(1) - expect(result.outputPaths).toEqual(['assets/icon.png']) - expect(mockStdout.write).toHaveBeenCalledWith('Included src/icon.png\n') + ) + + // Then + const expectedDest = joinPath(outputDir, 'assets/icon.png') + expect(fileExistsSync(expectedDest)).toBe(true) + await expect(readFile(expectedDest)).resolves.toBe('icon-content') + expect(result.filesCopied).toBe(1) + expect(result.outputPaths).toEqual(['assets/icon.png']) + expect(mockStdout.write).toHaveBeenCalledWith('Included src/icon.png\n') + }) }) test('copies directory under its own name when no destination is given', async () => { - // Given - vi.mocked(fs.fileExists).mockResolvedValue(true) - vi.mocked(fs.isDirectory).mockResolvedValue(true) - vi.mocked(fs.copyDirectoryContents).mockResolvedValue() - vi.mocked(fs.glob).mockResolvedValue(['index.html', 'logo.png']) - - // When - const result = await copySourceEntry( - {source: 'dist', destination: undefined, baseDir: '/ext', outputDir: '/out', appDirectory: '/ext'}, - {stdout: mockStdout}, - ) - - // Then - expect(fs.copyDirectoryContents).toHaveBeenCalledWith('/ext/dist', '/out/dist') - expect(result.filesCopied).toBe(2) - expect(result.outputPaths).toEqual(['dist/index.html', 'dist/logo.png']) - expect(mockStdout.write).toHaveBeenCalledWith('Included dist\n') + await inTemporaryDirectory(async (tmpDir) => { + // Given + const baseDir = joinPath(tmpDir, 'ext') + const outputDir = joinPath(tmpDir, 'out') + const appDirectory = tmpDir + await mkdir(baseDir) + await mkdir(outputDir) + + const distDir = joinPath(baseDir, 'dist') + await mkdir(distDir) + await writeFile(joinPath(distDir, 'index.html'), 'html') + await writeFile(joinPath(distDir, 'logo.png'), 'logo') + + // When + const result = await copySourceEntry( + {source: 'dist', destination: undefined, baseDir, outputDir, appDirectory}, + {stdout: mockStdout}, + ) + + // Then + expect(fileExistsSync(joinPath(outputDir, 'dist/index.html'))).toBe(true) + expect(fileExistsSync(joinPath(outputDir, 'dist/logo.png'))).toBe(true) + expect(result.filesCopied).toBe(2) + expect(result.outputPaths).toEqual(expect.arrayContaining(['dist/index.html', 'dist/logo.png'])) + expect(mockStdout.write).toHaveBeenCalledWith('Included dist\n') + }) }) test('copies file to basename in outputDir when source is a file and no destination given', async () => { - // Given - vi.mocked(fs.fileExists).mockResolvedValue(true) - vi.mocked(fs.isDirectory).mockResolvedValue(false) - vi.mocked(fs.mkdir).mockResolvedValue() - vi.mocked(fs.copyFile).mockResolvedValue() - - // When - const result = await copySourceEntry( - {source: 'README.md', destination: undefined, baseDir: '/ext', outputDir: '/out', appDirectory: '/ext'}, - {stdout: mockStdout}, - ) - - // Then - expect(fs.copyFile).toHaveBeenCalledWith('/ext/README.md', '/out/README.md') - expect(result.filesCopied).toBe(1) - expect(result.outputPaths).toEqual(['README.md']) - expect(mockStdout.write).toHaveBeenCalledWith('Included README.md\n') + await inTemporaryDirectory(async (tmpDir) => { + // Given + const baseDir = joinPath(tmpDir, 'ext') + const outputDir = joinPath(tmpDir, 'out') + const appDirectory = tmpDir + await mkdir(baseDir) + await mkdir(outputDir) + await writeFile(joinPath(baseDir, 'README.md'), 'readme') + + // When + const result = await copySourceEntry( + {source: 'README.md', destination: undefined, baseDir, outputDir, appDirectory}, + {stdout: mockStdout}, + ) + + // Then + const expectedDest = joinPath(outputDir, 'README.md') + expect(fileExistsSync(expectedDest)).toBe(true) + await expect(readFile(expectedDest)).resolves.toBe('readme') + expect(result.filesCopied).toBe(1) + expect(result.outputPaths).toEqual(['README.md']) + expect(mockStdout.write).toHaveBeenCalledWith('Included README.md\n') + }) }) test('copies directory to explicit destination path', async () => { - // Given - vi.mocked(fs.fileExists).mockResolvedValue(true) - vi.mocked(fs.isDirectory).mockResolvedValue(true) - vi.mocked(fs.copyDirectoryContents).mockResolvedValue() - vi.mocked(fs.glob).mockResolvedValue(['x.js']) - - // When - const result = await copySourceEntry( - {source: 'dist', destination: 'vendor/dist', baseDir: '/ext', outputDir: '/out', appDirectory: '/ext'}, - {stdout: mockStdout}, - ) - - // Then - expect(fs.copyDirectoryContents).toHaveBeenCalledWith('/ext/dist', '/out/vendor/dist') - expect(result.filesCopied).toBe(1) - expect(result.outputPaths).toEqual(['vendor/dist/x.js']) - expect(mockStdout.write).toHaveBeenCalledWith('Included dist\n') + await inTemporaryDirectory(async (tmpDir) => { + // Given + const baseDir = joinPath(tmpDir, 'ext') + const outputDir = joinPath(tmpDir, 'out') + const appDirectory = tmpDir + await mkdir(baseDir) + await mkdir(outputDir) + + const distDir = joinPath(baseDir, 'dist') + await mkdir(distDir) + await writeFile(joinPath(distDir, 'x.js'), 'js') + + // When + const result = await copySourceEntry( + {source: 'dist', destination: 'vendor/dist', baseDir, outputDir, appDirectory}, + {stdout: mockStdout}, + ) + + // Then + expect(fileExistsSync(joinPath(outputDir, 'vendor/dist/x.js'))).toBe(true) + expect(result.filesCopied).toBe(1) + expect(result.outputPaths).toEqual(['vendor/dist/x.js']) + expect(mockStdout.write).toHaveBeenCalledWith('Included dist\n') + }) }) test('returns count of files discovered in destination directory after directory copy', async () => { - // Given - vi.mocked(fs.fileExists).mockResolvedValue(true) - vi.mocked(fs.isDirectory).mockResolvedValue(true) - vi.mocked(fs.copyDirectoryContents).mockResolvedValue() - // Simulate 5 files inside the copied directory - vi.mocked(fs.glob).mockResolvedValue(['a.js', 'b.js', 'c.js', 'd.js', 'e.js']) - - // When - const result = await copySourceEntry( - {source: 'theme', destination: undefined, baseDir: '/ext', outputDir: '/out', appDirectory: '/ext'}, - {stdout: mockStdout}, - ) - - // Then — count comes from glob on destPath, not a constant - expect(result.filesCopied).toBe(5) - expect(result.outputPaths).toEqual(['theme/a.js', 'theme/b.js', 'theme/c.js', 'theme/d.js', 'theme/e.js']) + await inTemporaryDirectory(async (tmpDir) => { + // Given + const baseDir = joinPath(tmpDir, 'ext') + const outputDir = joinPath(tmpDir, 'out') + const appDirectory = tmpDir + await mkdir(baseDir) + await mkdir(outputDir) + + const themeDir = joinPath(baseDir, 'theme') + await mkdir(themeDir) + await writeFile(joinPath(themeDir, 'a.js'), 'a') + await writeFile(joinPath(themeDir, 'b.js'), 'b') + await writeFile(joinPath(themeDir, 'c.js'), 'c') + await writeFile(joinPath(themeDir, 'd.js'), 'd') + await writeFile(joinPath(themeDir, 'e.js'), 'e') + + // When + const result = await copySourceEntry( + {source: 'theme', destination: undefined, baseDir, outputDir, appDirectory}, + {stdout: mockStdout}, + ) + + // Then + expect(result.filesCopied).toBe(5) + expect(result.outputPaths).toEqual( + expect.arrayContaining(['theme/a.js', 'theme/b.js', 'theme/c.js', 'theme/d.js', 'theme/e.js']), + ) + }) }) test('creates parent directories before copying a file', async () => { - // Given - vi.mocked(fs.fileExists).mockResolvedValue(true) - vi.mocked(fs.isDirectory).mockResolvedValue(false) - vi.mocked(fs.mkdir).mockResolvedValue() - vi.mocked(fs.copyFile).mockResolvedValue() - - // When - await copySourceEntry( - { - source: 'src/deep/icon.png', - destination: 'assets/icons/icon.png', - baseDir: '/ext', - outputDir: '/out', - appDirectory: '/ext', - }, - {stdout: mockStdout}, - ) - - // Then — parent of destination path created - expect(fs.mkdir).toHaveBeenCalledWith('/out/assets/icons') + await inTemporaryDirectory(async (tmpDir) => { + // Given + const baseDir = joinPath(tmpDir, 'ext') + const outputDir = joinPath(tmpDir, 'out') + const appDirectory = tmpDir + await mkdir(baseDir) + await mkdir(outputDir) + + const deepDir = joinPath(baseDir, 'src/deep') + await mkdir(deepDir) + await writeFile(joinPath(deepDir, 'icon.png'), 'icon') + + // When + await copySourceEntry( + { + source: 'src/deep/icon.png', + destination: 'assets/icons/icon.png', + baseDir, + outputDir, + appDirectory, + }, + {stdout: mockStdout}, + ) + + // Then — parent of destination path created + expect(fileExistsSync(joinPath(outputDir, 'assets/icons/icon.png'))).toBe(true) + }) }) })