Skip to content

Commit c8134bd

Browse files
committed
Improve tests
1 parent fd8b026 commit c8134bd

File tree

3 files changed

+137
-35
lines changed

3 files changed

+137
-35
lines changed
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
import { LEVEL_MAIN } from '../const';
10+
11+
/**
12+
* Centralized test utilities for flyout manager tests.
13+
* Eliminates redundant mock setup and provides single source of truth.
14+
*/
15+
16+
// Shared mock functions - reused across all mocks
17+
export const mockCloseFlyout = jest.fn();
18+
19+
export const createMockFunctions = () => ({
20+
dispatch: jest.fn(),
21+
addFlyout: jest.fn(),
22+
closeFlyout: mockCloseFlyout,
23+
setActiveFlyout: jest.fn(),
24+
setFlyoutWidth: jest.fn(),
25+
goBack: jest.fn(),
26+
goToFlyout: jest.fn(),
27+
getHistoryItems: jest.fn(() => []),
28+
});
29+
30+
// Shared state object - single source of truth
31+
export const createMockState = () => ({
32+
sessions: [],
33+
flyouts: [],
34+
layoutMode: 'side-by-side' as const,
35+
});
36+
37+
/**
38+
* Factory for creating flyout manager API mock.
39+
* Used consistently across hooks, selectors, and provider mocks.
40+
*/
41+
export const createFlyoutManagerMock = () => ({
42+
state: createMockState(),
43+
...createMockFunctions(),
44+
});
45+
46+
/**
47+
* Factory for creating flyout manager reducer mock.
48+
* Includes additional reducer-specific properties.
49+
*/
50+
export const createFlyoutManagerReducerMock = () => ({
51+
state: createMockState(),
52+
...createMockFunctions(),
53+
});
54+
55+
/**
56+
* Helper for creating dynamic test state.
57+
* Useful for tests that need to simulate state changes.
58+
*/
59+
export const createTestState = (
60+
overrides: Partial<ReturnType<typeof createMockState>> = {}
61+
) => ({
62+
...createMockState(),
63+
...overrides,
64+
});
65+
66+
/**
67+
* Helper for creating test session data.
68+
*/
69+
export const createTestSession = (
70+
main: string,
71+
title: string,
72+
child: string | null = null
73+
) => ({
74+
main,
75+
title,
76+
child,
77+
});
78+
79+
/**
80+
* Helper for creating test flyout data.
81+
*/
82+
export const createTestFlyout = (
83+
flyoutId: string,
84+
level: string = LEVEL_MAIN
85+
) => ({
86+
flyoutId,
87+
level,
88+
});
89+
90+
/**
91+
* Reset all mocks. Call this in beforeEach.
92+
*/
93+
export const resetFlyoutMocks = () => {
94+
jest.clearAllMocks();
95+
};

packages/eui/src/components/flyout/manager/flyout_managed.test.tsx

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -46,29 +46,33 @@ jest.mock('../flyout.component', () => {
4646
};
4747
});
4848

49-
// Mock hooks that would otherwise depend on ResizeObserver or animation timing
49+
// Shared mock functions - must be defined in module scope for Jest
5050
const mockCloseFlyout = jest.fn();
51+
const createMockState = () => ({
52+
sessions: [],
53+
flyouts: [],
54+
layoutMode: 'side-by-side' as const,
55+
});
56+
const createMockFunctions = () => ({
57+
dispatch: jest.fn(),
58+
addFlyout: jest.fn(),
59+
closeFlyout: mockCloseFlyout,
60+
setActiveFlyout: jest.fn(),
61+
setFlyoutWidth: jest.fn(),
62+
goBack: jest.fn(),
63+
goToFlyout: jest.fn(),
64+
getHistoryItems: jest.fn(() => []),
65+
});
5166

