From a1fd1f71a0d99c890ec2990039b6f327b570235b Mon Sep 17 00:00:00 2001
From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com>
Date: Mon, 4 May 2026 00:15:45 +0000
Subject: [PATCH] [Tests] Refactor Notifier tests to use real filesystem

Replaced brittle filesystem mocks in `notifier.test.ts` with real filesystem operations using `inTemporaryDirectory`.
This makes the tests more trustworthy and better aligns with the project's testing standards.

Key changes:
- Removed `vi.mock('fs/promises')`.
- Used `inTemporaryDirectory` for tests interacting with the filesystem.
- Verified actual file content and timestamp updates.
- Improved test isolation with `vi.restoreAllMocks()`.
- Simulated filesystem errors deterministically by creating directory conflicts.
---
 .../theme/src/cli/utilities/notifier.test.ts  | 58 ++++++++++++-------
 1 file changed, 37 insertions(+), 21 deletions(-)

diff --git a/packages/theme/src/cli/utilities/notifier.test.ts b/packages/theme/src/cli/utilities/notifier.test.ts
index e3ad842a599..35416e1360e 100644
--- a/packages/theme/src/cli/utilities/notifier.test.ts
+++ b/packages/theme/src/cli/utilities/notifier.test.ts
@@ -1,15 +1,19 @@
 import {Notifier} from './notifier.js'
-import {vi, describe, expect, test} from 'vitest'
+import {vi, describe, expect, test, beforeEach} from 'vitest'
 import {outputWarn} from '@shopify/cli-kit/node/output'
+import {inTemporaryDirectory, readFile, mkdir} from '@shopify/cli-kit/node/fs'
+import {joinPath} from '@shopify/cli-kit/node/path'
+import {stat} from 'node:fs/promises'
 
-import fs from 'fs/promises'
-
-vi.mock('fs/promises')
 vi.mock('@shopify/cli-kit/node/output')
 
 describe('Notifier', () => {
   let notifier: Notifier
 
+  beforeEach(() => {
+    vi.restoreAllMocks()
+  })
+
   test('updates notifyPath via POST request when path is a URL', async () => {
     const mockFetch = vi.spyOn(global, 'fetch').mockResolvedValue(new Response())
     const url = 'https://example.com/notify'
@@ -26,13 +30,23 @@ 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 notifier.notify(fileName)
-
-    expect(fs.writeFile).toHaveBeenCalledWith(path, fileName)
+    await inTemporaryDirectory(async (tmpDir) => {
+      // Given
+      const path = joinPath(tmpDir, 'theme.update')
+      notifier = new Notifier(path)
+      const fileName = 'announcement.liquid'
+      const startTime = Date.now()
+
+      // When
+      await notifier.notify(fileName)
+
+      // Then
+      expect(await readFile(path)).toBe(fileName)
+      const stats = await stat(path)
+      // File system timestamps might have lower precision (seconds), so we subtract 1000ms to avoid flakiness
+      expect(stats.atime.getTime()).toBeGreaterThanOrEqual(startTime - 1000)
+      expect(stats.mtime.getTime()).toBeGreaterThanOrEqual(startTime - 1000)
+    })
   })
 
   test('does not update if path is empty', async () => {
@@ -43,7 +57,6 @@ 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 () => {
@@ -55,7 +68,6 @@ describe('Notifier', () => {
     await notifier.notify(fileName)
 
     expect(mockFetch).toHaveBeenCalled()
-    expect(fs.appendFile).not.toHaveBeenCalled()
   })
 
   test('prints error if response is not successful', async () => {
@@ -85,15 +97,19 @@ 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) => {
+      // Create a directory where the file should be, to cause writeFile to fail
+      const path = joinPath(tmpDir, 'theme.update')
+      await mkdir(path)
 
-    await notifier.notify(fileName)
+      notifier = new Notifier(path)
+      const fileName = 'announcement.liquid'
 
-    expect(outputWarn).toHaveBeenCalledWith(
-      `Failed to notify filechange listener at ${invalidPath}: No such file or directory`,
-    )
+      await notifier.notify(fileName)
+
+      expect(outputWarn).toHaveBeenCalledWith(
+        expect.stringContaining(`Failed to notify filechange listener at ${path}`),
+      )
+    })
   })
 })