Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cases] Fix property actions flaky tests #176709

Merged
merged 4 commits into from
Feb 13, 2024
Merged

[Cases] Fix property actions flaky tests #176709

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6813468
fix property actions flaky tests
js-jankisalvi Feb 12, 2024
e886e19
remove loop
js-jankisalvi Feb 12, 2024
2c86c2e
PR feedback
js-jankisalvi Feb 13, 2024
0d8cb79
remove waitForPopOverOpen, check for element directly
js-jankisalvi Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 2 additions & 19 deletions ...ins/cases/public/components/user_actions/property_actions/alert_property_actions.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ import {
} from '../../../common/mock';
import { AlertPropertyActions } from './alert_property_actions';

// FLAKY: https://github.com/elastic/kibana/issues/174667
describe.skip('AlertPropertyActions', () => {
describe('AlertPropertyActions', () => {
let appMock: AppMockRenderer;

const props = {
Expand Down Expand Up @@ -49,22 +48,6 @@ describe.skip('AlertPropertyActions', () => {
).toBeInTheDocument();
});

it('renders the modal info correctly for one alert', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed this test as it is already covered in remove alerts correctly

appMock.render(<AlertPropertyActions {...props} />);

expect(await screen.findByTestId('property-actions-user-action')).toBeInTheDocument();

userEvent.click(await screen.findByTestId('property-actions-user-action-ellipses'));
await waitForEuiPopoverOpen();

userEvent.click(await screen.findByTestId('property-actions-user-action-minusInCircle'));

expect(await screen.findByTestId('property-actions-confirm-modal')).toBeInTheDocument();

expect(await screen.findByTestId('confirmModalTitleText')).toHaveTextContent('Remove alert');
expect(await screen.findByText('Remove')).toBeInTheDocument();
});

it('renders the modal info correctly for multiple alert', async () => {
appMock.render(<AlertPropertyActions {...props} totalAlerts={2} />);

Expand Down Expand Up @@ -93,7 +76,7 @@ describe.skip('AlertPropertyActions', () => {

expect(await screen.findByTestId('property-actions-confirm-modal')).toBeInTheDocument();

userEvent.click(screen.getByText('Remove'));
userEvent.click(await screen.findByText('Remove'));

await waitFor(() => {
expect(props.onDelete).toHaveBeenCalled();
Expand Down
3 changes: 1 addition & 2 deletions ...components/user_actions/property_actions/registered_attachments_property_actions.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ import {
import { RegisteredAttachmentsPropertyActions } from './registered_attachments_property_actions';
import { AttachmentActionType } from '../../../client/attachment_framework/types';

// FLAKY: https://github.com/elastic/kibana/issues/174384
describe.skip('RegisteredAttachmentsPropertyActions', () => {
describe('RegisteredAttachmentsPropertyActions', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything we can do on this test suite to improve flakiness?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked but couldn't find anything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test that fails is called renders the correct number of actions with error Error: thrown: "Exceeded timeout of 5000 ms for a test.. Probably the waitForEuiPopoverOpen took time (it is the only one with a waitFor inside). I am wondering what if we remove it and wait for the component we expecting. Example: expect((await screen.findByTestId('property-actions-user-action-group')).toBeInTheDocument(). I am not sure if this will fix the flaky test. It is just a speculation. Also,

expect((await screen.findByTestId('property-actions-user-action-group')).children.length).toBe(
      1
    );

can be substituted by expect((await screen.findByTestId('property-actions-user-action-group')).toBeInTheDocument() right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm.. I could give it a try.

let appMock: AppMockRenderer;

const props = {
Expand Down
67 changes: 32 additions & 35 deletions ...es/public/components/user_actions/property_actions/user_comment_property_actions.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import React from 'react';
import { waitForEuiPopoverOpen } from '@elastic/eui/lib/test/rtl';
import { waitForEuiPopoverOpen, screen } from '@elastic/eui/lib/test/rtl';
import userEvent from '@testing-library/user-event';
import type { AppMockRenderer } from '../../../common/mock';
import {
Expand All @@ -17,12 +17,7 @@ import {
import { UserCommentPropertyActions } from './user_comment_property_actions';
import { waitFor } from '@testing-library/react';

// FLAKY: https://github.com/elastic/kibana/issues/175310
// FLAKY: https://github.com/elastic/kibana/issues/175311
// FLAKY: https://github.com/elastic/kibana/issues/175312
// FLAKY: https://github.com/elastic/kibana/issues/175313
// FLAKY: https://github.com/elastic/kibana/issues/175314
describe.skip('UserCommentPropertyActions', () => {
describe('UserCommentPropertyActions', () => {
let appMock: AppMockRenderer;

const props = {
Expand All @@ -38,80 +33,82 @@ describe.skip('UserCommentPropertyActions', () => {
});

it('renders the correct number of actions', async () => {
const result = appMock.render(<UserCommentPropertyActions {...props} />);
appMock.render(<UserCommentPropertyActions {...props} />);

expect(result.getByTestId('property-actions-user-action')).toBeInTheDocument();
expect(await screen.findByTestId('property-actions-user-action')).toBeInTheDocument();

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

expect(result.getByTestId('property-actions-user-action-group').children.length).toBe(3);
expect(result.queryByTestId('property-actions-user-action-pencil')).toBeInTheDocument();
expect(result.queryByTestId('property-actions-user-action-trash')).toBeInTheDocument();
expect(result.queryByTestId('property-actions-user-action-quote')).toBeInTheDocument();
expect((await screen.findByTestId('property-actions-user-action-group')).children.length).toBe(
3
);
expect(screen.queryByTestId('property-actions-user-action-pencil')).toBeInTheDocument();
expect(screen.queryByTestId('property-actions-user-action-trash')).toBeInTheDocument();
expect(screen.queryByTestId('property-actions-user-action-quote')).toBeInTheDocument();
});

it('edits the comment correctly', async () => {
const result = appMock.render(<UserCommentPropertyActions {...props} />);
appMock.render(<UserCommentPropertyActions {...props} />);

expect(result.getByTestId('property-actions-user-action')).toBeInTheDocument();
expect(await screen.findByTestId('property-actions-user-action')).toBeInTheDocument();

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

expect(result.queryByTestId('property-actions-user-action-pencil')).toBeInTheDocument();
expect(screen.queryByTestId('property-actions-user-action-pencil')).toBeInTheDocument();

userEvent.click(result.getByTestId('property-actions-user-action-pencil'));
userEvent.click(await screen.findByTestId('property-actions-user-action-pencil'));

expect(props.onEdit).toHaveBeenCalled();
});

it('quotes the comment correctly', async () => {
const result = appMock.render(<UserCommentPropertyActions {...props} />);
appMock.render(<UserCommentPropertyActions {...props} />);

expect(result.getByTestId('property-actions-user-action')).toBeInTheDocument();
expect(await screen.findByTestId('property-actions-user-action')).toBeInTheDocument();

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

expect(result.queryByTestId('property-actions-user-action-quote')).toBeInTheDocument();
expect(screen.queryByTestId('property-actions-user-action-quote')).toBeInTheDocument();

userEvent.click(result.getByTestId('property-actions-user-action-quote'));
userEvent.click(await screen.findByTestId('property-actions-user-action-quote'));

expect(props.onQuote).toHaveBeenCalled();
});

it('deletes the comment correctly', async () => {
const result = appMock.render(<UserCommentPropertyActions {...props} />);
appMock.render(<UserCommentPropertyActions {...props} />);

expect(result.getByTestId('property-actions-user-action')).toBeInTheDocument();
expect(await screen.findByTestId('property-actions-user-action')).toBeInTheDocument();

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

expect(result.queryByTestId('property-actions-user-action-trash')).toBeInTheDocument();
expect(screen.queryByTestId('property-actions-user-action-trash')).toBeInTheDocument();

userEvent.click(result.getByTestId('property-actions-user-action-trash'));
userEvent.click(await screen.findByTestId('property-actions-user-action-trash'));

await waitFor(() => {
expect(result.queryByTestId('property-actions-confirm-modal')).toBeInTheDocument();
expect(screen.queryByTestId('property-actions-confirm-modal')).toBeInTheDocument();
});

userEvent.click(result.getByText('Delete'));
userEvent.click(await screen.findByText('Delete'));
expect(props.onDelete).toHaveBeenCalled();
});

it('does not show the property actions without delete permissions', async () => {
appMock = createAppMockRenderer({ permissions: noCasesPermissions() });
const result = appMock.render(<UserCommentPropertyActions {...props} />);
appMock.render(<UserCommentPropertyActions {...props} />);

expect(result.queryByTestId('property-actions-user-action')).not.toBeInTheDocument();
expect(screen.queryByTestId('property-actions-user-action')).not.toBeInTheDocument();
});

it('does show the property actions with only delete permissions', async () => {
appMock = createAppMockRenderer({ permissions: onlyDeleteCasesPermission() });
const result = appMock.render(<UserCommentPropertyActions {...props} />);
appMock.render(<UserCommentPropertyActions {...props} />);

expect(result.getByTestId('property-actions-user-action')).toBeInTheDocument();
expect(await screen.findByTestId('property-actions-user-action')).toBeInTheDocument();
});
});