Skip to content

Commit

Permalink
FI-2490: Fix multiple test run results overlapping (#461)
Browse files Browse the repository at this point in the history
* update zustand

* update store to new zustand version

* update other store files

* reorganize zustand variables

* fix url parsing

* remove log

---------

Co-authored-by: Alyssa Wang <awang@mitre.org>
  • Loading branch information
AlyssaWang and AlyssaWang authored Mar 6, 2024
1 parent 5979ce7 commit fd6771c
Show file tree
Hide file tree
Showing 10 changed files with 465 additions and 411 deletions.
8 changes: 3 additions & 5 deletions client/src/components/App/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ import React, { FC, useEffect } from 'react';
import { RouterProvider } from 'react-router-dom';
import { Theme } from '@mui/material/styles';
import { SnackbarProvider } from 'notistack';
import { makeStyles } from 'tss-react/mui';
import { getTestSuites } from '~/api/TestSuitesApi';
import { router } from '~/components/App/Router';
import { TestSuite } from '~/models/testSuiteModels';
import { useAppStore } from '~/store/app';
import { useTestSessionStore } from '~/store/testSession';
import SnackbarCloseButton from 'components/_common/SnackbarCloseButton';
import { makeStyles } from 'tss-react/mui';
import SnackbarCloseButton from '~/components/_common/SnackbarCloseButton';

const useStyles = makeStyles<{ height: string }>()((theme: Theme, { height }) => ({
container: {
Expand All @@ -24,10 +23,9 @@ const App: FC<unknown> = () => {
const setTestSuites = useAppStore((state) => state.setTestSuites);
const smallWindowThreshold = useAppStore((state) => state.smallWindowThreshold);
const setWindowIsSmall = useAppStore((state) => state.setWindowIsSmall);
const testRunInProgress = useTestSessionStore((state) => state.testRunInProgress);

const { classes } = useStyles({
height: testRunInProgress ? `${72 + footerHeight}px` : `${footerHeight}px`,
height: `${footerHeight}px`,
});

// Update UI on window resize
Expand Down
24 changes: 11 additions & 13 deletions client/src/components/TestSuite/TestRunButton/TestRunButton.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import React, { FC } from 'react';
import { useLocation } from 'react-router-dom';
import { Button } from '@mui/material';
import PlayArrowIcon from '@mui/icons-material/PlayArrow';
import PlayCircleIcon from '@mui/icons-material/PlayCircle';
import { TestGroup, Runnable, RunnableType } from '~/models/testSuiteModels';
import { useTestSessionStore } from '~/store/testSession';
import CustomTooltip from '~/components/_common/CustomTooltip';
import { testRunInProgress } from '~/components/TestSuite/TestSuiteUtilities';
import lightTheme from '~/styles/theme';

import { useTestSessionStore } from '~/store/testSession';

export interface TestRunButtonProps {
runnable: Runnable;
runnableType: RunnableType;
Expand All @@ -21,15 +22,14 @@ const TestRunButton: FC<TestRunButtonProps> = ({
runnableType,
buttonText,
}) => {
const testRunInProgress = useTestSessionStore((state) => state.testRunInProgress);
/* Need to explicitly check against false because undefined needs to be treated
* as true. */
const currentRunnables = useTestSessionStore((state) => state.currentRunnables);
const showRunButton = (runnable as TestGroup).user_runnable !== false;
const inProgress = testRunInProgress(currentRunnables, useLocation().pathname);

const textButton = (
<Button
variant="contained"
disabled={testRunInProgress}
disabled={inProgress}
color="secondary"
size="small"
disableElevation
Expand All @@ -47,26 +47,24 @@ const TestRunButton: FC<TestRunButtonProps> = ({
const iconButton = (
<CustomTooltip describeChild title={`Run ${runnable.title}`}>
<PlayCircleIcon
aria-label={`Run ${runnable.title}${
testRunInProgress ? ' Disabled - Test Run in Progress' : ''
}`}
aria-label={`Run ${runnable.title}${inProgress ? ' Disabled - Test Run in Progress' : ''}`}
aria-hidden={false}
tabIndex={0}
color={testRunInProgress ? 'disabled' : 'secondary'}
color={inProgress ? 'disabled' : 'secondary'}
data-testid={`runButton-${runnable.id}`}
onClick={() => {
if (!testRunInProgress) runTests(runnableType, runnable.id);
if (!inProgress) runTests(runnableType, runnable.id);
}}
onKeyDown={(e) => {
e.stopPropagation();
if (e.key === 'Enter' && !testRunInProgress) {
if (e.key === 'Enter' && !inProgress) {
runTests(runnableType, runnable.id);
}
}}
sx={{
margin: '0 8px',
padding: '0.25em 0.25em',
':hover': testRunInProgress
':hover': inProgress
? {}
: {
background: lightTheme.palette.common.grayLightest,
Expand Down
35 changes: 21 additions & 14 deletions client/src/components/TestSuite/TestSession.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ import { useAppStore } from '~/store/app';
import { useTestSessionStore } from '~/store/testSession';
import { useEffectOnce } from '~/hooks/useEffectOnce';
import { useTimeout } from '~/hooks/useTimeout';
import { mapRunnableToId, resultsToMap, setIsRunning } from './TestSuiteUtilities';
import {
mapRunnableToId,
resultsToMap,
setIsRunning,
testRunInProgress,
} from '~/components/TestSuite/TestSuiteUtilities';
import lightTheme from '~/styles/theme';

export interface TestSessionComponentProps {
Expand Down Expand Up @@ -64,11 +69,9 @@ const TestSessionComponent: FC<TestSessionComponentProps> = ({
const footerHeight = useAppStore((state) => state.footerHeight);
const headerHeight = useAppStore((state) => state.headerHeight);
const windowIsSmall = useAppStore((state) => state.windowIsSmall);
const runnableId = useTestSessionStore((state) => state.runnableId);
const testRunInProgress = useTestSessionStore((state) => state.testRunInProgress);
const setRunnableId = useTestSessionStore((state) => state.setRunnableId);
const currentRunnables = useTestSessionStore((state) => state.currentRunnables);
const setCurrentRunnables = useTestSessionStore((state) => state.setCurrentRunnables);
const setTestRunId = useTestSessionStore((state) => state.setTestRunId);
const setTestRunInProgress = useTestSessionStore((state) => state.setTestRunInProgress);

const [inputModalVisible, setInputModalVisible] = React.useState(false);
const [waitingTestId, setWaitingTestId] = React.useState<string | null>();
Expand Down Expand Up @@ -118,23 +121,27 @@ const TestSessionComponent: FC<TestSessionComponentProps> = ({
}

if (testRunIsInProgress(testRun)) {
const runnableId = currentRunnables[testSession.id];
const runnable = runnableMap.get(runnableId);
if (runnable) setIsRunning(runnable, true);
}
});

// Set testRunIsInProgress and is_running status when testRun changes
useEffect(() => {
const inProgress = testRunIsInProgress(testRun);
setTestRunInProgress(inProgress);

// Wipe both currently running runnable and selected (currently rendered) runnable
if (!inProgress) {
if (testRun && !testRunIsInProgress(testRun)) {
const runnableId = currentRunnables[testSession.id];
const runnableFromId = runnableMap.get(runnableId);
if (runnableFromId) setIsRunning(runnableFromId, false);

const runnableFromSelected = runnableMap.get(selectedRunnable);
if (runnableFromSelected) setIsRunning(runnableFromSelected, false);

const runnableFromId = runnableMap.get(runnableId);
if (runnableFromId) setIsRunning(runnableFromId, false);
// Delete runnable from storage when test run is done
const updatedRunnables = currentRunnables;
delete updatedRunnables[testSession.id];
setCurrentRunnables(updatedRunnables);
}
}, [testRun]);

Expand Down Expand Up @@ -168,7 +175,7 @@ const TestSessionComponent: FC<TestSessionComponentProps> = ({
const showInputsModal = (runnableType: RunnableType, runnableId: string, inputs: TestInput[]) => {
setInputs(inputs);
setRunnableType(runnableType);
setRunnableId(runnableId);
setCurrentRunnables({ ...currentRunnables, [testSession.id]: runnableId });
setInputModalVisible(true);
};

Expand Down Expand Up @@ -294,7 +301,7 @@ const TestSessionComponent: FC<TestSessionComponentProps> = ({
: false;

const renderTestRunProgressBar = () => {
const duration = testRunInProgress ? null : 2000;
const duration = testRunInProgress(currentRunnables, useLocation().pathname) ? null : 2000;
return (
<TestRunProgressBar
showProgressBar={showProgressBar}
Expand Down Expand Up @@ -400,7 +407,7 @@ const TestSessionComponent: FC<TestSessionComponentProps> = ({
<InputsModal
createTestRun={createTestRun}
runnableType={runnableType}
runnableId={runnableId}
runnableId={currentRunnables[testSession.id]}
title={(runnableMap.get(selectedRunnable) as Runnable).title}
inputInstructions={(runnableMap.get(selectedRunnable) as Runnable).input_instructions}
inputs={inputs}
Expand Down
12 changes: 6 additions & 6 deletions client/src/components/TestSuite/TestSuiteUtilities.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ export const setIsRunning = (runnable: Runnable, value: boolean) => {
export const shouldShowDescription = (
runnable: Runnable,
description: JSX.Element | undefined
): boolean => {
if (description && runnable.description && runnable.description.length > 0) {
return true;
} else {
return false;
}
): boolean => !!description && !!runnable.description && runnable.description.length > 0;

export const testRunInProgress = (activeRunnables: Record<string, string>, location: string) => {
// Get session ID from URL string
const sessionId = location.split('?')[0].split('#')[0].split('/').reverse()[0];
return Object.keys(activeRunnables).includes(sessionId);
};
3 changes: 2 additions & 1 deletion client/src/models/testSuiteModels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ export interface TestRun {
status?: 'queued' | 'running' | 'waiting' | 'cancelling' | 'done';
test_count?: number;
test_group_id?: string;
test_suite_id?: string;
test_id?: string;
test_session_id?: string;
test_suite_id?: string;
}

export interface TestSession {
Expand Down
31 changes: 17 additions & 14 deletions client/src/store/app.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import create from 'zustand';
import { create } from 'zustand';
import { devtoolsInDev } from './devtools';

import { TestSuite, TestSession } from '~/models/testSuiteModels';
Expand All @@ -18,17 +18,20 @@ type AppStore = {

// this store is for global state, things at the top level
// other stores can be attached to child components
export const useAppStore = create<AppStore>(
devtoolsInDev((set, _get) => ({
footerHeight: 56, // default height, small window height is 36
headerHeight: 64,
testSuites: [] as TestSuite[],
testSession: undefined,
smallWindowThreshold: 1000,
windowIsSmall: undefined,
setFooterHeight: (footerHeight: number) => set({ footerHeight: footerHeight }),
setTestSuites: (testSuites: TestSuite[]) => set({ testSuites: testSuites }),
setTestSession: (testSession: TestSession | undefined) => set({ testSession: testSession }),
setWindowIsSmall: (windowIsSmall: boolean | undefined) => set({ windowIsSmall: windowIsSmall }),
}))
export const useAppStore = create<AppStore>()(
devtoolsInDev(
(set, _get): AppStore => ({
footerHeight: 56, // default height, small window height is 36
headerHeight: 64,
testSuites: [] as TestSuite[],
testSession: undefined,
smallWindowThreshold: 1000,
windowIsSmall: undefined,
setFooterHeight: (footerHeight: number) => set({ footerHeight: footerHeight }),
setTestSuites: (testSuites: TestSuite[]) => set({ testSuites: testSuites }),
setTestSession: (testSession: TestSession | undefined) => set({ testSession: testSession }),
setWindowIsSmall: (windowIsSmall: boolean | undefined) =>
set({ windowIsSmall: windowIsSmall }),
})
)
);
2 changes: 0 additions & 2 deletions client/src/store/devtools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,3 @@ import { devtools } from 'zustand/middleware';
export const devtoolsInDev = (process.env.NODE_ENV === 'development'
? devtools
: (fn: any) => fn) as unknown as typeof devtools;
/* eslint-enable @typescript-eslint/no-unsafe-return */
/* eslint-enable @typescript-eslint/no-explicit-any */
33 changes: 18 additions & 15 deletions client/src/store/testSession.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,33 @@
import create from 'zustand';
import { create } from 'zustand';
import { persist } from 'zustand/middleware';

import { devtoolsInDev } from './devtools';

interface CurrentRunnables {
[key: string]: string;
}

type TestSessionStore = {
currentRunnables: CurrentRunnables;
runnableId: string;
testRunId: string | undefined;
testRunInProgress: boolean;
setRunnableId: (runnableId: string | undefined) => void;
setCurrentRunnables: (currentRunnables: CurrentRunnables) => void;
setRunnableId: (runnableId: string) => void;
setTestRunId: (testRunId: string | undefined) => void;
setTestRunInProgress: (testRunInProgress: boolean) => void;
};

export const useTestSessionStore = create<TestSessionStore>(
export const useTestSessionStore = create<TestSessionStore>()(
persist(
devtoolsInDev(
(set, _get) =>
({
runnableId: '',
testRunId: undefined,
testRunInProgress: false,
setRunnableId: (runnableId: string) => set({ runnableId: runnableId }),
setTestRunId: (testRunId: string | undefined) => set({ testRunId: testRunId }),
setTestRunInProgress: (testRunInProgress: boolean) =>
set({ testRunInProgress: testRunInProgress }),
} as TestSessionStore)
(set, _get): TestSessionStore => ({
currentRunnables: {},
runnableId: '',
testRunId: undefined,
setCurrentRunnables: (currentRunnables: CurrentRunnables) =>
set({ currentRunnables: { ...currentRunnables } }),
setRunnableId: (runnableId: string) => set({ runnableId: runnableId }),
setTestRunId: (testRunId: string | undefined) => set({ testRunId: testRunId }),
})
),
{
name: 'test-session',
Expand Down
Loading

0 comments on commit fd6771c

Please sign in to comment.