Skip to content

Commit

Permalink
[Cases] Improve functional tests (#150117)
Browse files Browse the repository at this point in the history
This PR tries to improve how often our functional tests succeeds. I also
tried cleaning up a few things that seemed to be slowing the tests down
and also causing errors when the tests were run individually.

Fixes: #145271

Notable changes:
- I added a value to the `property-actions*` in most places so that the
functional tests can distinguish between a description, comment, or the
case ellipses this seems to work consistently where other methods have
not

Flaky test run:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1871
  • Loading branch information
jonathan-buttner authored Feb 6, 2023
1 parent 9a3e319 commit c3ea5e5
Show file tree
Hide file tree
Showing 18 changed files with 263 additions and 226 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,11 @@ describe('CaseView actions', () => {
);

expect(wrapper.find('[data-test-subj="confirm-delete-case-modal"]').exists()).toBeFalsy();
wrapper.find('button[data-test-subj="property-actions-ellipses"]').first().simulate('click');
wrapper.find('button[data-test-subj="property-actions-trash"]').simulate('click');
wrapper
.find('button[data-test-subj="property-actions-case-ellipses"]')
.first()
.simulate('click');
wrapper.find('button[data-test-subj="property-actions-case-trash"]').simulate('click');
expect(wrapper.find('[data-test-subj="confirm-delete-case-modal"]').exists()).toBeTruthy();
});

Expand All @@ -67,8 +70,11 @@ describe('CaseView actions', () => {
</TestProviders>
);

wrapper.find('button[data-test-subj="property-actions-ellipses"]').first().simulate('click');
wrapper.find('button[data-test-subj="property-actions-copyClipboard"]').simulate('click');
wrapper
.find('button[data-test-subj="property-actions-case-ellipses"]')
.first()
.simulate('click');
wrapper.find('button[data-test-subj="property-actions-case-copyClipboard"]').simulate('click');

expect(navigator.clipboard.writeText).toHaveBeenCalledWith(basicCase.id);

Expand All @@ -85,9 +91,14 @@ describe('CaseView actions', () => {
);

expect(wrapper.find('[data-test-subj="confirm-delete-case-modal"]').exists()).toBeFalsy();
wrapper.find('button[data-test-subj="property-actions-ellipses"]').first().simulate('click');
expect(wrapper.find('[data-test-subj="property-actions-trash"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="property-actions-copyClipboard"]').exists()).toBeTruthy();
wrapper
.find('button[data-test-subj="property-actions-case-ellipses"]')
.first()
.simulate('click');
expect(wrapper.find('[data-test-subj="property-actions-case-trash"]').exists()).toBeFalsy();
expect(
wrapper.find('[data-test-subj="property-actions-case-copyClipboard"]').exists()
).toBeTruthy();
});

