Skip to content

Commit

Permalink
[Response Ops] Fix flaky maintenance window E2E tests (#188487)
Browse files Browse the repository at this point in the history
## Summary

Issue: #187818

Fix flaky test where it's not waiting for the page to load before
searching maintenance window. Also slightly improve the maintenance
window table to not flicker when loading. Instead it should just show
the spinner.

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
  • Loading branch information
JiaweiWu authored Sep 4, 2024
1 parent 37d6545 commit 8b2cb75
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const useFindMaintenanceWindows = (props?: UseFindMaintenanceWindowsProps
const {
isLoading,
isFetching,
isInitialLoading,
data = [],
refetch,
} = useQuery({
Expand All @@ -54,6 +55,7 @@ export const useFindMaintenanceWindows = (props?: UseFindMaintenanceWindowsProps
return {
maintenanceWindows: data,
isLoading: enabled && (isLoading || isFetching),
isInitialLoading,
refetch,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ describe('MaintenanceWindowsList', () => {
const result = appMockRenderer.render(
<MaintenanceWindowsList
refreshData={() => {}}
loading={false}
isLoading={false}
items={items}
readOnly={false}
/>
Expand Down Expand Up @@ -125,7 +125,7 @@ describe('MaintenanceWindowsList', () => {
const result = appMockRenderer.render(
<MaintenanceWindowsList
refreshData={() => {}}
loading={false}
isLoading={false}
items={items}
readOnly={true}
/>
Expand All @@ -142,7 +142,7 @@ describe('MaintenanceWindowsList', () => {
const result = appMockRenderer.render(
<MaintenanceWindowsList
refreshData={refreshData}
loading={false}
isLoading={false}
items={items}
readOnly={false}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@
* 2.0.
*/

import React, { ReactElement, useCallback, useMemo } from 'react';
import React, { useCallback, useMemo } from 'react';
import {
formatDate,
EuiInMemoryTable,
EuiBasicTableColumn,
EuiFlexGroup,
EuiFlexItem,
SearchFilterConfig,
EuiBadge,
useEuiTheme,
EuiButton,
EuiSearchBarProps,
} from '@elastic/eui';
import { css } from '@emotion/react';
import { SortDirection } from '../types';
Expand All @@ -35,7 +35,7 @@ import { useArchiveMaintenanceWindow } from '../../../hooks/use_archive_maintena
import { useFinishAndArchiveMaintenanceWindow } from '../../../hooks/use_finish_and_archive_maintenance_window';

interface MaintenanceWindowsListProps {
loading: boolean;
isLoading: boolean;
items: MaintenanceWindow[];
readOnly: boolean;
refreshData: () => void;
Expand Down Expand Up @@ -99,21 +99,9 @@ const rowProps = (item: MaintenanceWindow) => ({
});

export const MaintenanceWindowsList = React.memo<MaintenanceWindowsListProps>(
({ loading, items, readOnly, refreshData }) => {
const search: { filters: SearchFilterConfig[]; toolsRight: ReactElement } = {
filters: [
{
type: 'custom_component',
component: StatusFilter,
},
],
toolsRight: (
<EuiButton data-test-subj="refresh-button" iconType="refresh" onClick={refreshData}>
{i18n.REFRESH}
</EuiButton>
),
};
({ isLoading, items, readOnly, refreshData }) => {
const { euiTheme } = useEuiTheme();

const { navigateToEditMaintenanceWindows } = useEditMaintenanceWindowsNavigation();
const onEdit = useCallback<TableActionsPopoverProps['onEdit']>(
(id) => navigateToEditMaintenanceWindows(id),
Expand All @@ -127,6 +115,7 @@ export const MaintenanceWindowsList = React.memo<MaintenanceWindowsListProps>(
);
const { mutate: archiveMaintenanceWindow, isLoading: isLoadingArchive } =
useArchiveMaintenanceWindow();

const onArchive = useCallback(
(id: string, archive: boolean) =>
archiveMaintenanceWindow(
Expand All @@ -137,11 +126,16 @@ export const MaintenanceWindowsList = React.memo<MaintenanceWindowsListProps>(
);
const { mutate: finishAndArchiveMaintenanceWindow, isLoading: isLoadingFinishAndArchive } =
useFinishAndArchiveMaintenanceWindow();

const onCancelAndArchive = useCallback(
(id: string) => finishAndArchiveMaintenanceWindow(id, { onSuccess: () => refreshData() }),
[finishAndArchiveMaintenanceWindow, refreshData]
);

const isMutatingOrLoading = useMemo(() => {
return isLoadingFinish || isLoadingArchive || isLoadingFinishAndArchive || isLoading;
}, [isLoadingFinish, isLoadingArchive, isLoadingFinishAndArchive, isLoading]);

const tableCss = useMemo(() => {
return css`
.euiTableRow {
Expand All @@ -160,6 +154,7 @@ export const MaintenanceWindowsList = React.memo<MaintenanceWindowsListProps>(
return (
<TableActionsPopover
id={id}
isLoading={isMutatingOrLoading}
status={status}
onEdit={onEdit}
onCancel={onCancel}
Expand All @@ -170,20 +165,43 @@ export const MaintenanceWindowsList = React.memo<MaintenanceWindowsListProps>(
},
},
],
[onArchive, onCancel, onCancelAndArchive, onEdit]
[isMutatingOrLoading, onArchive, onCancel, onCancelAndArchive, onEdit]
);

const columns = useMemo(
() => (readOnly ? COLUMNS : COLUMNS.concat(actions)),
[actions, readOnly]
);

const search: EuiSearchBarProps = useMemo(
() => ({
filters: [
{
type: 'custom_component',
component: StatusFilter,
},
],
toolsRight: (
<EuiButton
data-test-subj="refresh-button"
iconType="refresh"
onClick={refreshData}
isLoading={isMutatingOrLoading}
isDisabled={isMutatingOrLoading}
>
{i18n.REFRESH}
</EuiButton>
),
}),
[isMutatingOrLoading, refreshData]
);

return (
<EuiInMemoryTable
data-test-subj="maintenance-windows-table"
css={tableCss}
itemId="id"
loading={loading || isLoadingFinish || isLoadingArchive || isLoadingFinishAndArchive}
loading={isMutatingOrLoading}
tableCaption="Maintenance Windows List"
items={items}
columns={columns}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('TableActionsPopover', () => {
const result = appMockRenderer.render(
<TableActionsPopover
id={'123'}
isLoading={false}
status={MaintenanceWindowStatus.Running}
onEdit={() => {}}
onCancel={() => {}}
Expand All @@ -39,6 +40,7 @@ describe('TableActionsPopover', () => {
const result = appMockRenderer.render(
<TableActionsPopover
id={'123'}
isLoading={false}
status={MaintenanceWindowStatus.Running}
onEdit={() => {}}
onCancel={() => {}}
Expand All @@ -56,6 +58,7 @@ describe('TableActionsPopover', () => {
const result = appMockRenderer.render(
<TableActionsPopover
id={'123'}
isLoading={false}
status={MaintenanceWindowStatus.Upcoming}
onEdit={() => {}}
onCancel={() => {}}
Expand All @@ -72,6 +75,7 @@ describe('TableActionsPopover', () => {
const result = appMockRenderer.render(
<TableActionsPopover
id={'123'}
isLoading={false}
status={MaintenanceWindowStatus.Finished}
onEdit={() => {}}
onCancel={() => {}}
Expand All @@ -88,6 +92,7 @@ describe('TableActionsPopover', () => {
const result = appMockRenderer.render(
<TableActionsPopover
id={'123'}
isLoading={false}
status={MaintenanceWindowStatus.Archived}
onEdit={() => {}}
onCancel={() => {}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { MaintenanceWindowStatus } from '../../../../common';
export interface TableActionsPopoverProps {
id: string;
status: MaintenanceWindowStatus;
isLoading: boolean;
onEdit: (id: string) => void;
onCancel: (id: string) => void;
onArchive: (id: string, archive: boolean) => void;
Expand All @@ -30,7 +31,7 @@ type ModalType = 'cancel' | 'cancelAndArchive' | 'archive' | 'unarchive';
type ActionType = ModalType | 'edit';

export const TableActionsPopover: React.FC<TableActionsPopoverProps> = React.memo(
({ id, status, onEdit, onCancel, onArchive, onCancelAndArchive }) => {
({ id, status, isLoading, onEdit, onCancel, onArchive, onCancelAndArchive }) => {
const [isPopoverOpen, setIsPopoverOpen] = useState(false);
const [isModalVisible, setIsModalVisible] = useState(false);
const [modalType, setModalType] = useState<ModalType>();
Expand Down Expand Up @@ -197,14 +198,15 @@ export const TableActionsPopover: React.FC<TableActionsPopoverProps> = React.mem
const button = useMemo(
() => (
<EuiButtonIcon
isDisabled={isLoading}
data-test-subj="table-actions-icon-button"
iconType="boxesHorizontal"
size="s"
aria-label="Upcoming events"
onClick={onButtonClick}
/>
),
[onButtonClick]
[isLoading, onButtonClick]
);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const MaintenanceWindowsPage = React.memo(() => {

const { navigateToCreateMaintenanceWindow } = useCreateMaintenanceWindowNavigation();

const { isLoading, maintenanceWindows, refetch } = useFindMaintenanceWindows({
const { isLoading, isInitialLoading, maintenanceWindows, refetch } = useFindMaintenanceWindows({
enabled: hasLicense,
});

Expand Down Expand Up @@ -81,7 +81,7 @@ export const MaintenanceWindowsPage = React.memo(() => {
};
}, [setBadge, chrome]);

if (isLoading) {
if (isInitialLoading) {
return <CenterJustifiedSpinner />;
}

Expand Down Expand Up @@ -127,7 +127,7 @@ export const MaintenanceWindowsPage = React.memo(() => {
<MaintenanceWindowsList
readOnly={readOnly}
refreshData={refreshData}
loading={isLoading}
isLoading={isLoading}
items={maintenanceWindows}
/>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
let objectRemover: ObjectRemover;
const browser = getService('browser');

// Failing: See https://github.com/elastic/kibana/issues/187818
describe.skip('Maintenance windows table', function () {
describe('Maintenance windows table', function () {
before(async () => {
objectRemover = await createObjectRemover({ getService });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
await security.testUser.setRoles(['alerts_and_actions_role']);
});

after(async () => {
await security.testUser.restoreDefaults();
});

it('Loads the page', async () => {
await pageObjects.common.navigateToUrl(
'management',
Expand Down Expand Up @@ -77,6 +81,10 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
await security.testUser.setRoles(['only_actions_role']);
});

after(async () => {
await security.testUser.restoreDefaults();
});

it('Loads the page but shows missing permission prompt', async () => {
await pageObjects.common.navigateToUrl(
'management',
Expand All @@ -103,6 +111,10 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
);
});

after(async () => {
await security.testUser.restoreDefaults();
});

it('Loads the page', async () => {
log.debug('Checking for section heading to say Alerts.');

Expand Down

0 comments on commit 8b2cb75

Please sign in to comment.