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 flaky tests in the CaseViewPage component #172940

Merged
merged 21 commits into from
Jan 15, 2024
Merged

[Cases] Fix flaky tests in the CaseViewPage component #172940

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
29f9c5f
Fix flaky tests
cnasikas Dec 8, 2023
e0c1198
Run the test 50 times
cnasikas Dec 8, 2023
5cf170c
Remove exclusive suite
cnasikas Dec 8, 2023
02b437b
PR feedback
cnasikas Dec 9, 2023
d484379
Mock more hooks and use fake timers
cnasikas Dec 10, 2023
a53cc81
Merge branch 'main' into fix_149777
kibanamachine Dec 10, 2023
b0840d2
Merge branch 'main' into fix_149777
kibanamachine Dec 10, 2023
d49e5de
Merge branch 'main' into fix_149777
kibanamachine Dec 11, 2023
da0d8ad
Merge branch 'main' into fix_149777
kibanamachine Dec 12, 2023
35745f8
Mock use_push_to_service and move tests to a lower level
cnasikas Dec 19, 2023
e6a3735
Merge branch 'main' into fix_149777
cnasikas Dec 19, 2023
46e537d
Move tests to lower levels
cnasikas Dec 20, 2023
3ce63b4
Run 50 times
cnasikas Dec 20, 2023
327d09d
Merge branch 'main' into fix_149777
cnasikas Jan 10, 2024
fb92d1f
Remove alerts tab testing
cnasikas Jan 10, 2024
20369ab
Merge branch 'main' into fix_149777
kibanamachine Jan 12, 2024
fa64d23
Mock components and move tests
cnasikas Jan 15, 2024
44a611d
Merge branch 'fix_149777' of github.com:cnasikas/kibana into fix_149777
cnasikas Jan 15, 2024
48537b8
Merge branch 'main' into fix_149777
cnasikas Jan 15, 2024
63f5870
Fix bad test subj in functional test
cnasikas Jan 15, 2024
f47d60f
Remove loop
cnasikas Jan 15, 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
100 changes: 25 additions & 75 deletions x-pack/plugins/cases/public/components/case_view/case_view_page.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ import { useGetCaseUserActionsStats } from '../../containers/use_get_case_user_a
import { createQueryWithMarkup } from '../../common/test_utils';
import { useCasesFeatures } from '../../common/use_cases_features';
import { CaseMetricsFeature } from '../../../common/types/api';
import { useGetCategories } from '../../containers/use_get_categories';
import { useSuggestUserProfiles } from '../../containers/user_profiles/use_suggest_user_profiles';

