Skip to content

Commit

Permalink
[Cases] Fix flakiness in all_cases_list.test.tsx (#157285)
Browse files Browse the repository at this point in the history 
## Summary

This PR fixes the flaky tests in
`x-pack/plugins/cases/public/components/all_cases/all_cases_list.test.tsx`.
It modifies the `getComputedStyle` to be more performant. For more
information see
https://blog.sentry.io/sentrys-frontend-tests-migrating-from-enzyme-to-react-testing-library
and jsdom/jsdom#3234.

I run the test multiple times on CI to ensure that the test is not flaky
anymore: Specifically:

- Test run 50 times in
ccf2716
- Test run 40 times in
c56c650
- Test run 50 times in
b04d613
- Test run 50 times in
e0ef292
- Test run 55 times in
1ba6bac

Fixes: #148096,
#150923

### Checklist

Delete any items that are not applicable to this PR.

- [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

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
@cnasikas @kibanamachine @jasonrhodes
2 people authored and jasonrhodes committed May 17, 2023
1 parent d160ed1 commit 46c3566
Showing 1 changed file with 94 additions and 102 deletions.
196 changes: 94 additions & 102 deletions x-pack/plugins/cases/public/components/all_cases/all_cases_list.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ const mockKibana = () => {
} as unknown as ReturnType<typeof useKibana>);
};

