Webui Notification Improvements

locust/webui/src/components/LogViewer/LogDisplay.tsx

@@ -0,0 +1,26 @@
+import { Typography } from '@mui/material';
+import { red, amber, blue } from '@mui/material/colors';
+
+const getLogColor = (log: string) => {
+  if (log.includes('CRITICAL')) {
+    return red[900];
+  }
+  if (log.includes('ERROR')) {
+    return red[700];
+  }
+  if (log.includes('WARNING')) {
+    return amber[900];
+  }
+  if (log.includes('DEBUG')) {
+    return blue[700];
+  }
+  return 'text.primary';
+};
+
+export default function LogDisplay({ log }: { log: string }) {
+  return (
+    <Typography color={getLogColor(log)} fontFamily={'monospace'} variant='body2'>
+      {log}
+    </Typography>
+  );
+}

locust/webui/src/components/LogViewer/LogViewer.tsx

@@ -1,69 +1,9 @@
-import { useEffect, useState } from 'react';
-import ExpandMoreIcon from '@mui/icons-material/ExpandMore';
-import PriorityHighIcon from '@mui/icons-material/PriorityHigh';
-import {
-  Accordion,
-  AccordionDetails,
-  AccordionSummary,
-  Box,
-  Paper,
-  Typography,
-} from '@mui/material';
-import { red, amber, blue } from '@mui/material/colors';
+import { Box, Paper, Typography } from '@mui/material';
 
-import { isImportantLog } from 'components/LogViewer/logUtils';
+import LogDisplay from 'components/LogViewer/LogDisplay';
+import WorkerLogs from 'components/LogViewer/WorkerLogs';
 import { useSelector } from 'redux/hooks';
-import { objectLength } from 'utils/object';
-
-const getLogColor = (log: string) => {
-  if (log.includes('CRITICAL')) {
-    return red[900];
-  }
-  if (log.includes('ERROR')) {
-    return red[700];
-  }
-  if (log.includes('WARNING')) {
-    return amber[900];
-  }
-  if (log.includes('DEBUG')) {
-    return blue[700];
-  }
-  return 'text.primary';
-};
-
-function LogDisplay({ log }: { log: string }) {
-  return (
-    <Typography color={getLogColor(log)} fontFamily={'monospace'} variant='body2'>
-      {log}
-    </Typography>
-  );
-}
-
-function WorkerLogs({ workerId, logs }: { workerId: string; logs: string[] }) {
-  const [hasImportantLog, setHasImportantLog] = useState(false);
-
-  useEffect(() => {
-    if (logs.some(isImportantLog)) {
-      setHasImportantLog(true);
-    }
-  }, [logs]);
-
-  return (
-    <Accordion>
-      <AccordionSummary expandIcon={<ExpandMoreIcon />} onClick={() => setHasImportantLog(false)}>
-        {hasImportantLog && <PriorityHighIcon color='secondary' />}
-        {workerId}
-      </AccordionSummary>
-      <AccordionDetails>
-        <Paper elevation={3} sx={{ p: 2 }}>
-          {logs.map((log, index) => (
-            <LogDisplay key={`worker-${workerId}-log-${index}`} log={log} />
-          ))}
-        </Paper>
-      </AccordionDetails>
-    </Accordion>
-  );
-}
+import { isEmpty } from 'utils/object';
 
 export default function LogViewer() {
   const { master: masterLogs, workers: workerLogs } = useSelector(({ logViewer }) => logViewer);
@@ -80,7 +20,7 @@ export default function LogViewer() {
           ))}
         </Paper>
       </Box>
