test: improve Jotai atom test quality and add model-provider atoms tests

Replace dynamic imports with static imports in marketplace atom tests. Convert type-only and not-toThrow assertions into proper state-change verifications. Add comprehensive test suite for model-provider-page atoms covering all four hooks, cross-hook interaction, selectAtom granularity, and Provider isolation.
2026-05-03 08:58:09 +08:00 · 2026-03-05 22:49:09 +08:00
parent 56e0dc0ae6
commit 6d612c0909
3 changed files with 478 additions and 60 deletions
--- a/web/app/components/header/account-setting/model-provider-page/atoms.spec.tsx
+++ b/web/app/components/header/account-setting/model-provider-page/atoms.spec.tsx
@ -0,0 +1,399 @@
+import type { ReactNode } from 'react'
+import { act, renderHook } from '@testing-library/react'
+import { Provider } from 'jotai'
+import { beforeEach, describe, expect, it } from 'vitest'
+import {
+  useExpandModelProviderList,
+  useModelProviderListExpanded,
+  useResetModelProviderListExpanded,
+  useSetModelProviderListExpanded,
+} from './atoms'
+
+const createWrapper = () => {
+  return ({ children }: { children: ReactNode }) => (
+    <Provider>{children}</Provider>
+  )
+}
+
+describe('atoms', () => {
+  let wrapper: ReturnType<typeof createWrapper>
+
+  beforeEach(() => {
+    wrapper = createWrapper()
+  })
+
+  // Read hook: returns whether a specific provider is expanded
+  describe('useModelProviderListExpanded', () => {
+    it('should return false when provider has not been expanded', () => {
+      const { result } = renderHook(
+        () => useModelProviderListExpanded('openai'),
+        { wrapper },
+      )
+
+      expect(result.current).toBe(false)
+    })
+
+    it('should return false for any unknown provider name', () => {
+      const { result } = renderHook(
+        () => useModelProviderListExpanded('nonexistent-provider'),
+        { wrapper },
+      )
+
+      expect(result.current).toBe(false)
+    })
+
+    it('should return true when provider has been expanded via setter', () => {
+      const { result } = renderHook(
+        () => ({
+          expanded: useModelProviderListExpanded('openai'),
+          setExpanded: useSetModelProviderListExpanded('openai'),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.setExpanded(true)
+      })
+
+      expect(result.current.expanded).toBe(true)
+    })
+  })
+
+  // Setter hook: toggles expanded state for a specific provider
+  describe('useSetModelProviderListExpanded', () => {
+    it('should expand a provider when called with true', () => {
+      const { result } = renderHook(
+        () => ({
+          expanded: useModelProviderListExpanded('anthropic'),
+          setExpanded: useSetModelProviderListExpanded('anthropic'),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.setExpanded(true)
+      })
+
+      expect(result.current.expanded).toBe(true)
+    })
+
+    it('should collapse a provider when called with false', () => {
+      const { result } = renderHook(
+        () => ({
+          expanded: useModelProviderListExpanded('anthropic'),
+          setExpanded: useSetModelProviderListExpanded('anthropic'),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.setExpanded(true)
+      })
+      act(() => {
+        result.current.setExpanded(false)
+      })
+
+      expect(result.current.expanded).toBe(false)
+    })
+
+    it('should not affect other providers when setting one', () => {
+      const { result } = renderHook(
+        () => ({
+          openaiExpanded: useModelProviderListExpanded('openai'),
+          anthropicExpanded: useModelProviderListExpanded('anthropic'),
+          setOpenai: useSetModelProviderListExpanded('openai'),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.setOpenai(true)
+      })
+
+      expect(result.current.openaiExpanded).toBe(true)
+      expect(result.current.anthropicExpanded).toBe(false)
+    })
+  })
+
+  // Expand hook: expands any provider by name
+  describe('useExpandModelProviderList', () => {
+    it('should expand the specified provider', () => {
+      const { result } = renderHook(
+        () => ({
+          expanded: useModelProviderListExpanded('google'),
+          expand: useExpandModelProviderList(),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.expand('google')
+      })
+
+      expect(result.current.expanded).toBe(true)
+    })
+
+    it('should expand multiple providers independently', () => {
+      const { result } = renderHook(
+        () => ({
+          openaiExpanded: useModelProviderListExpanded('openai'),
+          anthropicExpanded: useModelProviderListExpanded('anthropic'),
+          expand: useExpandModelProviderList(),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.expand('openai')
+      })
+      act(() => {
+        result.current.expand('anthropic')
+      })
+
+      expect(result.current.openaiExpanded).toBe(true)
+      expect(result.current.anthropicExpanded).toBe(true)
+    })
+
+    it('should not collapse already expanded providers when expanding another', () => {
+      const { result } = renderHook(
+        () => ({
+          openaiExpanded: useModelProviderListExpanded('openai'),
+          anthropicExpanded: useModelProviderListExpanded('anthropic'),
+          expand: useExpandModelProviderList(),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.expand('openai')
+      })
+      act(() => {
+        result.current.expand('anthropic')
+      })
+
+      expect(result.current.openaiExpanded).toBe(true)
+    })
+  })
+
+  // Reset hook: clears all expanded state back to empty
+  describe('useResetModelProviderListExpanded', () => {
+    it('should reset all expanded providers to false', () => {
+      const { result } = renderHook(
+        () => ({
+          openaiExpanded: useModelProviderListExpanded('openai'),
+          anthropicExpanded: useModelProviderListExpanded('anthropic'),
+          expand: useExpandModelProviderList(),
+          reset: useResetModelProviderListExpanded(),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.expand('openai')
+      })
+      act(() => {
+        result.current.expand('anthropic')
+      })
+      act(() => {
+        result.current.reset()
+      })
+
+      expect(result.current.openaiExpanded).toBe(false)
+      expect(result.current.anthropicExpanded).toBe(false)
+    })
+
+    it('should be safe to call when no providers are expanded', () => {
+      const { result } = renderHook(
+        () => ({
+          expanded: useModelProviderListExpanded('openai'),
+          reset: useResetModelProviderListExpanded(),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.reset()
+      })
+
+      expect(result.current.expanded).toBe(false)
+    })
+
+    it('should allow re-expanding providers after reset', () => {
+      const { result } = renderHook(
+        () => ({
+          expanded: useModelProviderListExpanded('openai'),
+          expand: useExpandModelProviderList(),
+          reset: useResetModelProviderListExpanded(),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.expand('openai')
+      })
+      act(() => {
+        result.current.reset()
+      })
+      act(() => {
+        result.current.expand('openai')
+      })
+
+      expect(result.current.expanded).toBe(true)
+    })
+  })
+
+  // Cross-hook interaction: verify hooks cooperate through the shared atom
+  describe('Cross-hook interaction', () => {
+    it('should reflect state set by useSetModelProviderListExpanded in useModelProviderListExpanded', () => {
+      const { result } = renderHook(
+        () => ({
+          expanded: useModelProviderListExpanded('openai'),
+          setExpanded: useSetModelProviderListExpanded('openai'),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.setExpanded(true)
+      })
+
+      expect(result.current.expanded).toBe(true)
+    })
+
+    it('should reflect state set by useExpandModelProviderList in useModelProviderListExpanded', () => {
+      const { result } = renderHook(
+        () => ({
+          expanded: useModelProviderListExpanded('anthropic'),
+          expand: useExpandModelProviderList(),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.expand('anthropic')
+      })
+
+      expect(result.current.expanded).toBe(true)
+    })
+
+    it('should allow useSetModelProviderListExpanded to collapse a provider expanded by useExpandModelProviderList', () => {
+      const { result } = renderHook(
+        () => ({
+          expanded: useModelProviderListExpanded('openai'),
+          expand: useExpandModelProviderList(),
+          setExpanded: useSetModelProviderListExpanded('openai'),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.expand('openai')
+      })
+      expect(result.current.expanded).toBe(true)
+
+      act(() => {
+        result.current.setExpanded(false)
+      })
+      expect(result.current.expanded).toBe(false)
+    })
+
+    it('should reset state set by useSetModelProviderListExpanded via useResetModelProviderListExpanded', () => {
+      const { result } = renderHook(
+        () => ({
+          expanded: useModelProviderListExpanded('openai'),
+          setExpanded: useSetModelProviderListExpanded('openai'),
+          reset: useResetModelProviderListExpanded(),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.setExpanded(true)
+      })
+      act(() => {
+        result.current.reset()
+      })
+
+      expect(result.current.expanded).toBe(false)
+    })
+  })
+
+  // selectAtom granularity: changing one provider should not affect unrelated reads
+  describe('selectAtom granularity', () => {
+    it('should not cause unrelated provider reads to change when one provider is toggled', () => {
+      const { result } = renderHook(
+        () => ({
+          openai: useModelProviderListExpanded('openai'),
+          anthropic: useModelProviderListExpanded('anthropic'),
+          google: useModelProviderListExpanded('google'),
+          setOpenai: useSetModelProviderListExpanded('openai'),
+        }),
+        { wrapper },
+      )
+
+      const anthropicBefore = result.current.anthropic
+      const googleBefore = result.current.google
+
+      act(() => {
+        result.current.setOpenai(true)
+      })
+
+      expect(result.current.openai).toBe(true)
+      expect(result.current.anthropic).toBe(anthropicBefore)
+      expect(result.current.google).toBe(googleBefore)
+    })
+
+    it('should keep individual provider states independent across multiple expansions and collapses', () => {
+      const { result } = renderHook(
+        () => ({
+          openai: useModelProviderListExpanded('openai'),
+          anthropic: useModelProviderListExpanded('anthropic'),
+          setOpenai: useSetModelProviderListExpanded('openai'),
+          setAnthropic: useSetModelProviderListExpanded('anthropic'),
+        }),
+        { wrapper },
+      )
+
+      act(() => {
+        result.current.setOpenai(true)
+      })
+      act(() => {
+        result.current.setAnthropic(true)
+      })
+      act(() => {
+        result.current.setOpenai(false)
+      })
+
+      expect(result.current.openai).toBe(false)
+      expect(result.current.anthropic).toBe(true)
+    })
+  })
+
+  // Isolation: separate Provider instances have independent state
+  describe('Provider isolation', () => {
+    it('should have independent state across different Provider instances', () => {
+      const wrapper1 = createWrapper()
+      const wrapper2 = createWrapper()
+
+      const { result: result1 } = renderHook(
+        () => ({
+          expanded: useModelProviderListExpanded('openai'),
+          setExpanded: useSetModelProviderListExpanded('openai'),
+        }),
+        { wrapper: wrapper1 },
+      )
+
+      const { result: result2 } = renderHook(
+        () => useModelProviderListExpanded('openai'),
+        { wrapper: wrapper2 },
+      )
+
+      act(() => {
+        result1.current.setExpanded(true)
+      })
+
+      expect(result1.current.expanded).toBe(true)
+      expect(result2.current).toBe(false)
+    })
+  })
+})
--- a/web/app/components/plugins/marketplace/tests/atoms.spec.tsx
+++ b/web/app/components/plugins/marketplace/tests/atoms.spec.tsx
@ -3,6 +3,16 @@ import { act, renderHook } from '@testing-library/react'
 import { Provider as JotaiProvider } from 'jotai'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { createNuqsTestWrapper } from '@/test/nuqs-testing'
+import {
+  useActivePluginType,
+  useFilterPluginTags,
+  useMarketplaceMoreClick,
+  useMarketplaceSearchMode,
+  useMarketplaceSort,
+  useMarketplaceSortValue,
+  useSearchPluginText,
+  useSetMarketplaceSort,
+} from '../atoms'
 import { DEFAULT_SORT } from '../constants'

 const createWrapper = (searchParams = '') => {
@ -22,8 +32,7 @@ describe('Marketplace sort atoms', () => {
    vi.clearAllMocks()
  })

-  it('should return default sort value from useMarketplaceSort', async () => {
-    const { useMarketplaceSort } = await import('../atoms')
+  it('should return default sort value from useMarketplaceSort', () => {
    const { wrapper } = createWrapper()
    const { result } = renderHook(() => useMarketplaceSort(), { wrapper })

@ -31,24 +40,28 @@ describe('Marketplace sort atoms', () => {
    expect(typeof result.current[1]).toBe('function')
  })

-  it('should return default sort value from useMarketplaceSortValue', async () => {
-    const { useMarketplaceSortValue } = await import('../atoms')
+  it('should return default sort value from useMarketplaceSortValue', () => {
    const { wrapper } = createWrapper()
    const { result } = renderHook(() => useMarketplaceSortValue(), { wrapper })

    expect(result.current).toEqual(DEFAULT_SORT)
  })

-  it('should return setter from useSetMarketplaceSort', async () => {
-    const { useSetMarketplaceSort } = await import('../atoms')
+  it('should return setter from useSetMarketplaceSort', () => {
    const { wrapper } = createWrapper()
-    const { result } = renderHook(() => useSetMarketplaceSort(), { wrapper })
+    const { result } = renderHook(() => ({
+      setSort: useSetMarketplaceSort(),
+      sortValue: useMarketplaceSortValue(),
+    }), { wrapper })

-    expect(typeof result.current).toBe('function')
+    act(() => {
+      result.current.setSort({ sortBy: 'created_at', sortOrder: 'ASC' })
+    })
+
+    expect(result.current.sortValue).toEqual({ sortBy: 'created_at', sortOrder: 'ASC' })
  })

-  it('should update sort value via useMarketplaceSort setter', async () => {
-    const { useMarketplaceSort } = await import('../atoms')
+  it('should update sort value via useMarketplaceSort setter', () => {
    const { wrapper } = createWrapper()
    const { result } = renderHook(() => useMarketplaceSort(), { wrapper })

@ -65,8 +78,7 @@ describe('useSearchPluginText', () => {
    vi.clearAllMocks()
  })

-  it('should return empty string as default', async () => {
-    const { useSearchPluginText } = await import('../atoms')
+  it('should return empty string as default', () => {
    const { wrapper } = createWrapper()
    const { result } = renderHook(() => useSearchPluginText(), { wrapper })

@ -74,8 +86,7 @@ describe('useSearchPluginText', () => {
    expect(typeof result.current[1]).toBe('function')
  })

-  it('should parse q from search params', async () => {
-    const { useSearchPluginText } = await import('../atoms')
+  it('should parse q from search params', () => {
    const { wrapper } = createWrapper('?q=hello')
    const { result } = renderHook(() => useSearchPluginText(), { wrapper })

@ -83,16 +94,14 @@ describe('useSearchPluginText', () => {
  })

  it('should expose a setter function for search text', async () => {
-    const { useSearchPluginText } = await import('../atoms')
    const { wrapper } = createWrapper()
    const { result } = renderHook(() => useSearchPluginText(), { wrapper })

-    expect(typeof result.current[1]).toBe('function')
-
-    // Calling the setter should not throw
    await act(async () => {
      result.current[1]('search term')
    })
+
+    expect(result.current[0]).toBe('search term')
  })
 })

@ -101,16 +110,14 @@ describe('useActivePluginType', () => {
    vi.clearAllMocks()
  })

-  it('should return "all" as default category', async () => {
-    const { useActivePluginType } = await import('../atoms')
+  it('should return "all" as default category', () => {
    const { wrapper } = createWrapper()
    const { result } = renderHook(() => useActivePluginType(), { wrapper })

    expect(result.current[0]).toBe('all')
  })

-  it('should parse category from search params', async () => {
-    const { useActivePluginType } = await import('../atoms')
+  it('should parse category from search params', () => {
    const { wrapper } = createWrapper('?category=tool')
    const { result } = renderHook(() => useActivePluginType(), { wrapper })

@ -123,16 +130,14 @@ describe('useFilterPluginTags', () => {
    vi.clearAllMocks()
  })

-  it('should return empty array as default', async () => {
-    const { useFilterPluginTags } = await import('../atoms')
+  it('should return empty array as default', () => {
    const { wrapper } = createWrapper()
    const { result } = renderHook(() => useFilterPluginTags(), { wrapper })

    expect(result.current[0]).toEqual([])
  })

-  it('should parse tags from search params', async () => {
-    const { useFilterPluginTags } = await import('../atoms')
+  it('should parse tags from search params', () => {
    const { wrapper } = createWrapper('?tags=search')
    const { result } = renderHook(() => useFilterPluginTags(), { wrapper })

@ -145,42 +150,35 @@ describe('useMarketplaceSearchMode', () => {
    vi.clearAllMocks()
  })

-  it('should return false when no search text, no tags, and category has collections (all)', async () => {
-    const { useMarketplaceSearchMode } = await import('../atoms')
+  it('should return false when no search text, no tags, and category has collections (all)', () => {
    const { wrapper } = createWrapper('?category=all')
    const { result } = renderHook(() => useMarketplaceSearchMode(), { wrapper })

-    // "all" is in PLUGIN_CATEGORY_WITH_COLLECTIONS, so search mode should be false
    expect(result.current).toBe(false)
  })

-  it('should return true when search text is present', async () => {
-    const { useMarketplaceSearchMode } = await import('../atoms')
+  it('should return true when search text is present', () => {
    const { wrapper } = createWrapper('?q=test&category=all')
    const { result } = renderHook(() => useMarketplaceSearchMode(), { wrapper })

    expect(result.current).toBe(true)
  })

-  it('should return true when tags are present', async () => {
-    const { useMarketplaceSearchMode } = await import('../atoms')
+  it('should return true when tags are present', () => {
    const { wrapper } = createWrapper('?tags=search&category=all')
    const { result } = renderHook(() => useMarketplaceSearchMode(), { wrapper })

    expect(result.current).toBe(true)
  })

-  it('should return true when category does not have collections (e.g. model)', async () => {
-    const { useMarketplaceSearchMode } = await import('../atoms')
+  it('should return true when category does not have collections (e.g. model)', () => {
    const { wrapper } = createWrapper('?category=model')
    const { result } = renderHook(() => useMarketplaceSearchMode(), { wrapper })

-    // "model" is NOT in PLUGIN_CATEGORY_WITH_COLLECTIONS, so search mode = true
    expect(result.current).toBe(true)
  })

-  it('should return false when category has collections (tool) and no search/tags', async () => {
-    const { useMarketplaceSearchMode } = await import('../atoms')
+  it('should return false when category has collections (tool) and no search/tags', () => {
    const { wrapper } = createWrapper('?category=tool')
    const { result } = renderHook(() => useMarketplaceSearchMode(), { wrapper })

@ -193,27 +191,33 @@ describe('useMarketplaceMoreClick', () => {
    vi.clearAllMocks()
  })

-  it('should return a callback function', async () => {
-    const { useMarketplaceMoreClick } = await import('../atoms')
+  it('should return a callback function', () => {
    const { wrapper } = createWrapper()
    const { result } = renderHook(() => useMarketplaceMoreClick(), { wrapper })

    expect(typeof result.current).toBe('function')
  })

-  it('should do nothing when called with no params', async () => {
-    const { useMarketplaceMoreClick } = await import('../atoms')
+  it('should do nothing when called with no params', () => {
    const { wrapper } = createWrapper()
-    const { result } = renderHook(() => useMarketplaceMoreClick(), { wrapper })
+    const { result } = renderHook(() => ({
+      handleMoreClick: useMarketplaceMoreClick(),
+      sort: useMarketplaceSortValue(),
+      searchText: useSearchPluginText()[0],
+    }), { wrapper })
+
+    const sortBefore = result.current.sort
+    const searchTextBefore = result.current.searchText

-    // Should not throw when called with undefined
    act(() => {
-      result.current(undefined)
+      result.current.handleMoreClick(undefined)
    })
+
+    expect(result.current.sort).toEqual(sortBefore)
+    expect(result.current.searchText).toBe(searchTextBefore)
  })

-  it('should update search state when called with search params', async () => {
-    const { useMarketplaceMoreClick, useMarketplaceSortValue } = await import('../atoms')
+  it('should update search state when called with search params', () => {
    const { wrapper } = createWrapper()

    const { result } = renderHook(() => ({
@ -229,17 +233,20 @@ describe('useMarketplaceMoreClick', () => {
      })
    })

-    // Sort should be updated via the jotai atom
    expect(result.current.sort).toEqual({ sortBy: 'created_at', sortOrder: 'ASC' })
  })

-  it('should use defaults when search params fields are missing', async () => {
-    const { useMarketplaceMoreClick } = await import('../atoms')
+  it('should use defaults when search params fields are missing', () => {
    const { wrapper } = createWrapper()
-    const { result } = renderHook(() => useMarketplaceMoreClick(), { wrapper })
+    const { result } = renderHook(() => ({
+      handleMoreClick: useMarketplaceMoreClick(),
+      sort: useMarketplaceSortValue(),
+    }), { wrapper })

    act(() => {
-      result.current({})
+      result.current.handleMoreClick({})
    })
+
+    expect(result.current.sort).toEqual(DEFAULT_SORT)
  })
 })
--- a/web/app/components/plugins/marketplace/tests/plugin-type-switch.spec.tsx
+++ b/web/app/components/plugins/marketplace/tests/plugin-type-switch.spec.tsx
@ -74,31 +74,40 @@ describe('PluginTypeSwitch', () => {
    const { Wrapper } = createWrapper('?category=all')
    render(<PluginTypeSwitch />, { wrapper: Wrapper })

-    // Click on Models option — should not throw
-    expect(() => fireEvent.click(screen.getByText('Models'))).not.toThrow()
+    fireEvent.click(screen.getByText('Models'))
+
+    const modelsButton = screen.getByText('Models').closest('div')
+    expect(modelsButton?.className).toContain('!bg-components-main-nav-nav-button-bg-active')
  })

  it('should handle clicking on category with collections (Tools)', () => {
    const { Wrapper } = createWrapper('?category=model')
    render(<PluginTypeSwitch />, { wrapper: Wrapper })

-    // Click on "Tools" which has collections → setSearchMode(null)
-    expect(() => fireEvent.click(screen.getByText('Tools'))).not.toThrow()
+    fireEvent.click(screen.getByText('Tools'))
+
+    const toolsButton = screen.getByText('Tools').closest('div')
+    expect(toolsButton?.className).toContain('!bg-components-main-nav-nav-button-bg-active')
  })

  it('should handle clicking on category without collections (Models)', () => {
    const { Wrapper } = createWrapper('?category=all')
    render(<PluginTypeSwitch />, { wrapper: Wrapper })

-    // Click on "Models" which does NOT have collections → no setSearchMode call
-    expect(() => fireEvent.click(screen.getByText('Models'))).not.toThrow()
+    fireEvent.click(screen.getByText('Models'))
+
+    const modelsButton = screen.getByText('Models').closest('div')
+    expect(modelsButton?.className).toContain('!bg-components-main-nav-nav-button-bg-active')
  })

  it('should handle clicking on bundles', () => {
    const { Wrapper } = createWrapper('?category=all')
    render(<PluginTypeSwitch />, { wrapper: Wrapper })

-    expect(() => fireEvent.click(screen.getByText('Bundles'))).not.toThrow()
+    fireEvent.click(screen.getByText('Bundles'))
+
+    const bundlesButton = screen.getByText('Bundles').closest('div')
+    expect(bundlesButton?.className).toContain('!bg-components-main-nav-nav-button-bg-active')
  })

  it('should handle clicking on each category', () => {
@ -107,7 +116,10 @@ describe('PluginTypeSwitch', () => {

    const categories = ['All', 'Models', 'Tools', 'Data Sources', 'Triggers', 'Agents', 'Extensions', 'Bundles']
    categories.forEach((category) => {
-      expect(() => fireEvent.click(screen.getByText(category))).not.toThrow()
+      fireEvent.click(screen.getByText(category))
+
+      const button = screen.getByText(category).closest('div')
+      expect(button?.className).toContain('!bg-components-main-nav-nav-button-bg-active')
    })
  })