jest.mock('../../containers/use_get_action_license');
jest.mock('../../containers/use_update_case');
Expand All @@ -56,6 +58,7 @@ jest.mock('../../containers/use_post_push_to_service');
jest.mock('../../containers/use_get_case_connectors');
jest.mock('../../containers/use_get_case_users');
jest.mock('../../containers/user_profiles/use_bulk_get_user_profiles');
jest.mock('../../containers/user_profiles/use_suggest_user_profiles');
jest.mock('../../common/use_cases_features');
jest.mock('../user_actions/timestamp', () => ({
UserActionTimestamp: () => <></>,
Expand All @@ -64,6 +67,7 @@ jest.mock('../../common/navigation/hooks');
jest.mock('../../common/hooks');
jest.mock('../connectors/resilient/api');
jest.mock('../../common/lib/kibana');
jest.mock('../../containers/use_get_categories');

const useFetchCaseMock = useGetCase as jest.Mock;
const useUrlParamsMock = useUrlParams as jest.Mock;
Expand All @@ -79,6 +83,8 @@ const useGetCaseMetricsMock = useGetCaseMetrics as jest.Mock;
const useGetTagsMock = useGetTags as jest.Mock;
const useGetCaseUsersMock = useGetCaseUsers as jest.Mock;
const useCasesFeaturesMock = useCasesFeatures as jest.Mock;
const useGetCategoriesMock = useGetCategories as jest.Mock;
const useSuggestUserProfilesMock = useSuggestUserProfiles as jest.Mock;

const mockGetCase = (props: Partial<UseGetCase> = {}) => {
const data = {
Expand Down Expand Up @@ -183,6 +189,9 @@ describe('CaseViewPage', () => {
isAlertsEnabled: true,
isSyncAlertsEnabled: true,
});
useGetCategoriesMock.mockReturnValue({ data: [], isLoading: false });
useSuggestUserProfilesMock.mockReturnValue({ data: [], isLoading: false });
useUrlParamsMock.mockReturnValue({});

appMockRenderer = createAppMockRenderer({ license: platinumLicense });
});
Expand Down Expand Up @@ -337,27 +346,7 @@ describe('CaseViewPage', () => {
});
});

// FLAKY: https://github.com/elastic/kibana/issues/149777
describe.skip('Tabs', () => {
beforeEach(() => {
jest.clearAllMocks();
});

it('renders tabs correctly', async () => {
appMockRenderer.render(<CaseViewPage {...caseProps} />);

expect(await screen.findByRole('tablist')).toBeInTheDocument();

expect(await screen.findByTestId('case-view-tab-title-activity')).toBeInTheDocument();
expect(await screen.findByTestId('case-view-tab-title-alerts')).toBeInTheDocument();
expect(await screen.findByTestId('case-view-tab-title-files')).toBeInTheDocument();
});

it('renders the activity tab by default', async () => {
appMockRenderer.render(<CaseViewPage {...caseProps} />);
expect(await screen.findByTestId('case-view-tab-content-activity')).toBeInTheDocument();
});

describe('Tabs', () => {
it('renders the alerts tab when the query parameter tabId has alerts', async () => {
useUrlParamsMock.mockReturnValue({
urlParams: {
Expand All @@ -371,45 +360,6 @@ describe('CaseViewPage', () => {
expect(await screen.findByTestId('alerts-table')).toBeInTheDocument();
});

it('renders the activity tab when the query parameter tabId has activity', async () => {
useUrlParamsMock.mockReturnValue({
urlParams: {
tabId: CASE_VIEW_PAGE_TABS.ACTIVITY,
},
});

appMockRenderer.render(<CaseViewPage {...caseProps} />);

expect(await screen.findByTestId('case-view-tab-content-activity')).toBeInTheDocument();
});

it('renders the activity tab when the query parameter tabId has an unknown value', async () => {
useUrlParamsMock.mockReturnValue({
urlParams: {
tabId: 'what-is-love',
},
});

appMockRenderer.render(<CaseViewPage {...caseProps} />);

expect(await screen.findByTestId('case-view-tab-content-activity')).toBeInTheDocument();
expect(screen.queryByTestId('case-view-tab-content-alerts')).not.toBeInTheDocument();
});

it('navigates to the activity tab when the activity tab is clicked', async () => {
const navigateToCaseViewMock = useCaseViewNavigationMock().navigateToCaseView;
appMockRenderer.render(<CaseViewPage {...caseProps} />);

userEvent.click(await screen.findByTestId('case-view-tab-title-activity'));

await waitFor(() => {
expect(navigateToCaseViewMock).toHaveBeenCalledWith({
detailName: caseData.id,
tabId: CASE_VIEW_PAGE_TABS.ACTIVITY,
});
});
});

it('navigates to the alerts tab when the alerts tab is clicked', async () => {
const navigateToCaseViewMock = useCaseViewNavigationMock().navigateToCaseView;
appMockRenderer.render(<CaseViewPage {...caseProps} />);
Expand Down Expand Up @@ -459,27 +409,27 @@ describe('CaseViewPage', () => {
await screen.findByTestId('case-view-alerts-table-experimental-badge')
).toBeInTheDocument();
});
});

describe('description', () => {
it('renders the description correctly', async () => {
appMockRenderer.render(<CaseViewPage {...caseProps} />);
describe('description', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved outside the Tabs block.

it('renders the description correctly', async () => {
appMockRenderer.render(<CaseViewPage {...caseProps} />);

const description = within(await screen.findByTestId('description'));
const description = within(await screen.findByTestId('description'));

expect(await description.findByText(caseData.description)).toBeInTheDocument();
});
expect(await description.findByText(caseData.description)).toBeInTheDocument();
});

it('should display description when case is loading', async () => {
useUpdateCaseMock.mockImplementation(() => ({
...defaultUpdateCaseState,
isLoading: true,
updateKey: 'description',
}));
it('should display description when case is loading', async () => {
useUpdateCaseMock.mockImplementation(() => ({
...defaultUpdateCaseState,
isLoading: true,
updateKey: 'description',
}));

appMockRenderer.render(<CaseViewPage {...caseProps} />);
appMockRenderer.render(<CaseViewPage {...caseProps} />);

expect(await screen.findByTestId('description')).toBeInTheDocument();
});
expect(await screen.findByTestId('description')).toBeInTheDocument();
});
});
});
20 changes: 10 additions & 10 deletions x-pack/plugins/cases/public/components/case_view/case_view_page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui';
import React, { useCallback, useEffect, useMemo, useRef } from 'react';
import React, { useCallback, useEffect, useRef } from 'react';
import { CASE_VIEW_PAGE_TABS } from '../../../common/types';
import { useUrlParams } from '../../common/navigation';
import { useCasesContext } from '../cases_context/use_cases_context';
Expand All @@ -23,6 +23,14 @@ import type { CaseViewPageProps } from './types';
import { useRefreshCaseViewPage } from './use_on_refresh_case_view_page';
import { useOnUpdateField } from './use_on_update_field';

const getActiveTabId = (tabId?: string) => {
if (tabId && Object.values(CASE_VIEW_PAGE_TABS).includes(tabId as CASE_VIEW_PAGE_TABS)) {
return tabId;
}

return CASE_VIEW_PAGE_TABS.ACTIVITY;
};

export const CaseViewPage = React.memo<CaseViewPageProps>(
({
caseData,
Expand All @@ -39,12 +47,7 @@ export const CaseViewPage = React.memo<CaseViewPageProps>(

useCasesTitleBreadcrumbs(caseData.title);

const activeTabId = useMemo(() => {
if (urlParams.tabId && Object.values(CASE_VIEW_PAGE_TABS).includes(urlParams.tabId)) {
return urlParams.tabId;
}
return CASE_VIEW_PAGE_TABS.ACTIVITY;
}, [urlParams.tabId]);
const activeTabId = getActiveTabId(urlParams?.tabId);
Copy link
Member Author

@cnasikas cnasikas Dec 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

When I removed some tests the other tests started failing. There was an issue with mocking and memorization that affected the tests. I removed the useMemo as it is redundant.


const init = useRef(true);
const timelineUi = useTimelineContext()?.ui;
Expand Down Expand Up @@ -113,15 +116,12 @@ export const CaseViewPage = React.memo<CaseViewPageProps>(
onUpdateField={onUpdateField}
/>
</HeaderPage>

<EuiFlexGroup>
<EuiFlexItem>
<CaseViewMetrics data-test-subj="case-view-metrics" caseId={caseData.id} />
</EuiFlexItem>
</EuiFlexGroup>

<EuiSpacer size="l" />

<EuiFlexGroup data-test-subj={`case-view-tab-content-${activeTabId}`} alignItems="baseline">
{activeTabId === CASE_VIEW_PAGE_TABS.ACTIVITY && (
<CaseViewActivity
Expand Down
5 changes: 3 additions & 2 deletions x-pack/test/functional/services/cases/navigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ export function CasesNavigationProvider({ getPageObject, getService }: FtrProvid
await common.clickAndValidate('configure-case-button', 'case-configure-title');
},

async navigateToSingleCase(app: string = 'cases', caseId: string) {
await common.navigateToUrlWithBrowserHistory(app, caseId);
async navigateToSingleCase(app: string = 'cases', caseId: string, tabId?: string) {
const search = tabId != null ? `?tabId=${tabId}` : '';
await common.navigateToUrlWithBrowserHistory(app, caseId, search);
},
};
}
42 changes: 42 additions & 0 deletions x-pack/test/functional_with_es_ssl/apps/cases/group1/view_case.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,11 +1013,22 @@ export default ({ getPageObject, getService }: FtrProviderContext) => {
describe('Tabs', () => {
createOneCaseBeforeDeleteAllAfter(getPageObject, getService);

it('renders tabs correctly', async () => {
await testSubjects.existOrFail('case-view-tab-title-activity');
await testSubjects.existOrFail('case-view-tab-title-files');
// there are no alerts in stack management yet
Copy link
Member Author

Choose a reason for hiding this comment

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

When this PR #172217 is merged we can test the alerts table in the stack.

});

it('shows the "activity" tab by default', async () => {
await testSubjects.existOrFail('case-view-tab-title-activity');
await testSubjects.existOrFail('case-view-tab-content-activity');
});

it("shows the 'activity' tab when clicked", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this test make sense here following the previous one?

We are already in the activity tab, I don't know how relevant it is to check that nothing changes when we click it.

Maybe we could go to files and back instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. Much better.

await testSubjects.click('case-view-tab-title-activity');
await testSubjects.existOrFail('case-view-tab-content-activity');
});

// there are no alerts in stack management yet
it.skip("shows the 'alerts' tab when clicked", async () => {
await testSubjects.click('case-view-tab-title-alerts');
Expand All @@ -1028,6 +1039,37 @@ export default ({ getPageObject, getService }: FtrProviderContext) => {
await testSubjects.click('case-view-tab-title-files');
await testSubjects.existOrFail('case-view-tab-content-files');
});

describe('Query params', () => {
it('renders the activity tab when the query parameter tabId=activity', async () => {
const theCase = await createAndNavigateToCase(getPageObject, getService);

await cases.navigation.navigateToSingleCase('cases', theCase.id, 'activity');
await testSubjects.existOrFail('case-view-tab-title-activity');
});

// there are no alerts in stack management yet
it.skip('renders the activity tab when the query parameter tabId=alerts', async () => {
const theCase = await createAndNavigateToCase(getPageObject, getService);

await cases.navigation.navigateToSingleCase('cases', theCase.id, 'alerts');
await testSubjects.existOrFail('case-view-tab-title-activity');
});

it('renders the activity tab when the query parameter tabId=files', async () => {
const theCase = await createAndNavigateToCase(getPageObject, getService);

await cases.navigation.navigateToSingleCase('cases', theCase.id, 'files');
await testSubjects.existOrFail('case-view-tab-content-files');
});

it('renders the activity tab when the query parameter tabId has an unknown value', async () => {
const theCase = await createAndNavigateToCase(getPageObject, getService);

await cases.navigation.navigateToSingleCase('cases', theCase.id, 'fake');
await testSubjects.existOrFail('case-view-tab-title-activity');
});
});
});

describe('Files', () => {
Expand Down