From 5c99f89e9d95194bec0745efcc6af6dd1e8f92a8 Mon Sep 17 00:00:00 2001
From: Christian Byrne <cbyrne@comfy.org>
Date: Tue, 27 Jan 2026 21:18:34 -0800
Subject: [PATCH] test: improve refreshRemoteConfig test quality (#8356)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Addresses review feedback from #8266:

- Use vi.stubGlobal for global fetch mock
- Extract mockSuccessResponse and mockErrorResponse helpers to reduce
duplication
- Add test verifying 500 responses preserve existing config (only
401/403 clear config)

Related: #8266

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8356-test-improve-refreshRemoteConfig-test-quality-2f66d73d365081f7b207c2fd2b8a8179)
by [Unito](https://www.unito.io)
---
 .../remoteConfig/refreshRemoteConfig.test.ts  | 63 ++++++++++++-------
 1 file changed, 40 insertions(+), 23 deletions(-)

diff --git a/src/platform/remoteConfig/refreshRemoteConfig.test.ts b/src/platform/remoteConfig/refreshRemoteConfig.test.ts
index 34b75d1238..f947b822cb 100644
--- a/src/platform/remoteConfig/refreshRemoteConfig.test.ts
+++ b/src/platform/remoteConfig/refreshRemoteConfig.test.ts
@@ -11,11 +11,26 @@ vi.mock('@/scripts/api', () => ({
   }
 }))
 
-global.fetch = vi.fn()
+vi.stubGlobal('fetch', vi.fn())
 
 describe('refreshRemoteConfig', () => {
   const mockConfig = { feature1: true, feature2: 'value' }
 
+  function mockSuccessResponse(config = mockConfig) {
+    return {
+      ok: true,
+      json: async () => config
+    } as Response
+  }
+
+  function mockErrorResponse(status: number, statusText: string) {
+    return {
+      ok: false,
+      status,
+      statusText
+    } as Response
+  }
+
   beforeEach(() => {
     vi.clearAllMocks()
     remoteConfig.value = {}
@@ -24,10 +39,7 @@ describe('refreshRemoteConfig', () => {
 
   describe('with auth (default)', () => {
     it('uses api.fetchApi when useAuth is true', async () => {
-      vi.mocked(api.fetchApi).mockResolvedValue({
-        ok: true,
-        json: async () => mockConfig
-      } as Response)
+      vi.mocked(api.fetchApi).mockResolvedValue(mockSuccessResponse())
 
       await refreshRemoteConfig({ useAuth: true })
 
@@ -40,10 +52,7 @@ describe('refreshRemoteConfig', () => {
     })
 
     it('uses api.fetchApi by default', async () => {
-      vi.mocked(api.fetchApi).mockResolvedValue({
-        ok: true,
-        json: async () => mockConfig
-      } as Response)
+      vi.mocked(api.fetchApi).mockResolvedValue(mockSuccessResponse())
 
       await refreshRemoteConfig()
 
@@ -54,10 +63,7 @@ describe('refreshRemoteConfig', () => {
 
   describe('without auth', () => {
     it('uses raw fetch when useAuth is false', async () => {
-      vi.mocked(global.fetch).mockResolvedValue({
-        ok: true,
-        json: async () => mockConfig
-      } as Response)
+      vi.mocked(global.fetch).mockResolvedValue(mockSuccessResponse())
 
       await refreshRemoteConfig({ useAuth: false })
 
@@ -72,11 +78,9 @@ describe('refreshRemoteConfig', () => {
 
   describe('error handling', () => {
     it('clears config on 401 response', async () => {
-      vi.mocked(api.fetchApi).mockResolvedValue({
-        ok: false,
-        status: 401,
-        statusText: 'Unauthorized'
-      } as Response)
+      vi.mocked(api.fetchApi).mockResolvedValue(
+        mockErrorResponse(401, 'Unauthorized')
+      )
 
       await refreshRemoteConfig()
 
@@ -85,11 +89,9 @@ describe('refreshRemoteConfig', () => {
     })
 
     it('clears config on 403 response', async () => {
-      vi.mocked(api.fetchApi).mockResolvedValue({
-        ok: false,
-        status: 403,
-        statusText: 'Forbidden'
-      } as Response)
+      vi.mocked(api.fetchApi).mockResolvedValue(
+        mockErrorResponse(403, 'Forbidden')
+      )
 
       await refreshRemoteConfig()
 
@@ -105,5 +107,20 @@ describe('refreshRemoteConfig', () => {
       expect(remoteConfig.value).toEqual({})
       expect(window.__CONFIG__).toEqual({})
     })
+
+    it('preserves config on 500 response', async () => {
+      const existingConfig = { subscription_required: true }
+      remoteConfig.value = existingConfig
+      window.__CONFIG__ = existingConfig
+
+      vi.mocked(api.fetchApi).mockResolvedValue(
+        mockErrorResponse(500, 'Internal Server Error')
+      )
+
+      await refreshRemoteConfig()
+
+      expect(remoteConfig.value).toEqual(existingConfig)
+      expect(window.__CONFIG__).toEqual(existingConfig)
+    })
   })
 })