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

[Security Solution][Endpoint] Fix and unskip flaky test #149841

Merged
merged 8 commits into from
Feb 1, 2023
Merged

[Security Solution][Endpoint] Fix and unskip flaky test #149841

Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
db73b4e
Fix flaky test
ashokaditya Jan 30, 2023
7c8049d
Merge branch 'main' into task/dw-fix-flaky-test-145204
ashokaditya Jan 31, 2023
f096c3e
Merge branch 'main' into task/dw-fix-flaky-test-145204
ashokaditya Jan 31, 2023
7f16a10
Merge branch 'main' into task/dw-fix-flaky-test-145204
ashokaditya Jan 31, 2023
be32c7e
Merge branch 'main' into task/dw-fix-flaky-test-145204
ashokaditya Jan 31, 2023
6d1bbec
Merge branch 'main' into task/dw-fix-flaky-test-145204
ashokaditya Jan 31, 2023
5d2f4d2
Merge branch 'main' into task/dw-fix-flaky-test-145204
ashokaditya Feb 1, 2023
6dfd418
Merge branch 'main' into task/dw-fix-flaky-test-145204
ashokaditya Feb 1, 2023
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
6 changes: 2 additions & 4 deletions x-pack/plugins/security_solution/public/management/pages/integration_tests/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import type { AppContextTestRender } from '../../../common/mock/endpoint';
import { createAppRootMockRenderer } from '../../../common/mock/endpoint';
import { useUserPrivileges } from '../../../common/components/user_privileges';
import { endpointPageHttpMock } from '../endpoint_hosts/mocks';
import { getUserPrivilegesMockDefaultValue } from '../../../common/components/user_privileges/__mocks__';

jest.mock('../../../common/components/user_privileges');

Expand All @@ -30,7 +29,7 @@ describe('when in the Administration tab', () => {
});

afterEach(() => {
useUserPrivilegesMock.mockImplementation(getUserPrivilegesMockDefaultValue);
useUserPrivilegesMock.mockReset();
});
Comment on lines 31 to 33
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder why this solves the issue? The mock is setup in all of the flaky test cases - how can this have an effect on those?

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe the mockReset is the right way to reset and remove all mocked return values.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with you on this, I just don't see how this is connected to the flaky tests. Do you see the connection?

Copy link
Member Author

Choose a reason for hiding this comment

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

My best guess is that it's because we're using useCanSeeHostIsolationExceptionsMenu in 8.6 tests and that clashes with the mockImplementation of useUserPrivilegesMock. I notice now that we're actually missing a test for HIE page visibility when useCanSeeHostIsolationExceptionsMenu is true in 8.6. Moreover we're not using the useCanSeeHostIsolationExceptionsMenu in those 8.6 tests. I'll create a PR for 8.6 with that test. Also another one for main that uses the canReadHostIsolationExceptions for HIE page visibility.

Looks like the test was flaky again on the last test after this PR was merged to main.


describe('when the user has no permissions', () => {
Expand Down Expand Up @@ -97,8 +96,7 @@ describe('when in the Administration tab', () => {
});
});

// FLAKY: https://github.com/elastic/kibana/issues/145204
describe.skip('when the user has permissions', () => {
describe('when the user has permissions', () => {
it('should display the Management view if user has privileges', async () => {
useUserPrivilegesMock.mockReturnValue({
endpointPrivileges: { loading: false, canReadEndpointList: true, canAccessFleet: true },
Expand Down