-      {!!objectLength(workerLogs) && (
+      {!isEmpty(workerLogs) && (
         <Box>
           <Typography sx={{ mb: 2 }} variant='h5'>
             Worker Logs

locust/webui/src/components/LogViewer/WorkerLogs.tsx

@@ -0,0 +1,40 @@
+import { useCallback, useEffect, useState } from 'react';
+import ExpandMoreIcon from '@mui/icons-material/ExpandMore';
+import PriorityHighIcon from '@mui/icons-material/PriorityHigh';
+import { Accordion, AccordionDetails, AccordionSummary, Paper } from '@mui/material';
+
+import LogDisplay from 'components/LogViewer/LogDisplay';
+import { isImportantLog } from 'components/LogViewer/LogViewer.utils';
+
+export default function WorkerLogs({ workerId, logs }: { workerId: string; logs: string[] }) {
+  const [hasImportantLog, setHasImportantLog] = useState(false);
+
+  useEffect(() => {
+    if (logs.slice(localStorage[workerId]).some(isImportantLog)) {
+      setHasImportantLog(true);
+    }
+  }, [logs]);
+
+  const onExpandLogs = useCallback(() => {
+    setHasImportantLog(false);
+    localStorage[workerId] = logs.length;
+  }, []);
+
+  return (
+    <Accordion>
+      <AccordionSummary expandIcon={<ExpandMoreIcon />} onClick={onExpandLogs}>
+        {hasImportantLog && (
+          <PriorityHighIcon color='secondary' data-testid='worker-notification' />
+        )}
+        {workerId}
+      </AccordionSummary>
+      <AccordionDetails>
+        <Paper elevation={3} sx={{ p: 2 }}>
+          {logs.map((log, index) => (
+            <LogDisplay key={`worker-${workerId}-log-${index}`} log={log} />
+          ))}
+        </Paper>
+      </AccordionDetails>
+    </Accordion>
+  );
+}

locust/webui/src/components/LogViewer/tests/WorkerLogs.test.tsx

@@ -0,0 +1,32 @@
+import { act, fireEvent, render } from '@testing-library/react';
+import { describe, expect, test } from 'vitest';
+
+import WorkerLogs from '../WorkerLogs';
+
+const workerLogs = ['Worker Log 1', 'ERROR Worker Log 2'];
+const workerId = 'worker-1';
+
+describe('WorkerLogs', () => {
+  test('should render a notification with important worker logs', () => {
+    const { getByTestId } = render(<WorkerLogs logs={workerLogs} workerId={workerId} />);
+
+    expect(getByTestId('worker-notification')).toBeTruthy();
+  });
+
+  test('should hide notification and store log index when expanding logs', () => {
+    const { queryByTestId, getByTestId, getByRole } = render(
+      <WorkerLogs logs={workerLogs} workerId={workerId} />,
+    );
+
+    expect(getByTestId('worker-notification')).toBeTruthy();
+
+    const button = getByRole('button', { name: workerId });
+
+    act(() => {
+      fireEvent.click(button);
+    });
+
+    expect(queryByTestId('worker-notification')).toBeFalsy();
+    expect(localStorage[workerId]).toBe(String(workerLogs.length));
+  });
+});

locust/webui/src/components/LogViewer/tests/useLogViewer.test.tsx

@@ -1,7 +1,7 @@
 import { waitFor } from '@testing-library/react';
 import { http, HttpResponse } from 'msw';
 import { setupServer } from 'msw/node';
-import { afterAll, afterEach, beforeAll, describe, expect, test } from 'vitest';
+import { afterEach, describe, expect, test } from 'vitest';
 
 import useLogViewer from 'components/LogViewer/useLogViewer';
 import { TEST_BASE_API } from 'test/constants';
@@ -15,7 +15,19 @@ const mockLogs = {
   },
 };
 
-const server = setupServer(http.get(`${TEST_BASE_API}/logs`, () => HttpResponse.json(mockLogs)));
+const mockImportantMasterLog = {
+  master: ['Log 1', 'WARNING Log 2', 'Log 3'],
+  workers: {
+    '123': ['Worker Log'],
+  },
+};
+
+const mockImportantWorkerLog = {
+  master: ['Log 1', 'WARNING Log 2', 'Log 3'],
+  workers: {
+    '123': ['ERROR Worker Log'],
+  },
+};
 
 function MockHook() {
   const logs = useLogViewer();
@@ -24,11 +36,16 @@ function MockHook() {
 }
 
 describe('useLogViewer', () => {
-  beforeAll(() => server.listen());
-  afterEach(() => server.resetHandlers());
-  afterAll(() => server.close());
+  afterEach(() => {
+    localStorage.clear();
+  });
 
   test('should fetch logs from server and store them in state', async () => {
+    const server = setupServer(
+      http.get(`${TEST_BASE_API}/logs`, () => HttpResponse.json(mockLogs)),
+    );
+    server.listen();
+
     const { store, getByTestId } = renderWithProvider(<MockHook />, {
       swarm: swarmStateMock,
     });
@@ -37,5 +54,45 @@ describe('useLogViewer', () => {
       expect(getByTestId('logs').textContent).toBe(JSON.stringify(mockLogs));
       expect(store.getState().logViewer).toEqual(mockLogs);
     });
+    server.resetHandlers();
+    server.close();
+  });
+
+  test('should set a notification if important logs are present', async () => {
+    const server = setupServer(
+      http.get(`${TEST_BASE_API}/logs`, () => HttpResponse.json(mockImportantMasterLog)),
+    );
+    server.listen();
+
+    const { store, getByTestId } = renderWithProvider(<MockHook />, {
+      swarm: swarmStateMock,
+    });
+
+    await waitFor(() => {
+      expect(getByTestId('logs').textContent).toBe(JSON.stringify(mockImportantMasterLog));
+      expect(store.getState().logViewer).toEqual(mockImportantMasterLog);
+      expect(store.getState().notification).toEqual({ logViewer: true });
+    });
+
+    server.close();
+  });
+
+  test('should set a notification if important worker logs are present', async () => {
+    const server = setupServer(
+      http.get(`${TEST_BASE_API}/logs`, () => HttpResponse.json(mockImportantWorkerLog)),
+    );
+    server.listen();
+
+    const { store, getByTestId } = renderWithProvider(<MockHook />, {
+      swarm: swarmStateMock,
+    });
+
+    await waitFor(() => {
+      expect(getByTestId('logs').textContent).toBe(JSON.stringify(mockImportantWorkerLog));
+      expect(store.getState().logViewer).toEqual(mockImportantWorkerLog);
+      expect(store.getState().notification).toEqual({ logViewer: true });
+    });
+
+    server.close();
   });
 });

locust/webui/src/components/LogViewer/useLogViewer.ts

@@ -1,6 +1,6 @@
 import { useCallback, useEffect, useMemo } from 'react';
 
-import { isImportantLog } from 'components/LogViewer/logUtils';
+import { isImportantLog } from 'components/LogViewer/LogViewer.utils';
 import { LOG_VIEWER_KEY } from 'constants/logs';
 import { SWARM_STATE } from 'constants/swarm';
 import useInterval from 'hooks/useInterval';
@@ -10,19 +10,21 @@ import { useAction, useSelector } from 'redux/hooks';
 import { logViewerActions } from 'redux/slice/logViewer.slice';
 import { flatten } from 'utils/array';
 
+const defaultLogs = { master: [], workers: {} };
+
 export default function useLogViewer() {
   const swarm = useSelector(({ swarm }) => swarm);
   const setLogs = useAction(logViewerActions.setLogs);
   const { data, refetch: refetchLogs } = useGetLogsQuery();
 
-  const logs = data || { master: [], workers: {} };
+  const logs = data || defaultLogs;
 
   const workerLogs = useMemo(() => flatten<string>(Object.values(logs.workers)), [logs.workers]);
-  const allLogs = [...logs.master, ...workerLogs];
+  const allLogs = useMemo(() => [...logs.master].concat(workerLogs), [logs.master, logs.workers]);
 
   const shouldNotifyLogsUpdate = useCallback(
-    () => allLogs.slice(localStorage['logViewer']).some(isImportantLog),
-    [logs],
+    (key: string) => allLogs.slice(localStorage[key]).some(isImportantLog),
+    [allLogs],
   );
 
   useInterval(refetchLogs, 5000, {