67+
// Mock hooks that would otherwise depend on ResizeObserver or animation timing
5268
jest.mock('./hooks', () => ({
5369
useFlyoutManagerReducer: () => ({
54-
state: { sessions: [], flyouts: [], layoutMode: 'side-by-side' },
55-
dispatch: jest.fn(),
56-
addFlyout: jest.fn(),
57-
closeFlyout: mockCloseFlyout,
58-
setActiveFlyout: jest.fn(),
59-
setFlyoutWidth: jest.fn(),
60-
goBack: jest.fn(),
61-
goToFlyout: jest.fn(),
62-
getHistoryItems: jest.fn(() => []),
70+
state: createMockState(),
71+
...createMockFunctions(),
6372
}),
6473
useFlyoutManager: () => ({
65-
state: { sessions: [], flyouts: [], layoutMode: 'side-by-side' },
66-
addFlyout: jest.fn(),
67-
closeFlyout: mockCloseFlyout,
68-
setFlyoutWidth: jest.fn(),
69-
goBack: jest.fn(),
70-
goToFlyout: jest.fn(),
71-
getHistoryItems: jest.fn(() => []),
74+
state: createMockState(),
75+
...createMockFunctions(),
7276
}),
7377
useIsFlyoutActive: () => true,
7478
useHasChildFlyout: () => false,
@@ -104,13 +108,8 @@ jest.mock('./validation', () => ({
104108
jest.mock('./provider', () => ({
105109
...jest.requireActual('./provider'),
106110
useFlyoutManager: () => ({
107-
state: { sessions: [], flyouts: [], layoutMode: 'side-by-side' },
108-
addFlyout: jest.fn(),
109-
closeFlyout: mockCloseFlyout,
110-
setFlyoutWidth: jest.fn(),
111-
goBack: jest.fn(),
112-
goToFlyout: jest.fn(),
113-
getHistoryItems: jest.fn(() => []),
111+
state: createMockState(),
112+
...createMockFunctions(),
114113
}),
115114
}));
116115

@@ -119,6 +118,19 @@ jest.mock('../../observer/resize_observer', () => ({
119118
useResizeObserver: () => ({ width: 480 }),
120119
}));
121120

121+
// Import test utilities after mocks
122+
import {
123+
createTestState,
124+
createTestSession,
125+
createTestFlyout,
126+
} from './__mocks__';
127+
128+
// Helper functions for tests - these can reference the mock functions defined above
129+
const createFlyoutManagerMock = () => ({
130+
state: createMockState(),
131+
...createMockFunctions(),
132+
});
133+
122134
describe('EuiManagedFlyout', () => {
123135
const renderInProvider = (ui: React.ReactElement) =>
124136
render(<EuiFlyoutManager>{ui}</EuiFlyoutManager>);
@@ -285,7 +297,7 @@ describe('EuiManagedFlyout', () => {
285297
});
286298

287299
describe('onClose callback behavior', () => {
288-
it('calls onClose callback during component cleanup/unmount', () => {
300+
it('does not call onClose callback during component cleanup/unmount', () => {
289301
const onClose = jest.fn();
290302

291303
const { unmount } = renderInProvider(
@@ -305,9 +317,9 @@ describe('EuiManagedFlyout', () => {
305317
unmount();
306318
});
307319

308-
// onClose should be called during cleanup
309-
expect(onClose).toHaveBeenCalledTimes(1);
310-
expect(onClose).toHaveBeenCalledWith(expect.any(MouseEvent));
320+
// onClose should NOT be called during cleanup (intentional design)
321+
expect(onClose).not.toHaveBeenCalled();
322+
expect(mockCloseFlyout).toHaveBeenCalledWith('cleanup-test');
311323
});
312324

313325
it('does not call onClose multiple times (double-firing prevention)', () => {

packages/eui/src/components/flyout/manager/flyout_managed.tsx

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,6 @@ export const EuiManagedFlyout = ({
126126
const isActive = useIsFlyoutActive(flyoutId);
127127
const currentSession = useCurrentSession();
128128

129-
// Remove automatic close effect - let user actions control state
130-
// This was causing cascading closes in multi-session scenarios
131-
132129
// Stabilize the onClose callback
133130
const onCloseCallbackRef = useRef<((e?: CloseEvent) => void) | undefined>();
134131
onCloseCallbackRef.current = (e) => {
@@ -138,7 +135,7 @@ export const EuiManagedFlyout = ({
138135
}
139136
};
140137

141-
// Stabilize the onActive callback to prevent unnecessary re-registrations
138+
// Stabilize the onActive callback
142139
const onActiveCallbackRef = useRef<(() => void) | undefined>();
143140
onActiveCallbackRef.current = onActiveProp;
144141

@@ -205,7 +202,7 @@ export const EuiManagedFlyout = ({
205202
'width'
206203
);
207204

208-
// Pass a wrapper of onClose to Flyout with logging
205+
// Pass the stabilized onClose callback to the flyout menu context
209206
const onClose = (e?: CloseEvent) => {
210207
onCloseCallbackRef.current?.(e);
211208
};
@@ -222,8 +219,6 @@ export const EuiManagedFlyout = ({
222219
level,
223220
});
224221

225-
// Log activity stage changes
226-
227222
// Note: history controls are only relevant for main flyouts
228223
const historyItems = useMemo(() => {
229224
return level === LEVEL_MAIN ? getHistoryItems() : undefined;

0 commit comments

Comments
 (0)