From 22cb6dc10be4fff1fe9d74302068e48c6050e9ac Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Sun, 3 May 2026 00:36:47 +0000 Subject: [PATCH] [Tests] Refactor Notifier tests to remove filesystem mocks Refactored `packages/theme/src/cli/utilities/notifier.test.ts` to use `inTemporaryDirectory` and real filesystem operations instead of mocking `fs/promises`. Updated test assertions to verify actual file contents, ensuring behavior-driven testing rather than implementation-focused mock assertions. --- .../theme/src/cli/utilities/notifier.test.ts | 56 ++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/packages/theme/src/cli/utilities/notifier.test.ts b/packages/theme/src/cli/utilities/notifier.test.ts index e3ad842a599..bf4b59d91a5 100644 --- a/packages/theme/src/cli/utilities/notifier.test.ts +++ b/packages/theme/src/cli/utilities/notifier.test.ts @@ -1,10 +1,9 @@ import {Notifier} from './notifier.js' import {vi, describe, expect, test} from 'vitest' import {outputWarn} from '@shopify/cli-kit/node/output' +import {inTemporaryDirectory, readFile} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' -import fs from 'fs/promises' - -vi.mock('fs/promises') vi.mock('@shopify/cli-kit/node/output') describe('Notifier', () => { @@ -26,13 +25,16 @@ describe('Notifier', () => { }) test('updates file atime and mtime when path is not a URL', async () => { - const path = 'theme.update' - notifier = new Notifier(path) - const fileName = 'announcement.liquid' + await inTemporaryDirectory(async (tmpDir) => { + const path = joinPath(tmpDir, 'theme.update') + notifier = new Notifier(path) + const fileName = 'announcement.liquid' - await notifier.notify(fileName) + await notifier.notify(fileName) - expect(fs.writeFile).toHaveBeenCalledWith(path, fileName) + const content = await readFile(path) + expect(content).toEqual(fileName) + }) }) test('does not update if path is empty', async () => { @@ -43,19 +45,22 @@ describe('Notifier', () => { await notifier.notify(fileName) expect(fetchSpy).not.toHaveBeenCalled() - expect(fs.appendFile).not.toHaveBeenCalled() }) test('does not notify file when path is URL', async () => { - const url = 'https://example.com/notify' - const mockFetch = vi.spyOn(global, 'fetch').mockResolvedValue(new Response()) - notifier = new Notifier(url) - const fileName = 'announcement.liquid' - - await notifier.notify(fileName) - - expect(mockFetch).toHaveBeenCalled() - expect(fs.appendFile).not.toHaveBeenCalled() + await inTemporaryDirectory(async (tmpDir) => { + const url = 'https://example.com/notify' + const mockFetch = vi.spyOn(global, 'fetch').mockResolvedValue(new Response()) + notifier = new Notifier(url) + const fileName = 'announcement.liquid' + const path = joinPath(tmpDir, 'theme.update') + + await notifier.notify(fileName) + + expect(mockFetch).toHaveBeenCalled() + const fileExists = await readFile(path).catch(() => null) + expect(fileExists).toBeNull() + }) }) test('prints error if response is not successful', async () => { @@ -85,15 +90,14 @@ describe('Notifier', () => { }) test('outputs error if file update fails', async () => { - const invalidPath = 'dir/file:theme.update' - vi.spyOn(fs, 'writeFile').mockRejectedValue(new Error('No such file or directory')) - notifier = new Notifier(invalidPath) - const fileName = 'announcement.liquid' + await inTemporaryDirectory(async (tmpDir) => { + const invalidPath = tmpDir + notifier = new Notifier(invalidPath) + const fileName = 'announcement.liquid' - await notifier.notify(fileName) + await notifier.notify(fileName) - expect(outputWarn).toHaveBeenCalledWith( - `Failed to notify filechange listener at ${invalidPath}: No such file or directory`, - ) + expect(outputWarn).toHaveBeenCalledWith(expect.stringContaining(`Failed to notify filechange listener at ${tmpDir}`)) + }) }) })