From 080bef214bcaf7f1d6ca00da8384dad7f92af4e2 Mon Sep 17 00:00:00 2001 From: Louis Date: Tue, 1 Oct 2024 13:09:17 +0700 Subject: [PATCH 1/3] fix: hide system monitoring bar should stop spawning subprocess --- .../BottomPanel/SystemMonitor/index.tsx | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/web/containers/Layout/BottomPanel/SystemMonitor/index.tsx b/web/containers/Layout/BottomPanel/SystemMonitor/index.tsx index a69e34d57..fb334082e 100644 --- a/web/containers/Layout/BottomPanel/SystemMonitor/index.tsx +++ b/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,24 +51,27 @@ 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 ( @@ -79,7 +82,7 @@ const SystemMonitor = () => { showSystemMonitorPanel && 'bg-[hsla(var(--secondary-bg))]' )} onClick={() => { - setShowSystemMonitorPanel(!showSystemMonitorPanel) + toggleShowSystemMonitorPanel(!showSystemMonitorPanel) setShowFullScreen(false) }} > @@ -123,7 +126,7 @@ const SystemMonitor = () => { size={16} className="text-[hsla(var(--text-secondary))]" onClick={() => { - setShowSystemMonitorPanel(false) + toggleShowSystemMonitorPanel(false) setShowFullScreen(false) }} /> From 6fdfe760e84425f13c3b2f2352a23e10b05a0898 Mon Sep 17 00:00:00 2001 From: Louis Date: Tue, 1 Oct 2024 13:36:13 +0700 Subject: [PATCH 2/3] chore: add tests --- .../SystemMonitor/SystemMonitor.test.tsx | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 web/containers/Layout/BottomPanel/SystemMonitor/SystemMonitor.test.tsx diff --git a/web/containers/Layout/BottomPanel/SystemMonitor/SystemMonitor.test.tsx b/web/containers/Layout/BottomPanel/SystemMonitor/SystemMonitor.test.tsx new file mode 100644 index 000000000..501639287 --- /dev/null +++ b/web/containers/Layout/BottomPanel/SystemMonitor/SystemMonitor.test.tsx @@ -0,0 +1,86 @@ +import '@testing-library/jest-dom' +import React from 'react' +import { render, screen } from '@testing-library/react' +import SystemMonitor from './index' +import { useAtom, useAtomValue } from 'jotai' +import { + cpuUsageAtom, + gpusAtom, + totalRamAtom, + usedRamAtom, +} from '@/helpers/atoms/SystemBar.atom' + +// 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', () => ({ + __esModule: true, + default: () => ({ watch: jest.fn(), stopWatching: jest.fn() }), +})) + +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', () => { + beforeAll(() => { + jest.clearAllMocks() + }) + it('renders without crashing', () => { + ;(useAtom as jest.Mock).mockReturnValue([false, jest.fn()]) + render() + 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() + + 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() + }) +}) From da5865f3b462c34ecd4eb42cd66c6c6609e6af14 Mon Sep 17 00:00:00 2001 From: Louis Date: Tue, 1 Oct 2024 15:06:51 +0700 Subject: [PATCH 3/3] test: add test cases --- .../SystemMonitor/SystemMonitor.test.tsx | 48 +++++++++++++++++-- .../BottomPanel/SystemMonitor/index.tsx | 1 + 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/web/containers/Layout/BottomPanel/SystemMonitor/SystemMonitor.test.tsx b/web/containers/Layout/BottomPanel/SystemMonitor/SystemMonitor.test.tsx index 501639287..dce55b595 100644 --- a/web/containers/Layout/BottomPanel/SystemMonitor/SystemMonitor.test.tsx +++ b/web/containers/Layout/BottomPanel/SystemMonitor/SystemMonitor.test.tsx @@ -1,6 +1,6 @@ import '@testing-library/jest-dom' import React from 'react' -import { render, screen } from '@testing-library/react' +import { render, screen, waitFor } from '@testing-library/react' import SystemMonitor from './index' import { useAtom, useAtomValue } from 'jotai' import { @@ -9,6 +9,7 @@ import { totalRamAtom, usedRamAtom, } from '@/helpers/atoms/SystemBar.atom' +import useGetSystemResources from '@/hooks/useGetSystemResources' // Mock dependencies jest.mock('jotai', () => ({ @@ -19,10 +20,7 @@ jest.mock('jotai', () => ({ })) // Mock the hooks and atoms -jest.mock('@/hooks/useGetSystemResources', () => ({ - __esModule: true, - default: () => ({ watch: jest.fn(), stopWatching: jest.fn() }), -})) +jest.mock('@/hooks/useGetSystemResources') jest.mock('@/hooks/usePath', () => ({ usePath: () => ({ onRevealInFinder: jest.fn() }), @@ -45,8 +43,14 @@ jest.mock('@/helpers/atoms/SystemBar.atom', () => ({ })) 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()]) @@ -83,4 +87,38 @@ describe('SystemMonitor', () => { 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() + + const toggle = screen.getByTestId('system-monitoring') + toggle.click() + }) + + expect(mockWatch).not.toHaveBeenCalled() + expect(mockStopWatching).toHaveBeenCalled() + }) }) diff --git a/web/containers/Layout/BottomPanel/SystemMonitor/index.tsx b/web/containers/Layout/BottomPanel/SystemMonitor/index.tsx index fb334082e..7fdc598ec 100644 --- a/web/containers/Layout/BottomPanel/SystemMonitor/index.tsx +++ b/web/containers/Layout/BottomPanel/SystemMonitor/index.tsx @@ -77,6 +77,7 @@ const SystemMonitor = () => {