Merge pull request #3747 from janhq/fix/system-monitoring-run-in-background

fix: Optimize resource watching

web/containers/Layout/BottomPanel/SystemMonitor/SystemMonitor.test.tsx

@@ -0,0 +1,124 @@
+import '@testing-library/jest-dom'
+import React from 'react'
+import { render, screen, waitFor } from '@testing-library/react'
+import SystemMonitor from './index'
+import { useAtom, useAtomValue } from 'jotai'
+import {
+  cpuUsageAtom,
+  gpusAtom,
+  totalRamAtom,
+  usedRamAtom,
+} from '@/helpers/atoms/SystemBar.atom'
+import useGetSystemResources from '@/hooks/useGetSystemResources'
+
+// Mock dependencies
+jest.mock('jotai', () => ({
+  useAtomValue: jest.fn(),
+  useSetAtom: jest.fn(),
+  useAtom: jest.fn(),
+  atom: jest.fn(),
+}))
+
+// Mock the hooks and atoms
+jest.mock('@/hooks/useGetSystemResources')
+
+jest.mock('@/hooks/usePath', () => ({
+  usePath: () => ({ onRevealInFinder: jest.fn() }),
+}))
+
+jest.mock('@/helpers/atoms/App.atom', () => ({
+  showSystemMonitorPanelAtom: { init: false },
+}))
+
+jest.mock('@/helpers/atoms/Setting.atom', () => ({
+  reduceTransparentAtom: { init: false },
+}))
+
+jest.mock('@/helpers/atoms/SystemBar.atom', () => ({
+  totalRamAtom: { init: 16000000000 },
+  usedRamAtom: { init: 8000000000 },
+  cpuUsageAtom: { init: 50 },
+  gpusAtom: { init: [] },
+  ramUtilitizedAtom: { init: 50 },
+}))
+
+describe('SystemMonitor', () => {
+  const mockWatch = jest.fn()
+  const mockStopWatching = jest.fn()
+  beforeAll(() => {
+    jest.clearAllMocks()
+    ;(useGetSystemResources as jest.Mock).mockReturnValue({
+      watch: mockWatch,
+      stopWatching: mockStopWatching,
+    })
+  })
+  it('renders without crashing', () => {
+    ;(useAtom as jest.Mock).mockReturnValue([false, jest.fn()])
+    render(<SystemMonitor />)
+    expect(screen.getByText('System Monitor')).toBeInTheDocument()
+  })
+
+  it('renders information on expand', () => {
+    const mockGpusAtom = jest.fn()
+    const mockShowPanel = jest.fn()
+    ;(useAtom as jest.Mock).mockImplementation(mockShowPanel)
+    // Mock Jotai hooks
+    ;(useAtomValue as jest.Mock).mockImplementation((atom) => {
+      switch (atom) {
+        case totalRamAtom:
+          return 16000000000
+        case usedRamAtom:
+          return 8000000000
+        case cpuUsageAtom:
+          return 30
+        case gpusAtom:
+          return mockGpusAtom
+        default:
+          return jest.fn()
+      }
+    })
+    mockGpusAtom.mockImplementation(() => [])
+    mockShowPanel.mockImplementation(() => [true, jest.fn()])
+
+    render(<SystemMonitor />)
+
+    expect(screen.getByText('Running Models')).toBeInTheDocument()
+    expect(screen.getByText('App Log')).toBeInTheDocument()
+    expect(screen.getByText('7.45/14.90 GB')).toBeInTheDocument()
+    expect(screen.getByText('30%')).toBeInTheDocument()
+  })
+
+  it('it should not request system resource on close', async () => {
+    const mockGpusAtom = jest.fn()
+    const mockShowPanel = jest.fn()
+    ;(useAtom as jest.Mock).mockImplementation(mockShowPanel)
+
+    // Mock Jotai hooks
+    ;(useAtomValue as jest.Mock).mockImplementation((atom) => {
+      switch (atom) {
+        case totalRamAtom:
+          return 16000000000
+        case usedRamAtom:
+          return 8000000000
+        case cpuUsageAtom:
+          return 30
+        case gpusAtom:
+          return mockGpusAtom
+        default:
+          return jest.fn()
+      }
+    })
+    mockGpusAtom.mockImplementation(() => [])
+    mockShowPanel.mockImplementation(() => [true, jest.fn()])
+
+    await waitFor(async () => {
+      await render(<SystemMonitor />)
+
+      const toggle = screen.getByTestId('system-monitoring')
+      toggle.click()
+    })
+
+    expect(mockWatch).not.toHaveBeenCalled()
+    expect(mockStopWatching).toHaveBeenCalled()
+  })
+})

web/containers/Layout/BottomPanel/SystemMonitor/index.tsx

@@ -1,4 +1,4 @@
-import { Fragment, useEffect, useState } from 'react'
+import { Fragment, useCallback, useState } from 'react'
 
 import { Progress } from '@janhq/joi'
 import { useClickOutside } from '@janhq/joi'
@@ -51,35 +51,39 @@ const SystemMonitor = () => {
   const reduceTransparent = useAtomValue(reduceTransparentAtom)
 
   const { watch, stopWatching } = useGetSystemResources()
+
   useClickOutside(
     () => {
-      setShowSystemMonitorPanel(false)
+      toggleShowSystemMonitorPanel(false)
       setShowFullScreen(false)
     },
     null,
     [control, elementExpand]
   )
 
-  useEffect(() => {
-    // Watch for resource update
-    watch()
-
-    return () => {
-      stopWatching()
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [])
+  const toggleShowSystemMonitorPanel = useCallback(
+    (isShow: boolean) => {
+      setShowSystemMonitorPanel(isShow)
+      if (isShow) {
+        watch()
+      } else {
+        stopWatching()
+      }
+    },
+    [setShowSystemMonitorPanel, stopWatching, watch]
+  )
 
   return (
     <Fragment>
       <div
         ref={setControl}
+        data-testid="system-monitoring"
         className={twMerge(
           'flex cursor-pointer items-center gap-x-1 rounded px-1 py-0.5 hover:bg-[hsla(var(--secondary-bg))]',
           showSystemMonitorPanel && 'bg-[hsla(var(--secondary-bg))]'
         )}
         onClick={() => {
-          setShowSystemMonitorPanel(!showSystemMonitorPanel)
+          toggleShowSystemMonitorPanel(!showSystemMonitorPanel)
           setShowFullScreen(false)
         }}
       >
@@ -123,7 +127,7 @@ const SystemMonitor = () => {
                 size={16}
                 className="text-[hsla(var(--text-secondary))]"
                 onClick={() => {
-                  setShowSystemMonitorPanel(false)
+                  toggleShowSystemMonitorPanel(false)
                   setShowFullScreen(false)
                 }}
               />