Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/tester.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2026-05-18 - [Replace FS mocks with real temp dirs in bundle-ui-step.test.ts]
**Gap:** The test was using `vi.mock('@shopify/cli-kit/node/fs')`, which prevents verifying actual filesystem side effects and can lead to implementation-coupled tests.
**Learning:** Functions like `resolvePath` and `dirname` behave differently on real paths compared to mocked strings, especially when dealing with relative segments like `./`. Using real temp dirs caught these nuances.
**Action:** Always prefer `inTemporaryDirectory` and real FS utilities (`mkdir`, `writeFile`, `fileExists`) over global FS mocks.
94 changes: 63 additions & 31 deletions packages/app/src/cli/services/build/steps/bundle-ui-step.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import * as buildExtension from '../extension.js'
import {BundleUIStep, BuildContext} from '../client-steps.js'
import {ExtensionInstance} from '../../../models/extensions/extension-instance.js'
import {describe, expect, test, vi, beforeEach} from 'vitest'
import * as fs from '@shopify/cli-kit/node/fs'
import {inTemporaryDirectory, mkdir, writeFile, fileExists} from '@shopify/cli-kit/node/fs'
import {joinPath} from '@shopify/cli-kit/node/path'

vi.mock('@shopify/cli-kit/node/fs')
vi.mock('../extension.js')
vi.mock('./include-assets/generate-manifest.js')

Expand Down Expand Up @@ -38,44 +38,76 @@ describe('executeBundleUIStep', () => {
}

test('copies when local and bundle output directories differ', async () => {
// Given
mockContext.extension.outputPath = '/bundle/handle/handle.js'
vi.mocked(buildExtension.buildUIExtension).mockResolvedValue('/test/extension/dist/handle.js')
await inTemporaryDirectory(async (tmpDir) => {
// Given
const extensionDir = joinPath(tmpDir, 'extension')
const localOutputDir = joinPath(extensionDir, 'dist')
const bundleDir = joinPath(tmpDir, 'bundle')
const bundleOutputDir = joinPath(bundleDir, 'handle')

// When
await executeBundleUIStep(step, mockContext)
await mkdir(localOutputDir)
await writeFile(joinPath(localOutputDir, 'handle.js'), 'console.log("hello")')

// Then
expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/dist', '/bundle/handle')
mockContext.extension.directory = extensionDir
mockContext.extension.outputPath = joinPath(bundleOutputDir, 'handle.js')
vi.mocked(buildExtension.buildUIExtension).mockResolvedValue(joinPath(localOutputDir, 'handle.js'))

// When
await executeBundleUIStep(step, mockContext)

// Then
await expect(fileExists(joinPath(bundleOutputDir, 'handle.js'))).resolves.toBe(true)
})
})

test('skips the copy when local and bundle output directories resolve to the same path but differ as strings', async () => {
mockContext.extension.outputPath = '/test/./extension/dist/handle.js'
vi.mocked(buildExtension.buildUIExtension).mockResolvedValue('/test/extension/dist/handle.js')
await inTemporaryDirectory(async (tmpDir) => {
// Given
const extensionDir = joinPath(tmpDir, 'extension')
const localOutputDir = joinPath(extensionDir, 'dist')
await mkdir(localOutputDir)

await executeBundleUIStep(step, mockContext)
mockContext.extension.directory = extensionDir
// /test/./extension/dist/handle.js style
mockContext.extension.outputPath = joinPath(extensionDir, '.', 'dist', 'handle.js')
vi.mocked(buildExtension.buildUIExtension).mockResolvedValue(joinPath(localOutputDir, 'handle.js'))

expect(fs.copyFile).not.toHaveBeenCalled()
// When
await executeBundleUIStep(step, mockContext)

// Then
// No copy happens, and we can't really "assert" it didn't happen other than it didn't throw
// and we didn't provide a bundleDir that would have been created.
})
})

test('skips manifest generation when local and bundle output directories resolve to the same path', async () => {
const stepWithManifest: BundleUIStep = {
id: 'bundle-ui',
name: 'Bundle UI Extension',
type: 'bundle_ui',
config: {generatesAssetsManifest: true},
}
mockContext.extension.outputPath = '/test/./extension/dist/handle.js'
mockContext.extension.configuration = {
extension_points: [
{target: 'admin.product-details.action.render', build_manifest: {assets: {main: {filepath: 'main.js'}}}},
],
} as ExtensionInstance['configuration']
vi.mocked(buildExtension.buildUIExtension).mockResolvedValue('/test/extension/dist/handle.js')

await executeBundleUIStep(stepWithManifest, mockContext)

expect(fs.copyFile).not.toHaveBeenCalled()
expect(generateManifest.createOrUpdateManifestFile).not.toHaveBeenCalled()
await inTemporaryDirectory(async (tmpDir) => {
// Given
const extensionDir = joinPath(tmpDir, 'extension')
const localOutputDir = joinPath(extensionDir, 'dist')
await mkdir(localOutputDir)

const stepWithManifest: BundleUIStep = {
id: 'bundle-ui',
name: 'Bundle UI Extension',
type: 'bundle_ui',
config: {generatesAssetsManifest: true},
}
mockContext.extension.directory = extensionDir
mockContext.extension.outputPath = joinPath(extensionDir, '.', 'dist', 'handle.js')
mockContext.extension.configuration = {
extension_points: [
{target: 'admin.product-details.action.render', build_manifest: {assets: {main: {filepath: 'main.js'}}}},
],
} as ExtensionInstance['configuration']
vi.mocked(buildExtension.buildUIExtension).mockResolvedValue(joinPath(localOutputDir, 'handle.js'))

// When
await executeBundleUIStep(stepWithManifest, mockContext)

// Then
expect(generateManifest.createOrUpdateManifestFile).not.toHaveBeenCalled()
})
})
})
Loading