// FLAKY: https://github.com/elastic/kibana/issues/150923
describe.skip('AllCasesListGeneric', () => {
describe('AllCasesListGeneric', () => {
const refetchCases = jest.fn();
const onRowClick = jest.fn();
const updateCaseProperty = jest.fn();
Expand All @@ -101,15 +100,50 @@ describe.skip('AllCasesListGeneric', () => {
};

const removeMsFromDate = (value: string) => moment(value).format('YYYY-MM-DDTHH:mm:ss[Z]');
// eslint-disable-next-line prefer-object-spread
const originalGetComputedStyle = Object.assign({}, window.getComputedStyle);

let appMockRenderer: AppMockRenderer;

beforeAll(() => {
// The JSDOM implementation is too slow
// Especially for dropdowns that try to position themselves
// perf issue - https://github.com/jsdom/jsdom/issues/3234
Object.defineProperty(window, 'getComputedStyle', {
value: (el: HTMLElement) => {
/**
* This is based on the jsdom implementation of getComputedStyle
* https://github.com/jsdom/jsdom/blob/9dae17bf0ad09042cfccd82e6a9d06d3a615d9f4/lib/jsdom/browser/Window.js#L779-L820
*
* It is missing global style parsing and will only return styles applied directly to an element.
* Will not return styles that are global or from emotion
*/
const declaration = new CSSStyleDeclaration();
const { style } = el;

Array.prototype.forEach.call(style, (property: string) => {
declaration.setProperty(
property,
style.getPropertyValue(property),
style.getPropertyPriority(property)
);
});

return declaration;
},
configurable: true,
writable: true,
});

mockKibana();
const actionTypeRegistry = useKibanaMock().services.triggersActionsUi.actionTypeRegistry;
registerConnectorsToMockActionRegistry(actionTypeRegistry, connectorsMock);
});

afterAll(() => {
Object.defineProperty(window, 'getComputedStyle', originalGetComputedStyle);
});

beforeEach(() => {
jest.clearAllMocks();
appMockRenderer = createAppMockRenderer();
Expand Down Expand Up @@ -714,19 +748,13 @@ describe.skip('AllCasesListGeneric', () => {
appMockRenderer.render(<AllCasesList />);

await waitFor(() => {
expect(screen.getByTestId('checkboxSelectAll')).toBeInTheDocument();
expect(screen.getByTestId('cases-table')).toBeInTheDocument();
});

userEvent.click(screen.getByTestId('checkboxSelectAll'));

await waitFor(() => {
expect(screen.getByText('Bulk actions')).toBeInTheDocument();
});

expect(screen.getByText('Bulk actions')).toBeInTheDocument();
userEvent.click(screen.getByText('Bulk actions'));

await waitForEuiPopoverOpen();

expect(screen.getByTestId('case-bulk-action-status')).toBeInTheDocument();
expect(screen.getByTestId('cases-bulk-action-delete')).toBeInTheDocument();
});
Expand All @@ -737,37 +765,35 @@ describe.skip('AllCasesListGeneric', () => {
appMockRenderer.render(<AllCasesList />);

await waitFor(() => {
expect(screen.getByTestId('checkboxSelectAll')).toBeInTheDocument();
expect(screen.getByTestId('cases-table')).toBeInTheDocument();
});

userEvent.click(screen.getByTestId('checkboxSelectAll'));

await waitFor(() => {
expect(screen.getByText('Bulk actions')).toBeInTheDocument();
});
expect(screen.getByText('Bulk actions')).toBeInTheDocument();

userEvent.click(screen.getByText('Bulk actions'));

await waitForEuiPopoverOpen();

userEvent.click(screen.getByTestId('case-bulk-action-status'));
userEvent.click(screen.getByTestId('case-bulk-action-status'), undefined, {
skipPointerEventsCheck: true,
});

await waitFor(() => {
expect(screen.getByTestId(`cases-bulk-action-status-${status}`)).toBeInTheDocument();
});

userEvent.click(screen.getByTestId(`cases-bulk-action-status-${status}`));

await waitForComponentToUpdate();

expect(updateCasesSpy).toBeCalledWith(
useGetCasesMockState.data.cases.map(({ id, version }) => ({
id,
version,
status,
})),
expect.anything()
);
await waitFor(() => {
expect(updateCasesSpy).toBeCalledWith(
useGetCasesMockState.data.cases.map(({ id, version }) => ({
id,
version,
status,
})),
expect.anything()
);
});
}
);

Expand All @@ -780,63 +806,55 @@ describe.skip('AllCasesListGeneric', () => {
appMockRenderer.render(<AllCasesList />);

await waitFor(() => {
expect(screen.getByTestId('checkboxSelectAll')).toBeInTheDocument();
expect(screen.getByTestId('cases-table')).toBeInTheDocument();
});

userEvent.click(screen.getByTestId('checkboxSelectAll'));

await waitFor(() => {
expect(screen.getByText('Bulk actions')).toBeInTheDocument();
});
expect(screen.getByText('Bulk actions')).toBeInTheDocument();

userEvent.click(screen.getByText('Bulk actions'));

await waitForEuiPopoverOpen();

userEvent.click(screen.getByTestId('case-bulk-action-severity'));
userEvent.click(screen.getByTestId('case-bulk-action-severity'), undefined, {
skipPointerEventsCheck: true,
});

await waitFor(() => {
expect(screen.getByTestId(`cases-bulk-action-severity-${severity}`)).toBeInTheDocument();
});

userEvent.click(screen.getByTestId(`cases-bulk-action-severity-${severity}`));

await waitForComponentToUpdate();

expect(updateCasesSpy).toBeCalledWith(
useGetCasesMockState.data.cases.map(({ id, version }) => ({
id,
version,
severity,
})),
expect.anything()
);
await waitFor(() => {
expect(updateCasesSpy).toBeCalledWith(
useGetCasesMockState.data.cases.map(({ id, version }) => ({
id,
version,
severity,
})),
expect.anything()
);
});
});

it('Bulk delete', async () => {
appMockRenderer.render(<AllCasesList />);

await waitFor(() => {
expect(screen.getByTestId('checkboxSelectAll')).toBeInTheDocument();
expect(screen.getByTestId('cases-table')).toBeInTheDocument();
});

userEvent.click(screen.getByTestId('checkboxSelectAll'));

await waitFor(() => {
expect(screen.getByText('Bulk actions')).toBeInTheDocument();
});
expect(screen.getByText('Bulk actions')).toBeInTheDocument();

userEvent.click(screen.getByText('Bulk actions'));

await waitForEuiPopoverOpen();

userEvent.click(screen.getByTestId('cases-bulk-action-delete'), undefined, {
skipPointerEventsCheck: true,
});

await waitFor(() => {
expect(screen.getByTestId('confirm-delete-case-modal')).toBeInTheDocument();
});
expect(screen.getByTestId('confirm-delete-case-modal')).toBeInTheDocument();

userEvent.click(screen.getByTestId('confirmModalConfirmButton'));

Expand All @@ -861,17 +879,13 @@ describe.skip('AllCasesListGeneric', () => {
appMockRenderer = createAppMockRenderer({ permissions: readCasesPermissions() });
appMockRenderer.render(<AllCasesList />);

await waitFor(() => {
expect(screen.getByTestId('checkboxSelectAll')).toBeInTheDocument();
});

expect(screen.getByTestId('checkboxSelectAll')).toBeDisabled();

await waitFor(() => {
for (const theCase of defaultGetCases.data.cases) {
for (const theCase of defaultGetCases.data.cases) {
await waitFor(() => {
expect(screen.getByTestId(`checkboxSelectRow-${theCase.id}`)).toBeDisabled();
}
});
});
}
});
});

Expand All @@ -892,13 +906,13 @@ describe.skip('AllCasesListGeneric', () => {
it('should render row actions', async () => {
appMockRenderer.render(<AllCasesList />);

await waitFor(() => {
for (const theCase of defaultGetCases.data.cases) {
for (const theCase of defaultGetCases.data.cases) {
await waitFor(() => {
expect(
screen.getByTestId(`case-action-popover-button-${theCase.id}`)
).toBeInTheDocument();
}
});
});
}
});

it.each(statusTests)('update the status of a case: %s', async (status) => {
Expand All @@ -907,17 +921,11 @@ describe.skip('AllCasesListGeneric', () => {
const inProgressCase = useGetCasesMockState.data.cases[1];
const theCase = status === CaseStatuses.open ? inProgressCase : openCase;

await waitFor(() => {
expect(
screen.getByTestId(`case-action-popover-button-${theCase.id}`)
).toBeInTheDocument();
});
expect(screen.getByTestId(`case-action-popover-button-${theCase.id}`)).toBeInTheDocument();

userEvent.click(screen.getByTestId(`case-action-popover-button-${theCase.id}`));

await waitFor(() => {
expect(screen.getByTestId(`case-action-status-panel-${theCase.id}`)).toBeInTheDocument();
});
expect(screen.getByTestId(`case-action-status-panel-${theCase.id}`)).toBeInTheDocument();

userEvent.click(screen.getByTestId(`case-action-status-panel-${theCase.id}`), undefined, {
skipPointerEventsCheck: true,
Expand All @@ -943,19 +951,11 @@ describe.skip('AllCasesListGeneric', () => {
const mediumCase = useGetCasesMockState.data.cases[1];
const theCase = severity === CaseSeverity.LOW ? mediumCase : lowCase;

await waitFor(() => {
expect(
screen.getByTestId(`case-action-popover-button-${theCase.id}`)
).toBeInTheDocument();
});
expect(screen.getByTestId(`case-action-popover-button-${theCase.id}`)).toBeInTheDocument();

userEvent.click(screen.getByTestId(`case-action-popover-button-${theCase.id}`));

await waitFor(() => {
expect(
screen.getByTestId(`case-action-severity-panel-${theCase.id}`)
).toBeInTheDocument();
});
expect(screen.getByTestId(`case-action-severity-panel-${theCase.id}`)).toBeInTheDocument();

userEvent.click(screen.getByTestId(`case-action-severity-panel-${theCase.id}`), undefined, {
skipPointerEventsCheck: true,
Expand All @@ -979,25 +979,17 @@ describe.skip('AllCasesListGeneric', () => {
appMockRenderer.render(<AllCasesList />);
const theCase = defaultGetCases.data.cases[0];

await waitFor(() => {
expect(
screen.getByTestId(`case-action-popover-button-${theCase.id}`)
).toBeInTheDocument();
});
expect(screen.getByTestId(`case-action-popover-button-${theCase.id}`)).toBeInTheDocument();

userEvent.click(screen.getByTestId(`case-action-popover-button-${theCase.id}`));

await waitFor(() => {
expect(screen.getByTestId('cases-bulk-action-delete')).toBeInTheDocument();
});
expect(screen.getByTestId('cases-bulk-action-delete')).toBeInTheDocument();

userEvent.click(screen.getByTestId('cases-bulk-action-delete'), undefined, {
skipPointerEventsCheck: true,
});

await waitFor(() => {
expect(screen.getByTestId('confirm-delete-case-modal')).toBeInTheDocument();
});
expect(screen.getByTestId('confirm-delete-case-modal')).toBeInTheDocument();

userEvent.click(screen.getByTestId('confirmModalConfirmButton'));

Expand All @@ -1011,11 +1003,11 @@ describe.skip('AllCasesListGeneric', () => {

userEvent.click(screen.getByTestId('checkboxSelectAll'));

await waitFor(() => {
for (const theCase of defaultGetCases.data.cases) {
for (const theCase of defaultGetCases.data.cases) {
await waitFor(() => {
expect(screen.getByTestId(`case-action-popover-button-${theCase.id}`)).toBeDisabled();
}
});
});
}
});

it('should disable row actions when selecting a case', async () => {
Expand All @@ -1024,11 +1016,11 @@ describe.skip('AllCasesListGeneric', () => {

userEvent.click(screen.getByTestId(`checkboxSelectRow-${caseToSelect.id}`));

await waitFor(() => {
for (const theCase of defaultGetCases.data.cases) {
for (const theCase of defaultGetCases.data.cases) {
await waitFor(() => {
expect(screen.getByTestId(`case-action-popover-button-${theCase.id}`)).toBeDisabled();
}
});
});
}
});
});

Expand Down

0 comments on commit 46c3566

Please sign in to comment.