Spike for unusual test timeout for fleet #201443
Conversation
|
🤖 Jobs for this PR can be triggered through checkboxes. 🚧
ℹ️ To trigger the CI, please tick the checkbox below 👇
|
| act(() => { | ||
| fireEvent.click(results.getByTestId('standaloneTab')); | ||
| }); |
There was a problem hiding this comment.
There's no need to wrap fireEvent in act
| @@ -23,11 +23,8 @@ import type { FlyOutProps } from './types'; | |||
| import { AgentEnrollmentFlyout } from '.'; | |||
|
|
|||
| const render = (props?: Partial<FlyOutProps>) => { | |||
| cleanup(); | |||
There was a problem hiding this comment.
We shouldn't be cleaning up just before test starts, what's more with jest this is handled automatically for us
| act(() => { | ||
| results = render(); | ||
| }); |
There was a problem hiding this comment.
Also there's no need to render here, if we still invoke render again in each test suite
| @@ -118,7 +118,8 @@ describe('PackagePolicyActionsMenu', () => { | |||
| const agentPolicies = createMockAgentPolicies(); | |||
| const packagePolicy = createMockPackagePolicy({ hasUpgrade: false }); | |||
| const { utils } = renderMenu({ agentPolicies, packagePolicy }); | |||
| await act(async () => { | |||
There was a problem hiding this comment.
there's no need to have assertions run within an act function
| @@ -129,7 +130,7 @@ describe('PackagePolicyActionsMenu', () => { | |||
| const packagePolicy = createMockPackagePolicy({ hasUpgrade: true }); | |||
| const { utils } = renderMenu({ agentPolicies, packagePolicy }); | |||
|
|
|||
| await act(async () => { | |||
eokoneyo
force-pushed
the
fix/correct-usages-of-testing-library-for-fleet
branch
from
1392385
to
fde6dad
Compare
| @@ -7,7 +7,7 @@ | |||
|
|
|||
| import React from 'react'; | |||
|
|
|||
| import { act, fireEvent } from '@testing-library/react'; | |||
There was a problem hiding this comment.
removing instances of act wrapped around fireEvent shaved some seconds off the test running time here;
with act
## with waitFor
|
|
||
| import { createFleetTestRendererMock } from '../../mock'; | ||
|
|
||
| import { RootPrivilegesCallout } from './root_privileges_callout'; | ||
|
|
||
| describe('RootPrivilegesCallout', () => { | ||
| function render(rootIntegrations?: Array<{ name: string; title: string }>) { | ||
| cleanup(); |
There was a problem hiding this comment.
yields from from removing this:
| if (!deleteRowEl) { | ||
| throw new Error('Delete row button not found'); | ||
| } |
There was a problem hiding this comment.
This should actually be an assertion
| if (deleteRowButtons.length !== 3) { | ||
| throw new Error('Delete row buttons not found'); | ||
| } |
There was a problem hiding this comment.
This could be an assertion
Summary
Checklist
Check the PR satisfies following conditions.
Reviewers should verify this PR satisfies this list as well.
release_note:breakinglabel should be applied in these situations.release_note:*label is applied per the guidelinesIdentify risks
Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.
Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.