From 2da3adb7fc2749a8d7143119bd1dbe46ed9e5638 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Sat, 10 Oct 2026 21:21:33 -0400 Subject: [PATCH 1/4] Add off-by-default filesystem automock rule --- packages/eslint-plugin-cli/config.js | 1 + packages/eslint-plugin-cli/index.js | 1 + .../rules/no-filesystem-auto-mocks.js | 56 +++++++++++++++++ .../rules/no-filesystem-auto-mocks.test.js | 61 +++++++++++++++++++ 4 files changed, 119 insertions(+) create mode 100644 packages/eslint-plugin-cli/rules/no-filesystem-auto-mocks.js create mode 100644 packages/eslint-plugin-cli/rules/no-filesystem-auto-mocks.test.js diff --git a/packages/eslint-plugin-cli/config.js b/packages/eslint-plugin-cli/config.js index d164966cf68..1e728b358c4 100644 --- a/packages/eslint-plugin-cli/config.js +++ b/packages/eslint-plugin-cli/config.js @@ -12,6 +12,7 @@ const rules = { 'command-conventional-flag-env': require('./rules/command-conventional-flag-env'), 'command-reserved-flags': require('./rules/command-reserved-flags'), 'command-json-output': require('./rules/command-json-output'), + 'no-filesystem-auto-mocks': require('./rules/no-filesystem-auto-mocks'), 'no-error-factory-functions': require('./rules/no-error-factory-functions'), 'no-process-cwd': require('./rules/no-process-cwd'), 'no-trailing-js-in-cli-kit-imports': require('./rules/no-trailing-js-in-cli-kit-imports'), diff --git a/packages/eslint-plugin-cli/index.js b/packages/eslint-plugin-cli/index.js index e0440d0d368..9e1596e3132 100644 --- a/packages/eslint-plugin-cli/index.js +++ b/packages/eslint-plugin-cli/index.js @@ -8,6 +8,7 @@ const plugin = { 'command-conventional-flag-env': require('./rules/command-conventional-flag-env'), 'command-reserved-flags': require('./rules/command-reserved-flags'), 'command-json-output': require('./rules/command-json-output'), + 'no-filesystem-auto-mocks': require('./rules/no-filesystem-auto-mocks'), 'no-error-factory-functions': require('./rules/no-error-factory-functions'), 'no-process-cwd': require('./rules/no-process-cwd'), 'no-trailing-js-in-cli-kit-imports': require('./rules/no-trailing-js-in-cli-kit-imports'), diff --git a/packages/eslint-plugin-cli/rules/no-filesystem-auto-mocks.js b/packages/eslint-plugin-cli/rules/no-filesystem-auto-mocks.js new file mode 100644 index 00000000000..3d0c9470edb --- /dev/null +++ b/packages/eslint-plugin-cli/rules/no-filesystem-auto-mocks.js @@ -0,0 +1,56 @@ +const path = require('node:path') + +const filesystemModules = new Set(['fs', 'node:fs', 'fs/promises', 'node:fs/promises', '@shopify/cli-kit/node/fs']) + +module.exports = { + meta: { + type: 'problem', + docs: {description: 'use real filesystem fixtures instead of automatic filesystem mocks'}, + schema: [], + messages: {filesystemMock: 'Use real files in a temporary directory instead of an automatic filesystem mock.'}, + }, + create(context) { + if (!/\.test\.[jt]sx?$/.test(context.filename)) return {} + + return { + CallExpression(node) { + if (node.callee.type !== 'MemberExpression' || node.callee.computed) return + if (node.callee.object.name !== 'vi' || !['mock', 'doMock'].includes(node.callee.property.name)) return + if (node.arguments.length !== 1) return + + const argument = node.arguments[0] + let moduleName + if (argument.type === 'Literal') moduleName = argument.value + else if (argument.type === 'ImportExpression') moduleName = argument.source.value + if (typeof moduleName !== 'string') return + const isFilesystemModule = moduleName.startsWith('.') + ? /\/packages\/cli-kit\/src\/public\/node\/fs\.[jt]s$/.test( + path.posix.resolve(path.posix.dirname(context.filename.replaceAll('\\', '/')), moduleName), + ) + : filesystemModules.has(moduleName.replace(/\.js$/, '')) + if (!isFilesystemModule) return + const variable = findVariable(context.sourceCode.getScope(node), 'vi') + if ( + variable && + !variable.defs.some( + (definition) => + definition.type === 'ImportBinding' && + definition.parent.source.value === 'vitest' && + definition.node.imported?.name === 'vi', + ) + ) + return + + context.report({node, messageId: 'filesystemMock'}) + }, + } + }, +} + +function findVariable(scope, name) { + for (let current = scope; current; current = current.upper) { + const variable = current.set.get(name) + if (variable) return variable + } + return undefined +} diff --git a/packages/eslint-plugin-cli/rules/no-filesystem-auto-mocks.test.js b/packages/eslint-plugin-cli/rules/no-filesystem-auto-mocks.test.js new file mode 100644 index 00000000000..d39ee17e92c --- /dev/null +++ b/packages/eslint-plugin-cli/rules/no-filesystem-auto-mocks.test.js @@ -0,0 +1,61 @@ +const {RuleTester} = require('eslint') +const typescriptParser = require('@typescript-eslint/parser') + +const rule = require('./no-filesystem-auto-mocks') + +const ruleTester = new RuleTester({languageOptions: {parser: typescriptParser}}) +const filename = '/repo/services/widgets.test.ts' + +ruleTester.run('no-filesystem-auto-mocks', rule, { + valid: [ + {filename, code: 'vi.mock("./service")'}, + {filename, code: 'vi.mock("../fs.js")'}, + {filename, code: 'import {vi as v} from "vitest"; v.mock("fs")'}, + {filename, code: 'import * as vitest from "vitest"; vitest.vi.mock("fs")'}, + {filename, code: 'vi.mock(`fs`)'}, + {filename: '/repo/services/widgets.spec.ts', code: 'vi.mock("fs")'}, + {filename, code: 'vi.mock("fs", async (importOriginal) => ({...await importOriginal()}))'}, + {filename: '/repo/packages/cli-kit/src/public/node/context/local.test.ts', code: 'vi.mock("../fs.js", () => ({}))'}, + {filename: '/repo/packages/cli-kit/src/public/node/context/local.test.ts', code: 'vi.mock("./fs.js")'}, + { + filename, + code: 'vi.mock("@shopify/cli-kit/node/fs", () => ({generateRandomNameForSubdirectory: () => "fixture"}))', + }, + {filename, code: 'const vi = {mock() {}}; vi.mock("node:fs")'}, + {filename, code: 'function check(vi) { vi.mock("node:fs") }'}, + {filename, code: 'import {vi} from "another-library"; vi.mock("node:fs")'}, + {filename, code: 'vi.mock(moduleName)'}, + {filename: '/repo/services/widgets.ts', code: 'vi.mock("node:fs")'}, + ], + invalid: [ + {filename, code: 'vi.mock("node:fs")', errors: [{messageId: 'filesystemMock'}]}, + {filename, code: 'vi.mock("@shopify/cli-kit/node/fs.js")', errors: [{messageId: 'filesystemMock'}]}, + {filename, code: 'vi.mock("fs/promises.js")', errors: [{messageId: 'filesystemMock'}]}, + { + filename: '/repo/packages/cli-kit/src/public/node/context/local.test.ts', + code: 'vi.mock("../fs.js")', + errors: [{messageId: 'filesystemMock'}], + }, + { + filename: '/repo/packages/cli-kit/src/public/node/api/bulk-operations/query.test.ts', + code: 'vi.mock("../../fs.js")', + errors: [{messageId: 'filesystemMock'}], + }, + { + filename: '/repo/packages/cli-kit/src/public/node/doctor/framework.test.ts', + code: 'vi.doMock(import("../fs.ts"))', + errors: [{messageId: 'filesystemMock'}], + }, + { + filename: String.raw`C:\repo\packages\cli-kit\src\public\node\doctor\framework.test.ts`, + code: 'vi.mock("../fs.js")', + errors: [{messageId: 'filesystemMock'}], + }, + {filename, code: 'vi.doMock("node:fs/promises")', errors: [{messageId: 'filesystemMock'}]}, + {filename, code: 'vi.mock("fs")', errors: [{messageId: 'filesystemMock'}]}, + {filename, code: 'vi.mock("fs/promises")', errors: [{messageId: 'filesystemMock'}]}, + {filename, code: 'vi.mock("@shopify/cli-kit/node/fs")', errors: [{messageId: 'filesystemMock'}]}, + {filename, code: 'import {vi} from "vitest"; vi.mock("fs")', errors: [{messageId: 'filesystemMock'}]}, + {filename, code: 'vi.mock(import("node:fs"))', errors: [{messageId: 'filesystemMock'}]}, + ], +}) From fbc8af190c3224b2a9870f7974ae4775dde9f9c9 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Sun, 11 Oct 2026 10:22:41 -0400 Subject: [PATCH 2/4] Enable filesystem automock lint rule and use real test files --- docs/cli/testing-strategy.md | 2 + .../dev/processes/app-logs-polling.test.ts | 11 +- .../dev-session/dev-session-process.test.ts | 161 ++++++++++-------- .../node/api/bulk-operations/query.test.ts | 34 ++-- .../src/public/node/context/local.test.ts | 20 ++- .../src/public/node/doctor/framework.test.ts | 1 - .../src/cli/commands/docs/generate.test.ts | 24 ++- packages/eslint-plugin-cli/config.js | 1 + .../src/cli/utilities/theme-command.test.ts | 44 +++-- 9 files changed, 169 insertions(+), 129 deletions(-) diff --git a/docs/cli/testing-strategy.md b/docs/cli/testing-strategy.md index a8c4ed6d1c6..3c548a88556 100644 --- a/docs/cli/testing-strategy.md +++ b/docs/cli/testing-strategy.md @@ -57,6 +57,8 @@ See [JSON output tests](json-output.md#test-a-new-command) and [UI tests](../cli ### Filesystem I/O and temporary directories +The `@shopify/cli/no-filesystem-auto-mocks` ESLint rule rejects automatic filesystem mocks in test files. + For filesystem tests, use real files in a temporary directory. The test must delete the directory after use, including when it fails: ```ts diff --git a/packages/app/src/cli/services/dev/processes/app-logs-polling.test.ts b/packages/app/src/cli/services/dev/processes/app-logs-polling.test.ts index 86c00b2955a..fbe65092c3f 100644 --- a/packages/app/src/cli/services/dev/processes/app-logs-polling.test.ts +++ b/packages/app/src/cli/services/dev/processes/app-logs-polling.test.ts @@ -9,11 +9,11 @@ import {DeveloperPlatformClient} from '../../../utilities/developer-platform-cli import * as appLogsUtils from '../../app-logs/utils.js' import {AppEventWatcher} from '../app-events/app-event-watcher.js' import {AbortSignal, AbortController} from '@shopify/cli-kit/node/abort' -import {mkdir} from '@shopify/cli-kit/node/fs' +import {inTemporaryDirectory, mkdir} from '@shopify/cli-kit/node/fs' +import * as fs from '@shopify/cli-kit/node/fs' import {outputDebug} from '@shopify/cli-kit/node/output' import {describe, expect, vi, Mock, beforeEach, test} from 'vitest' -vi.mock('@shopify/cli-kit/node/fs') vi.mock('@shopify/cli-kit/node/output') vi.mock('../../app-logs/dev/poll-app-logs.js') @@ -102,7 +102,7 @@ describe('app-logs-polling', () => { developerPlatformClient = testDeveloperPlatformClient({subscribeToAppLogs}) - vi.mocked(mkdir).mockResolvedValue() + vi.spyOn(fs, 'mkdir') vi.mocked(pollAppLogs).mockResolvedValue() vi.spyOn(appLogsUtils, 'subscribeToAppLogs').mockResolvedValue(JWT_TOKEN) }) @@ -134,7 +134,10 @@ describe('app-logs-polling', () => { startTime: [0, 0], path: '', } - await startCallback(appEvent) + await inTemporaryDirectory(async (tempDir) => { + vi.spyOn(localApp, 'getLogsDir').mockReturnValue(tempDir) + await startCallback(appEvent) + }) // Then expect(outputDebug).toHaveBeenCalledWith('Function extensions detected, starting logs polling') diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts index 70ee061db60..04dde1248ab 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts @@ -14,14 +14,15 @@ import { } from '../../../../models/app/app.test-data.js' import {getUploadURL, writeManifestToBundle} from '../../../bundle.js' import {formData} from '@shopify/cli-kit/node/http' -import {describe, expect, test, vi, beforeEach, afterEach} from 'vitest' +import {describe, expect, vi, beforeEach, afterEach} from 'vitest' +import {testWithTempDir as test} from '@shopify/cli-kit/node/testing/test-with-temp-dir' import {AbortController} from '@shopify/cli-kit/node/abort' import {flushPromises} from '@shopify/cli-kit/node/promises' import * as outputContext from '@shopify/cli-kit/node/ui/components' -import {readdir} from '@shopify/cli-kit/node/fs' +import {mkdir} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' import {SerialBatchProcessor} from '@shopify/cli-kit/node/serial-batch-processor' -vi.mock('@shopify/cli-kit/node/fs') vi.mock('@shopify/cli-kit/node/archiver') vi.mock('@shopify/cli-kit/node/http') vi.mock('../../../../utilities/app/app-url.js') @@ -76,14 +77,42 @@ describe('pushUpdatesForDevSession', () => { let app: AppLinkedInterface let abortController: AbortController let devSessionStatusManager: DevSessionStatusManager + let startPromise: Promise | undefined + let processingPromises: Promise[] + const originalEnqueue = SerialBatchProcessor.prototype.enqueue - beforeEach(() => { + const waitForDevSession = async () => { + await flushPromises() + await startPromise + await Promise.all(processingPromises) + } + + beforeEach(({tempDir}) => { vi.mocked(formData).mockReturnValue({append: vi.fn(), getHeaders: vi.fn()} as any) stdout = {write: vi.fn()} stderr = {write: vi.fn()} developerPlatformClient = testDeveloperPlatformClient() - app = testAppLinked() + app = testAppLinked({directory: tempDir}) appWatcher = new AppEventWatcher(app) + startPromise = undefined + processingPromises = [] + const originalOnStart = appWatcher.onStart + vi.spyOn(appWatcher, 'onStart').mockImplementation((listener) => + originalOnStart.call(appWatcher, (event) => { + startPromise = Promise.resolve(listener(event)) + return startPromise + }), + ) + vi.spyOn(SerialBatchProcessor.prototype, 'enqueue').mockImplementation(function ( + this: SerialBatchProcessor, + item: unknown, + ) { + originalEnqueue.call(this, item) + const completion = this.waitForCompletion() + // Keep the rejection available for assertions without an unhandled rejection. + completion.catch(() => {}) + processingPromises.push(completion) + }) abortController = new AbortController() devSessionStatusManager = new DevSessionStatusManager() options = { @@ -108,7 +137,7 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() // Then expect(developerPlatformClient.devSessionCreate).toHaveBeenCalled() @@ -120,9 +149,9 @@ describe('pushUpdatesForDevSession', () => { await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: await testWebhookExtensions()}]}) - await flushPromises() + await waitForDevSession() expect(developerPlatformClient.devSessionCreate).toHaveBeenCalledWith( expect.objectContaining({unsafeValidation: true}), @@ -138,11 +167,11 @@ describe('pushUpdatesForDevSession', () => { const spyContext = vi.spyOn(outputContext, 'useConcurrentOutputContext') await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() const extension = await testUIExtension() appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension}]}) - await flushPromises() + await waitForDevSession() // Then expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Updated dev preview on test.myshopify.com')) @@ -161,7 +190,7 @@ describe('pushUpdatesForDevSession', () => { // When await appWatcher.start({stdout, stderr, signal: abortController.signal}) await pushUpdatesForDevSession({stderr, stdout, abortSignal: new AbortController().signal}, options) - await flushPromises() + await waitForDevSession() // Then expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Error')) @@ -172,7 +201,7 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: await testWebhookExtensions()}]}) - await flushPromises() + await waitForDevSession() // Then expect(stdout.write).toHaveBeenCalledWith( @@ -190,9 +219,9 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: await testWebhookExtensions()}]}) - await flushPromises() + await waitForDevSession() // Then expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Error')) @@ -220,9 +249,9 @@ describe('pushUpdatesForDevSession', () => { await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: await testWebhookExtensions()}]}) - await flushPromises() + await waitForDevSession() expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining(renderedMessage)) expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Updated dev preview on test.myshopify.com')) @@ -240,9 +269,9 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: await testWebhookExtensions()}]}) - await flushPromises() + await waitForDevSession() // Then expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Validation warning')) @@ -258,9 +287,9 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('all', event) - await flushPromises() + await waitForDevSession() // Then expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Updated dev preview on test.myshopify.com')) @@ -280,9 +309,9 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('all', {app: newApp, extensionEvents: [{type: 'updated', extension}]}) - await flushPromises() + await waitForDevSession() // Then expect(devSessionStatusManager.status.previewURL).toBe(options.appPreviewURL) @@ -296,9 +325,9 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('all', {app: newApp, extensionEvents: [{type: 'updated', extension}]}) - await flushPromises() + await waitForDevSession() // Then expect(devSessionStatusManager.status.previewURL).toBe(options.appPreviewURL) @@ -312,9 +341,9 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('all', {app: newApp, extensionEvents: [{type: 'updated', extension}]}) - await flushPromises() + await waitForDevSession() // Then expect(devSessionStatusManager.status.isReady).toBe(true) @@ -335,7 +364,7 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) appWatcher.emit('error', testError) - await flushPromises() + await waitForDevSession() // Then expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Error')) @@ -358,7 +387,7 @@ describe('pushUpdatesForDevSession', () => { // When - Start the session await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() // Then - Ready state expect(statusSpy).toHaveBeenCalledWith('READY') @@ -366,7 +395,7 @@ describe('pushUpdatesForDevSession', () => { // When - Emit an update event const extension = await testUIExtension() appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension}]}) - await flushPromises() + await waitForDevSession() // Then - Loading state during update expect(statusSpy).toHaveBeenCalledWith('CHANGE_DETECTED') @@ -392,9 +421,9 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('all', errorEvent) - await flushPromises() + await waitForDevSession() // Then expect(devSessionStatusManager.status.statusMessage).toEqual({ @@ -411,9 +440,9 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: await testWebhookExtensions()}]}) - await flushPromises() + await waitForDevSession() // Then expect(devSessionStatusManager.status.statusMessage).toEqual({ @@ -430,9 +459,9 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('error', validationError) - await flushPromises() + await waitForDevSession() // Then expect(devSessionStatusManager.status.statusMessage).toEqual({ @@ -443,7 +472,6 @@ describe('pushUpdatesForDevSession', () => { test('manifest sent in update payload only includes affected extensions', async () => { // Given - vi.mocked(readdir).mockResolvedValue(['assets', 'assets/updated-extension']) vi.mocked(getUploadURL).mockResolvedValue('https://gcs.url') const updatedExtension = await testUIExtension() @@ -456,12 +484,13 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await mkdir(joinPath(appWatcher.buildOutputPath, 'assets', 'updated-extension')) + await waitForDevSession() appWatcher.emit('all', { app: appWithMultipleExtensions, extensionEvents: [{type: 'updated', extension: updatedExtension}], }) - await flushPromises() + await waitForDevSession() // Then expect(developerPlatformClient.devSessionUpdate).toHaveBeenCalledWith({ @@ -492,7 +521,6 @@ describe('pushUpdatesForDevSession', () => { test('writes full manifest to bundle on session update, not just create', async () => { // Given - vi.mocked(readdir).mockResolvedValue([]) vi.mocked(getUploadURL).mockResolvedValue('https://gcs.url') const updatedExtension = await testUIExtension({handle: 'updated-ext', uid: 'updated-uid'}) @@ -506,7 +534,7 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() // Capture manifests at call time (the object is mutated after writeManifestToBundle) const capturedManifests: any[] = [] @@ -519,7 +547,7 @@ describe('pushUpdatesForDevSession', () => { app: appWithMultipleExtensions, extensionEvents: [{type: 'updated', extension: updatedExtension}], }) - await flushPromises() + await waitForDevSession() // Then - writeManifestToBundle should have been called with ALL modules, not just the updated one expect(capturedManifests).toHaveLength(1) @@ -531,7 +559,6 @@ describe('pushUpdatesForDevSession', () => { test('writes full manifest including new extension on created event', async () => { // Given - vi.mocked(readdir).mockResolvedValue([]) vi.mocked(getUploadURL).mockResolvedValue('https://gcs.url') const existingExtension = await testUIExtension({handle: 'existing-ext', uid: 'existing-uid'}) @@ -547,7 +574,7 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() // Capture manifests at call time (the object is mutated after writeManifestToBundle) const capturedManifests: any[] = [] @@ -560,7 +587,7 @@ describe('pushUpdatesForDevSession', () => { app: appAfterReload, extensionEvents: [{type: 'created', extension: newExtension}], }) - await flushPromises() + await waitForDevSession() // Then - writeManifestToBundle should include ALL modules (existing + new) expect(capturedManifests).toHaveLength(1) @@ -573,8 +600,6 @@ describe('pushUpdatesForDevSession', () => { test('assetsURL is only generated if affected extensions have assets', async () => { // Given vi.mocked(formData).mockReturnValue({append: vi.fn(), getHeaders: vi.fn()} as any) - // Mock readdir to return that a folder for the extension assets exists - vi.mocked(readdir).mockResolvedValue(['other-folders', 'ui-extension-uid']) vi.mocked(getUploadURL).mockResolvedValue('https://gcs.url') const extensionWithAssets = await testUIExtension({handle: 'ui-extension-handle', uid: 'ui-extension-uid'}) @@ -584,12 +609,14 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await mkdir(joinPath(appWatcher.buildOutputPath, 'other-folders')) + await mkdir(joinPath(appWatcher.buildOutputPath, 'ui-extension-uid')) + await waitForDevSession() appWatcher.emit('all', { app, extensionEvents: [{type: 'updated', extension: extensionWithAssets}], }) - await flushPromises() + await waitForDevSession() // Then expect(developerPlatformClient.devSessionUpdate).toHaveBeenCalledWith({ @@ -610,7 +637,7 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() // Then expect(developerPlatformClient.devSessionCreate).toHaveBeenCalledWith({ @@ -624,7 +651,6 @@ describe('pushUpdatesForDevSession', () => { test('multiple updates to different extensions are consolidated if a request is in progress', async () => { vi.mocked(getUploadURL).mockResolvedValue('https://gcs.url') - vi.mocked(readdir).mockResolvedValue([]) const extension1 = await testUIExtension({type: 'ui_extension'}) extension1.uid = 'ext1' @@ -640,7 +666,7 @@ describe('pushUpdatesForDevSession', () => { await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await options.appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() // First event is processed immediat options.appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: extension1}]}) @@ -649,7 +675,7 @@ describe('pushUpdatesForDevSession', () => { options.appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: extension2}]}) options.appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: extension1}]}) - await flushPromises() + await waitForDevSession() expect(developerPlatformClient.devSessionUpdate).toHaveBeenCalledTimes(2) const calls = developerPlatformClient.devSessionUpdate.mock.calls @@ -690,7 +716,7 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() // Then expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining("⚠️ You took over another user's session")) @@ -698,18 +724,6 @@ describe('pushUpdatesForDevSession', () => { }) test('detects session takeover during update when user ID changes', async () => { - // The session takeover detection throws an AbortError that propagates as an unhandled - // rejection through the SerialBatchProcessor (due to a missing await in bundleExtensionsAndUpload). - // Attach a .catch() to the processing promise to prevent the unhandled rejection. - const originalEnqueue = SerialBatchProcessor.prototype.enqueue - const enqueueSpy = vi.spyOn(SerialBatchProcessor.prototype, 'enqueue').mockImplementation(function ( - this: any, - ...args: [any] - ) { - originalEnqueue.apply(this, args) - this.processingPromise?.catch(() => {}) - }) - // Given - Create with initial session state developerPlatformClient.devSessionCreate = vi.fn().mockResolvedValue({ devSessionCreate: { @@ -739,16 +753,16 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: await testWebhookExtensions()}]}) await flushPromises() + await startPromise + await expect(Promise.all(processingPromises)).rejects.toThrow('has taken ownership of this dev preview') // Then expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Another developer')) expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('user2@test.com')) expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('taken ownership of this dev preview')) - - enqueueSpy.mockRestore() }) test('does not detect session takeover when session state matches during update', async () => { @@ -780,9 +794,9 @@ describe('pushUpdatesForDevSession', () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: await testWebhookExtensions()}]}) - await flushPromises() + await waitForDevSession() // Then - Should succeed normally without takeover error expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Updated dev preview on test.myshopify.com')) @@ -792,7 +806,6 @@ describe('pushUpdatesForDevSession', () => { test('retries failed events along with newly received events', async () => { vi.mocked(getUploadURL).mockResolvedValue('https://gcs.url') - vi.mocked(readdir).mockResolvedValue([]) // Setup test extensions const extension1 = await testUIExtension() extension1.uid = 'ext1-uid' @@ -815,11 +828,11 @@ describe('pushUpdatesForDevSession', () => { // Start the dev session await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) await appWatcher.start({stdout, stderr, signal: abortController.signal}) - await flushPromises() + await waitForDevSession() // First event (will fail) appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: extension1}]}) - await flushPromises() + await waitForDevSession() // Verify the update was attempted and failed expect(developerPlatformClient.devSessionUpdate).toHaveBeenCalledTimes(1) @@ -828,7 +841,7 @@ describe('pushUpdatesForDevSession', () => { // Second event (should include retry of first failed event) appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: extension2}]}) - await flushPromises() + await waitForDevSession() // Verify a second update attempt was made expect(developerPlatformClient.devSessionUpdate).toHaveBeenCalledTimes(2) diff --git a/packages/cli-kit/src/public/node/api/bulk-operations/query.test.ts b/packages/cli-kit/src/public/node/api/bulk-operations/query.test.ts index edd278f7659..b2b1b16173c 100644 --- a/packages/cli-kit/src/public/node/api/bulk-operations/query.test.ts +++ b/packages/cli-kit/src/public/node/api/bulk-operations/query.test.ts @@ -1,15 +1,11 @@ import {resolveBulkOperationQuery} from './query.js' -import {fileExists, readFile} from '../../fs.js' +import {writeFile} from '../../fs.js' +import {joinPath} from '../../path.js' +import {testWithTempDir as test} from '../../testing/test-with-temp-dir.js' import {AbortError, BugError} from '../../error.js' -import {describe, test, expect, vi, beforeEach} from 'vitest' - -vi.mock('../../fs.js') +import {describe, expect} from 'vitest' describe('resolveBulkOperationQuery', () => { - beforeEach(() => { - vi.mocked(fileExists).mockResolvedValue(true) - }) - test('returns a valid inline query', async () => { await expect(resolveBulkOperationQuery({query: 'query { shop { name } }'})).resolves.toBe('query { shop { name } }') }) @@ -18,20 +14,22 @@ describe('resolveBulkOperationQuery', () => { await expect(resolveBulkOperationQuery({query: ' '})).rejects.toThrow(AbortError) }) - test('reads and returns a valid query file', async () => { - vi.mocked(readFile).mockResolvedValue('query { shop { name } }' as never) - await expect(resolveBulkOperationQuery({queryFile: './op.graphql'})).resolves.toBe('query { shop { name } }') - expect(readFile).toHaveBeenCalledWith('./op.graphql', {encoding: 'utf8'}) + test('reads and returns a valid query file', async ({tempDir}) => { + const queryFile = joinPath(tempDir, 'op.graphql') + await writeFile(queryFile, 'query { shop { name } }') + await expect(resolveBulkOperationQuery({queryFile})).resolves.toBe('query { shop { name } }') }) - test('throws when the query file does not exist', async () => { - vi.mocked(fileExists).mockResolvedValue(false) - await expect(resolveBulkOperationQuery({queryFile: './missing.graphql'})).rejects.toThrow(AbortError) + test('throws when the query file does not exist', async ({tempDir}) => { + await expect(resolveBulkOperationQuery({queryFile: joinPath(tempDir, 'missing.graphql')})).rejects.toThrow( + AbortError, + ) }) - test('throws when the query file is empty', async () => { - vi.mocked(readFile).mockResolvedValue(' \n' as never) - await expect(resolveBulkOperationQuery({queryFile: './empty.graphql'})).rejects.toThrow(AbortError) + test('throws when the query file is empty', async ({tempDir}) => { + const queryFile = joinPath(tempDir, 'empty.graphql') + await writeFile(queryFile, ' \n') + await expect(resolveBulkOperationQuery({queryFile})).rejects.toThrow(AbortError) }) test('throws a BugError when neither query nor query file is provided', async () => { diff --git a/packages/cli-kit/src/public/node/context/local.test.ts b/packages/cli-kit/src/public/node/context/local.test.ts index 5884cc49138..6d4869046a5 100644 --- a/packages/cli-kit/src/public/node/context/local.test.ts +++ b/packages/cli-kit/src/public/node/context/local.test.ts @@ -11,13 +11,14 @@ import { getThemeKitAccessDomain, opentelemetryDomain, } from './local.js' -import {fileExists} from '../fs.js' +import {inTemporaryDirectory, writeFile} from '../fs.js' +import {joinPath} from '../path.js' +import {pathConstants} from '../../../private/node/constants.js' import {exec} from '../system.js' import {afterEach, expect, describe, vi, test} from 'vitest' import * as os from 'os' import type {NetworkInterfaceInfo} from 'os' -vi.mock('../fs.js') vi.mock('../system.js') vi.mock('../environment.js') vi.mock('os', async (importOriginal) => { @@ -132,11 +133,16 @@ describe('isShopify', () => { }) test('returns true when dev is installed', async () => { - // Given - vi.mocked(fileExists).mockResolvedValue(true) - - // When - await expect(isShopify()).resolves.toBe(true) + await inTemporaryDirectory(async (tempDir) => { + const originalDevPath = pathConstants.executables.dev + pathConstants.executables.dev = joinPath(tempDir, 'dev') + try { + await writeFile(pathConstants.executables.dev, '') + await expect(isShopify({})).resolves.toBe(true) + } finally { + Object.assign(pathConstants.executables, {dev: originalDevPath}) + } + }) }) }) diff --git a/packages/cli-kit/src/public/node/doctor/framework.test.ts b/packages/cli-kit/src/public/node/doctor/framework.test.ts index 67cd754d5bb..1324333f638 100644 --- a/packages/cli-kit/src/public/node/doctor/framework.test.ts +++ b/packages/cli-kit/src/public/node/doctor/framework.test.ts @@ -2,7 +2,6 @@ import {DoctorSuite} from './framework.js' import {describe, expect, test, vi, beforeEach} from 'vitest' import type {DoctorContext} from './types.js' -vi.mock('../fs.js') vi.mock('../system.js') /** diff --git a/packages/cli/src/cli/commands/docs/generate.test.ts b/packages/cli/src/cli/commands/docs/generate.test.ts index 7dab0a80478..679f76119e3 100644 --- a/packages/cli/src/cli/commands/docs/generate.test.ts +++ b/packages/cli/src/cli/commands/docs/generate.test.ts @@ -1,8 +1,6 @@ -import {CommandData, CommandWithMarkdown, extractCommandData, writeCommandFlagInterface} from './generate.js' -import {writeFile} from '@shopify/cli-kit/node/fs' -import {describe, test, vi, expect} from 'vitest' - -vi.mock('@shopify/cli-kit/node/fs') +import {CommandData, CommandWithMarkdown, extractCommandData} from './generate.js' +import {testWithTempDir as test} from '@shopify/cli-kit/node/testing/test-with-temp-dir' +import {describe, vi, expect} from 'vitest' const testCommand: CommandWithMarkdown = { aliases: [], @@ -45,7 +43,13 @@ describe('extractCommandData', () => { }) describe('writeCommandFlagInterface', () => { - test('calls writeFile with the correct content', async () => { + test('calls writeFile with the correct content', async ({tempDir}) => { + vi.resetModules() + const path = await import('@shopify/cli-kit/node/path') + const fs = await import('@shopify/cli-kit/node/fs') + vi.spyOn(path, 'cwd').mockReturnValue(tempDir) + const writeFile = vi.spyOn(fs, 'writeFile') + const {writeCommandFlagInterface} = await import('./generate.js') await writeCommandFlagInterface(testCommand, commandData) expect(writeFile).toHaveBeenCalledWith( @@ -72,7 +76,13 @@ export interface topictestcommand { ) }) - test('calls writeFile with the correct content for a command with no flags', async () => { + test('calls writeFile with the correct content for a command with no flags', async ({tempDir}) => { + vi.resetModules() + const path = await import('@shopify/cli-kit/node/path') + const fs = await import('@shopify/cli-kit/node/fs') + vi.spyOn(path, 'cwd').mockReturnValue(tempDir) + const writeFile = vi.spyOn(fs, 'writeFile') + const {writeCommandFlagInterface} = await import('./generate.js') await writeCommandFlagInterface({...testCommand, flags: {}}, commandData) expect(writeFile).toHaveBeenCalledWith( diff --git a/packages/eslint-plugin-cli/config.js b/packages/eslint-plugin-cli/config.js index 1e728b358c4..33e27519a37 100644 --- a/packages/eslint-plugin-cli/config.js +++ b/packages/eslint-plugin-cli/config.js @@ -161,6 +161,7 @@ const baseRules = { '@shopify/cli/command-conventional-flag-env': 'error', '@shopify/cli/command-reserved-flags': 'error', '@shopify/cli/command-json-output': 'error', + '@shopify/cli/no-filesystem-auto-mocks': 'error', '@shopify/cli/no-error-factory-functions': 'error', '@shopify/cli/no-process-cwd': 'error', '@shopify/cli/no-trailing-js-in-cli-kit-imports': 'error', diff --git a/packages/theme/src/cli/utilities/theme-command.test.ts b/packages/theme/src/cli/utilities/theme-command.test.ts index 31d8d27605a..fe72d740397 100644 --- a/packages/theme/src/cli/utilities/theme-command.test.ts +++ b/packages/theme/src/cli/utilities/theme-command.test.ts @@ -1,6 +1,7 @@ import ThemeCommand, {RequiredFlags} from './theme-command.js' import {ensureThemeStore} from './theme-store.js' -import {describe, vi, expect, test, beforeEach} from 'vitest' +import {describe, vi, expect, beforeEach} from 'vitest' +import {testWithTempDir as test} from '@shopify/cli-kit/node/testing/test-with-temp-dir' import {Config, Flags} from '@oclif/core' import {AdminSession, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session' import { @@ -8,8 +9,10 @@ import { listCurrentStoredStoreAppSessions, } from '@shopify/cli-kit/node/store-auth-session' import {loadEnvironment} from '@shopify/cli-kit/node/environments' -import {fileExistsSync} from '@shopify/cli-kit/node/fs' -import {resolvePath} from '@shopify/cli-kit/node/path' +import {fileExistsSync, rmdir, removeFile, writeFile} from '@shopify/cli-kit/node/fs' +import * as fs from '@shopify/cli-kit/node/fs' +import {joinPath, resolvePath} from '@shopify/cli-kit/node/path' +import * as path from '@shopify/cli-kit/node/path' import {renderConcurrent, renderConfirmationPrompt, renderError, renderWarning} from '@shopify/cli-kit/node/ui' import {addPublicMetadata, addSensitiveMetadata} from '@shopify/cli-kit/node/metadata' import {hashString} from '@shopify/cli-kit/node/crypto' @@ -33,7 +36,6 @@ vi.mock('@shopify/cli-kit/node/metadata', () => ({ addSensitiveMetadata: vi.fn(), })) vi.mock('./theme-store.js') -vi.mock('@shopify/cli-kit/node/fs') const CommandConfig = new Config({root: __dirname}) @@ -196,8 +198,15 @@ class TestThemeCommandWithoutStoreRequired extends ThemeCommand { describe('ThemeCommand', () => { let mockSession: AdminSession - - beforeEach(() => { + let themeDirectory: string + + beforeEach(async ({tempDir}) => { + themeDirectory = tempDir + TestThemeCommand.flags.path.default = tempDir + TestThemeCommandWithoutStoreRequired.flags.path.default = tempDir + vi.spyOn(path, 'cwd').mockReturnValue(tempDir) + vi.spyOn(fs, 'fileExistsSync') + await writeFile(joinPath(tempDir, 'shopify.theme.toml'), '') mockSession = { token: 'test-token', storeFqdn: 'test-store.myshopify.com', @@ -206,7 +215,6 @@ describe('ThemeCommand', () => { vi.mocked(ensureAuthenticatedThemes).mockResolvedValue(mockSession) vi.mocked(getCurrentStoredStoreAppSession).mockReturnValue(undefined) vi.mocked(listCurrentStoredStoreAppSessions).mockReturnValue([]) - vi.mocked(fileExistsSync).mockReturnValue(true) mockAndCaptureOutput().clear() }) @@ -245,7 +253,7 @@ describe('ThemeCommand', () => { // Then expect(loadEnvironment).toHaveBeenCalledWith('development', 'shopify.theme.toml', { - from: 'current/working/directory', + from: themeDirectory, }) expect(ensureAuthenticatedThemes).toHaveBeenCalledTimes(1) expect(renderConcurrent).not.toHaveBeenCalled() @@ -484,7 +492,7 @@ describe('ThemeCommand', () => { test('single environment provided without store - does not throw when store is not required', async () => { // Given - const environmentConfig = {path: '/some/path'} + const environmentConfig = {path: themeDirectory} vi.mocked(loadEnvironment).mockResolvedValue(environmentConfig) await CommandConfig.load() @@ -498,7 +506,7 @@ describe('ThemeCommand', () => { expect(command.commandCalls[0]).toMatchObject({ flags: { environment: ['development'], - path: '/some/path', + path: themeDirectory, }, session: undefined, multiEnvironment: false, @@ -508,7 +516,7 @@ describe('ThemeCommand', () => { test('single environment provided with store - does not create session when command does not require auth', async () => { // Given - const environmentConfig = {path: '/some/path', store: 'store.myshopify.com'} + const environmentConfig = {path: themeDirectory, store: 'store.myshopify.com'} vi.mocked(loadEnvironment).mockResolvedValue(environmentConfig) await CommandConfig.load() @@ -540,11 +548,11 @@ describe('ThemeCommand', () => { // Then expect(loadEnvironment).toHaveBeenCalledWith('development', 'shopify.theme.toml', { - from: 'current/working/directory', + from: themeDirectory, silent: true, }) expect(loadEnvironment).toHaveBeenCalledWith('staging', 'shopify.theme.toml', { - from: 'current/working/directory', + from: themeDirectory, silent: true, }) @@ -607,10 +615,10 @@ describe('ThemeCommand', () => { await CommandConfig.load() const command = new TestThemeCommand([], CommandConfig) - vi.mocked(fileExistsSync).mockReturnValue(false) + await rmdir(themeDirectory) - await expect(command.run()).rejects.toThrow('Path does not exist: current/working/directory') - expect(fileExistsSync).toHaveBeenCalledWith('current/working/directory') + await expect(command.run()).rejects.toThrow(`Path does not exist: ${themeDirectory}`) + expect(fileExistsSync).toHaveBeenCalledWith(themeDirectory) }) test('multiple environments provided - displays warning if not allowed', async () => { @@ -996,7 +1004,7 @@ describe('ThemeCommand', () => { const environmentConfig = {store: 'store.myshopify.com'} vi.mocked(loadEnvironment).mockResolvedValue(environmentConfig) vi.mocked(renderConfirmationPrompt).mockResolvedValue(true) - vi.mocked(fileExistsSync).mockReturnValue(false) + await removeFile(joinPath(themeDirectory, 'shopify.theme.toml')) await CommandConfig.load() const command = new TestThemeCommand( @@ -1059,7 +1067,7 @@ describe('ThemeCommand', () => { expect(developmentEnvFlags?.['no-color']).toEqual(true) const liveEnvFlags = commandCalls[2]?.flags - expect(liveEnvFlags?.path).toEqual('current/working/directory') + expect(liveEnvFlags?.path).toEqual(themeDirectory) expect(liveEnvFlags?.store).toEqual('store3.myshopify.com') expect(liveEnvFlags?.live).toEqual(true) expect(liveEnvFlags?.['no-color']).toEqual(true) From 565300a1ea4554437c7bf245dc2b778d1fe7e3bc Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Sun, 11 Oct 2026 10:24:35 -0400 Subject: [PATCH 3/4] Keep query file read assertion on the real implementation --- .../src/public/node/api/bulk-operations/query.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/cli-kit/src/public/node/api/bulk-operations/query.test.ts b/packages/cli-kit/src/public/node/api/bulk-operations/query.test.ts index b2b1b16173c..5a4b975471d 100644 --- a/packages/cli-kit/src/public/node/api/bulk-operations/query.test.ts +++ b/packages/cli-kit/src/public/node/api/bulk-operations/query.test.ts @@ -1,9 +1,10 @@ import {resolveBulkOperationQuery} from './query.js' import {writeFile} from '../../fs.js' +import * as fs from '../../fs.js' import {joinPath} from '../../path.js' import {testWithTempDir as test} from '../../testing/test-with-temp-dir.js' import {AbortError, BugError} from '../../error.js' -import {describe, expect} from 'vitest' +import {describe, expect, vi} from 'vitest' describe('resolveBulkOperationQuery', () => { test('returns a valid inline query', async () => { @@ -17,7 +18,9 @@ describe('resolveBulkOperationQuery', () => { test('reads and returns a valid query file', async ({tempDir}) => { const queryFile = joinPath(tempDir, 'op.graphql') await writeFile(queryFile, 'query { shop { name } }') + const readFile = vi.spyOn(fs, 'readFile') await expect(resolveBulkOperationQuery({queryFile})).resolves.toBe('query { shop { name } }') + expect(readFile).toHaveBeenCalledWith(queryFile, {encoding: 'utf8'}) }) test('throws when the query file does not exist', async ({tempDir}) => { From db29e4fce6e7ea07ab59b78c0d241ebbd7a755e6 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Sun, 11 Oct 2026 10:41:56 -0400 Subject: [PATCH 4/4] Type temp-dir fixtures in beforeEach and assert generated docs paths --- .../dev/processes/dev-session/dev-session-process.test.ts | 5 +++-- packages/cli/src/cli/commands/docs/generate.test.ts | 5 +++-- packages/theme/src/cli/utilities/theme-command.test.ts | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts index 04dde1248ab..afbacfbc796 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts @@ -14,7 +14,7 @@ import { } from '../../../../models/app/app.test-data.js' import {getUploadURL, writeManifestToBundle} from '../../../bundle.js' import {formData} from '@shopify/cli-kit/node/http' -import {describe, expect, vi, beforeEach, afterEach} from 'vitest' +import {describe, expect, vi, afterEach} from 'vitest' import {testWithTempDir as test} from '@shopify/cli-kit/node/testing/test-with-temp-dir' import {AbortController} from '@shopify/cli-kit/node/abort' import {flushPromises} from '@shopify/cli-kit/node/promises' @@ -87,7 +87,7 @@ describe('pushUpdatesForDevSession', () => { await Promise.all(processingPromises) } - beforeEach(({tempDir}) => { + test.beforeEach(({tempDir}) => { vi.mocked(formData).mockReturnValue({append: vi.fn(), getHeaders: vi.fn()} as any) stdout = {write: vi.fn()} stderr = {write: vi.fn()} @@ -757,6 +757,7 @@ describe('pushUpdatesForDevSession', () => { appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: await testWebhookExtensions()}]}) await flushPromises() await startPromise + // The takeover error escapes because bundleExtensionsAndUpload returns devSessionUpdateWithRetry without await. await expect(Promise.all(processingPromises)).rejects.toThrow('has taken ownership of this dev preview') // Then diff --git a/packages/cli/src/cli/commands/docs/generate.test.ts b/packages/cli/src/cli/commands/docs/generate.test.ts index 679f76119e3..2c087842bdd 100644 --- a/packages/cli/src/cli/commands/docs/generate.test.ts +++ b/packages/cli/src/cli/commands/docs/generate.test.ts @@ -1,5 +1,6 @@ import {CommandData, CommandWithMarkdown, extractCommandData} from './generate.js' import {testWithTempDir as test} from '@shopify/cli-kit/node/testing/test-with-temp-dir' +import {joinPath} from '@shopify/cli-kit/node/path' import {describe, vi, expect} from 'vitest' const testCommand: CommandWithMarkdown = { @@ -53,7 +54,7 @@ describe('writeCommandFlagInterface', () => { await writeCommandFlagInterface(testCommand, commandData) expect(writeFile).toHaveBeenCalledWith( - expect.stringContaining('docs-shopify.dev/commands/interfaces/topic-test-command.interface.ts'), + joinPath(tempDir, 'docs-shopify.dev/commands/interfaces/topic-test-command.interface.ts'), `// This is an autogenerated file. Don't edit this file manually. /** * The following flags are available for the \`topic test-command\` command: @@ -86,7 +87,7 @@ export interface topictestcommand { await writeCommandFlagInterface({...testCommand, flags: {}}, commandData) expect(writeFile).toHaveBeenCalledWith( - expect.stringContaining('docs-shopify.dev/commands/interfaces/topic-test-command.interface.ts'), + joinPath(tempDir, 'docs-shopify.dev/commands/interfaces/topic-test-command.interface.ts'), `// This is an autogenerated file. Don't edit this file manually. /** * The following flags are available for the \`topic test-command\` command: diff --git a/packages/theme/src/cli/utilities/theme-command.test.ts b/packages/theme/src/cli/utilities/theme-command.test.ts index fe72d740397..54d2f87426d 100644 --- a/packages/theme/src/cli/utilities/theme-command.test.ts +++ b/packages/theme/src/cli/utilities/theme-command.test.ts @@ -1,6 +1,6 @@ import ThemeCommand, {RequiredFlags} from './theme-command.js' import {ensureThemeStore} from './theme-store.js' -import {describe, vi, expect, beforeEach} from 'vitest' +import {describe, vi, expect} from 'vitest' import {testWithTempDir as test} from '@shopify/cli-kit/node/testing/test-with-temp-dir' import {Config, Flags} from '@oclif/core' import {AdminSession, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session' @@ -200,7 +200,7 @@ describe('ThemeCommand', () => { let mockSession: AdminSession let themeDirectory: string - beforeEach(async ({tempDir}) => { + test.beforeEach(async ({tempDir}) => { themeDirectory = tempDir TestThemeCommand.flags.path.default = tempDir TestThemeCommandWithoutStoreRequired.flags.path.default = tempDir