it('toggle delete modal and confirm', async () => {
Expand All @@ -101,8 +112,11 @@ describe('CaseView actions', () => {
</TestProviders>
);

wrapper.find('button[data-test-subj="property-actions-ellipses"]').first().simulate('click');
wrapper.find('button[data-test-subj="property-actions-trash"]').simulate('click');
wrapper
.find('button[data-test-subj="property-actions-case-ellipses"]')
.first()
.simulate('click');
wrapper.find('button[data-test-subj="property-actions-case-trash"]').simulate('click');

expect(wrapper.find('[data-test-subj="confirm-delete-case-modal"]').exists()).toBeTruthy();
wrapper.find('button[data-test-subj="confirmModalConfirmButton"]').simulate('click');
Expand All @@ -126,9 +140,12 @@ describe('CaseView actions', () => {

expect(wrapper.find('[data-test-subj="confirm-delete-case-modal"]').exists()).toBeFalsy();

wrapper.find('button[data-test-subj="property-actions-ellipses"]').first().simulate('click');
wrapper
.find('button[data-test-subj="property-actions-case-ellipses"]')
.first()
.simulate('click');
expect(
wrapper.find('[data-test-subj="property-actions-popout"]').first().prop('aria-label')
wrapper.find('[data-test-subj="property-actions-case-popout"]').first().prop('aria-label')
).toEqual(i18n.VIEW_INCIDENT(basicPush.externalTitle));
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const ActionsComponent: React.FC<CaseViewActions> = ({ caseData, currentExternal

return (
<EuiFlexItem grow={false} data-test-subj="case-view-actions">
<PropertyActions propertyActions={propertyActions} />
<PropertyActions propertyActions={propertyActions} customDataTestSubj={'case'} />
{isModalVisible ? (
<ConfirmDeleteCaseModal
totalCasesToBeDeleted={1}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,10 @@ describe('CaseActionBar', () => {
</TestProviders>
);

userEvent.click(screen.getByTestId('property-actions-ellipses'));
userEvent.click(screen.getByTestId('property-actions-case-ellipses'));
expect(queryByText('Delete case')).not.toBeInTheDocument();
expect(queryByTestId('property-actions-trash')).not.toBeInTheDocument();
expect(queryByTestId('property-actions-copyClipboard')).toBeInTheDocument();
expect(queryByTestId('property-actions-case-trash')).not.toBeInTheDocument();
expect(queryByTestId('property-actions-case-copyClipboard')).toBeInTheDocument();
});

it('should show the the delete item in the menu when the user does have delete privileges', () => {
Expand All @@ -220,7 +220,7 @@ describe('CaseActionBar', () => {
</TestProviders>
);

userEvent.click(screen.getByTestId('property-actions-ellipses'));
userEvent.click(screen.getByTestId('property-actions-case-ellipses'));
expect(queryByText('Delete case')).toBeInTheDocument();
});

Expand All @@ -239,10 +239,10 @@ describe('CaseActionBar', () => {
</TestProviders>
);

userEvent.click(screen.getByTestId('property-actions-ellipses'));
userEvent.click(screen.getByTestId('property-actions-case-ellipses'));

await waitFor(() => {
expect(screen.getByTestId('property-actions-popout')).toBeInTheDocument();
expect(screen.getByTestId('property-actions-case-popout')).toBeInTheDocument();
});
});

Expand All @@ -253,8 +253,8 @@ describe('CaseActionBar', () => {
</TestProviders>
);

userEvent.click(screen.getByTestId('property-actions-ellipses'));
userEvent.click(screen.getByTestId('property-actions-case-ellipses'));

expect(screen.queryByTestId('property-actions-popout')).not.toBeInTheDocument();
expect(screen.queryByTestId('property-actions-case-popout')).not.toBeInTheDocument();
});
});
143 changes: 79 additions & 64 deletions x-pack/plugins/cases/public/components/property_actions/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,86 +17,101 @@ export interface PropertyActionButtonProps {
iconType: string;
label: string;
color?: EuiButtonProps['color'];
customDataTestSubj?: string;
}

const ComponentId = 'property-actions';

const PropertyActionButton = React.memo<PropertyActionButtonProps>(
({ disabled = false, onClick, iconType, label, color }) => (
<EuiButtonEmpty
aria-label={label}
color={color ? color : 'text'}
data-test-subj={`${ComponentId}-${iconType}`}
iconSide="left"
iconType={iconType}
isDisabled={disabled}
onClick={onClick}
>
{label}
</EuiButtonEmpty>
)
({ disabled = false, onClick, iconType, label, color, customDataTestSubj }) => {
const dataTestSubjPrepend = makeDataTestSubjPrepend(customDataTestSubj);

return (
<EuiButtonEmpty
aria-label={label}
color={color ? color : 'text'}
data-test-subj={`${dataTestSubjPrepend}-${iconType}`}
iconSide="left"
iconType={iconType}
isDisabled={disabled}
onClick={onClick}
>
{label}
</EuiButtonEmpty>
);
}
);

PropertyActionButton.displayName = 'PropertyActionButton';

export interface PropertyActionsProps {
propertyActions: PropertyActionButtonProps[];
customDataTestSubj?: string;
}

export const PropertyActions = React.memo<PropertyActionsProps>(({ propertyActions }) => {
const [showActions, setShowActions] = useState(false);
export const PropertyActions = React.memo<PropertyActionsProps>(
({ propertyActions, customDataTestSubj }) => {
const [showActions, setShowActions] = useState(false);

const onButtonClick = useCallback(() => {
setShowActions((prevShowActions) => !prevShowActions);
}, []);
const onButtonClick = useCallback(() => {
setShowActions((prevShowActions) => !prevShowActions);
}, []);

const onClosePopover = useCallback((cb?: () => void) => {
setShowActions(false);
if (cb != null) {
cb();
}
}, []);

return (
<EuiPopover
anchorPosition="downRight"
data-test-subj={ComponentId}
ownFocus
button={
<EuiButtonIcon
data-test-subj={`${ComponentId}-ellipses`}
aria-label={i18n.ACTIONS_ARIA}
iconType="boxesHorizontal"
onClick={onButtonClick}
/>
const onClosePopover = useCallback((cb?: () => void) => {
setShowActions(false);
if (cb != null) {
cb();
}
id="settingsPopover"
isOpen={showActions}
closePopover={onClosePopover}
repositionOnScroll
>
<EuiFlexGroup
alignItems="flexStart"
data-test-subj={`${ComponentId}-group`}
direction="column"
gutterSize="none"
}, []);

const dataTestSubjPrepend = makeDataTestSubjPrepend(customDataTestSubj);

return (
<EuiPopover
anchorPosition="downRight"
data-test-subj={dataTestSubjPrepend}
ownFocus
button={
<EuiButtonIcon
data-test-subj={`${dataTestSubjPrepend}-ellipses`}
aria-label={i18n.ACTIONS_ARIA}
iconType="boxesHorizontal"
onClick={onButtonClick}
/>
}
id="settingsPopover"
isOpen={showActions}
closePopover={onClosePopover}
repositionOnScroll
>
{propertyActions.map((action, key) => (
<EuiFlexItem grow={false} key={`${action.label}${key}`}>
<span>
<PropertyActionButton
disabled={action.disabled}
iconType={action.iconType}
label={action.label}
color={action.color}
onClick={() => onClosePopover(action.onClick)}
/>
</span>
</EuiFlexItem>
))}
</EuiFlexGroup>
</EuiPopover>
);
});
<EuiFlexGroup
alignItems="flexStart"
data-test-subj={`${dataTestSubjPrepend}-group`}
direction="column"
gutterSize="none"
>
{propertyActions.map((action, key) => (
<EuiFlexItem grow={false} key={`${action.label}${key}`}>
<span>
<PropertyActionButton
disabled={action.disabled}
iconType={action.iconType}
label={action.label}
color={action.color}
onClick={() => onClosePopover(action.onClick)}
customDataTestSubj={customDataTestSubj}
/>
</span>
</EuiFlexItem>
))}
</EuiFlexGroup>
</EuiPopover>
);
}
);

PropertyActions.displayName = 'PropertyActions';

const makeDataTestSubjPrepend = (customDataTestSubj?: string) => {
return customDataTestSubj == null ? ComponentId : `${ComponentId}-${customDataTestSubj}`;
};
Original file line number Diff line number Diff line change
Expand Up @@ -223,13 +223,13 @@ describe('createCommentUserActionBuilder', () => {
);

expect(result.getByText('Solve this fast!')).toBeInTheDocument();
expect(result.getByTestId('property-actions')).toBeInTheDocument();
expect(result.getByTestId('property-actions-user-action')).toBeInTheDocument();

userEvent.click(result.getByTestId('property-actions-ellipses'));
userEvent.click(result.getByTestId('property-actions-user-action-ellipses'));
await waitForEuiPopoverOpen();

expect(result.queryByTestId('property-actions-pencil')).toBeInTheDocument();
userEvent.click(result.getByTestId('property-actions-pencil'));
expect(result.queryByTestId('property-actions-user-action-pencil')).toBeInTheDocument();
userEvent.click(result.getByTestId('property-actions-user-action-pencil'));

await waitFor(() => {
expect(builderArgs.handleManageMarkdownEditId).toHaveBeenCalledWith('basic-comment-id');
Expand All @@ -254,13 +254,13 @@ describe('createCommentUserActionBuilder', () => {
);

expect(result.getByText('Solve this fast!')).toBeInTheDocument();
expect(result.getByTestId('property-actions')).toBeInTheDocument();
expect(result.getByTestId('property-actions-user-action')).toBeInTheDocument();

userEvent.click(result.getByTestId('property-actions-ellipses'));
userEvent.click(result.getByTestId('property-actions-user-action-ellipses'));
await waitForEuiPopoverOpen();

expect(result.queryByTestId('property-actions-quote')).toBeInTheDocument();
userEvent.click(result.getByTestId('property-actions-quote'));
expect(result.queryByTestId('property-actions-user-action-quote')).toBeInTheDocument();
userEvent.click(result.getByTestId('property-actions-user-action-quote'));

await waitFor(() => {
expect(builderArgs.handleManageQuote).toHaveBeenCalledWith('Solve this fast!');
Expand Down Expand Up @@ -769,14 +769,14 @@ describe('createCommentUserActionBuilder', () => {
});

const deleteAttachment = async (result: RenderResult, deleteIcon: string, buttonLabel: string) => {
expect(result.getByTestId('property-actions')).toBeInTheDocument();
expect(result.getByTestId('property-actions-user-action')).toBeInTheDocument();

userEvent.click(result.getByTestId('property-actions-ellipses'));
userEvent.click(result.getByTestId('property-actions-user-action-ellipses'));
await waitForEuiPopoverOpen();

expect(result.queryByTestId(`property-actions-${deleteIcon}`)).toBeInTheDocument();
expect(result.queryByTestId(`property-actions-user-action-${deleteIcon}`)).toBeInTheDocument();

userEvent.click(result.getByTestId(`property-actions-${deleteIcon}`));
userEvent.click(result.getByTestId(`property-actions-user-action-${deleteIcon}`));

await waitFor(() => {
expect(result.queryByTestId('property-actions-confirm-modal')).toBeInTheDocument();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,13 @@ describe('createDescriptionUserActionBuilder ', () => {
</TestProviders>
);

expect(res.getByTestId('property-actions')).toBeInTheDocument();
expect(res.getByTestId('property-actions-description')).toBeInTheDocument();

userEvent.click(res.getByTestId('property-actions-ellipses'));
userEvent.click(res.getByTestId('property-actions-description-ellipses'));
await waitForEuiPopoverOpen();

expect(res.queryByTestId('property-actions-pencil')).toBeInTheDocument();
userEvent.click(res.getByTestId('property-actions-pencil'));
expect(res.queryByTestId('property-actions-description-pencil')).toBeInTheDocument();
userEvent.click(res.getByTestId('property-actions-description-pencil'));

await waitFor(() => {
expect(builderArgs.handleManageMarkdownEditId).toHaveBeenCalledWith('description');
Expand All @@ -84,13 +84,13 @@ describe('createDescriptionUserActionBuilder ', () => {
</TestProviders>
);

expect(res.getByTestId('property-actions')).toBeInTheDocument();
expect(res.getByTestId('property-actions-description')).toBeInTheDocument();

userEvent.click(res.getByTestId('property-actions-ellipses'));
userEvent.click(res.getByTestId('property-actions-description-ellipses'));
await waitForEuiPopoverOpen();

expect(res.queryByTestId('property-actions-quote')).toBeInTheDocument();
userEvent.click(res.getByTestId('property-actions-quote'));
expect(res.queryByTestId('property-actions-description-quote')).toBeInTheDocument();
userEvent.click(res.getByTestId('property-actions-description-quote'));

await waitFor(() => {
expect(builderArgs.handleManageQuote).toHaveBeenCalledWith('Security banana Issue');
Expand Down
Loading

0 comments on commit c3ea5e5

Please sign in to comment.