Skip to content

Commit 402c7be

Browse files
yurishkuroclaude
andauthored
chore(jaeger-ui): replace require() in test bodies with static imports (#3692)
Replace require() calls in test body scope with static ES imports (H2a, ADR-0007). require() is unavailable in Vitest's native ESM test scope; static imports work in both Jest and Vitest. Changes across 9 files: - Move require() to top-level import statements - Add namespace imports for jest.spyOn targets (TracePage) - Consolidate require('react') in mock factory to { forwardRef } - Move misplaced imports above jest.mock() (PlexusDemo, TraceFlamegraph) Three files excluded (need vi.resetModules + await import in H3): - utils/prefix-url.test.js - utils/tracking/ga-coverage.test.js - prefix-url-coverage.test.js ## AI Usage in this PR (choose one) See [AI Usage Policy](https://github.com/jaegertracing/jaeger/blob/main/CONTRIBUTING_GUIDELINES.md#ai-usage-policy). - [ ] **None**: No AI tools were used in creating this PR - [ ] **Light**: AI provided minor assistance (formatting, simple suggestions) - [ ] **Moderate**: AI helped with code generation or debugging specific parts - [x] **Heavy**: AI generated most or all of the code changes --------- Signed-off-by: Yuri Shkuro <github@ysh.us> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 08b2a2b commit 402c7be

File tree

9 files changed

+57
-61
lines changed

9 files changed

+57
-61
lines changed

docs/adr/0007-vite-plus-migration.md

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -364,10 +364,16 @@ so each is reviewable in isolation and CI catches regressions early.
364364
121 of the 207 test files contained JSX but had a `.js` extension. Vite/esbuild does not parse JSX in
365365
`.js` files, so these must be renamed before the Vitest switch. Pure `git mv` with zero logic changes.
366366

367-
#### PR H2a — Replace `require()` in test bodies with static `import` (~12 files)
367+
#### PR H2a — Replace `require()` in test bodies with static `import` ([#3692](https://github.com/jaegertracing/jaeger-ui/pull/3692))
368368

369-
`require()` is unavailable in Vitest's native ESM test scope. Moving these to top-level `import`
370-
statements is valid under Jest (Babel transpiles them to CJS) and necessary for Vitest.
369+
`require()` is unavailable in Vitest's native ESM test scope. Converted 9 files.
370+
371+
Three files were **excluded** — they use `require()` after `jest.resetModules()` or inside runtime
372+
`jest.mock()` calls within `it` blocks (not hoisted). These use a module re-isolation pattern that
373+
requires `vi.resetModules()` + `await import()` and will be handled in H3:
374+
- `utils/prefix-url.test.js`
375+
- `utils/tracking/ga-coverage.test.js`
376+
- `prefix-url-coverage.test.js`
371377

372378
#### PR H2b — Replace arrow function constructors with regular functions (~16 files)
373379

@@ -585,7 +591,7 @@ confirm no errors or unexpected HTML injection.
585591
| ✅ E | Consolidate `jaeger-ui` tsconfigs; remove `main` from plexus package.json ([#3689](https://github.com/jaegertracing/jaeger-ui/pull/3689)) | Unknown 1 | Done |
586592
| 🔶 F | Migrate Jest → Vitest in both packages; remove Babel test deps ([#3690](https://github.com/jaegertracing/jaeger-ui/pull/3690) plexus ✅, jaeger-ui pending) | Unknowns 3, 4, 5, 6 | Partial |
587593
| ✅ H1 | Rename `.test.js``.test.jsx` in jaeger-ui (121 files, pure rename) ([#3691](https://github.com/jaegertracing/jaeger-ui/pull/3691)) | None | Done |
588-
| H2a | Replace `require()` in test bodies with static `import` (~12 files) | None | After H1 |
594+
| H2a | Replace `require()` in test bodies with static `import` ([#3692](https://github.com/jaegertracing/jaeger-ui/pull/3692)) | None | Done |
589595
| H2b | Replace arrow function constructors with regular functions (~16 files) | None | After H1 |
590596
| H2c | Introduce `mockDefault` helper in affected mock factories | None | After H1 |
591597
| H3 | Vitest switch for jaeger-ui | Unknowns 3, 4, 5, 6 | After H2a–c |

packages/jaeger-ui/src/components/DeepDependencies/index.test.jsx

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import * as React from 'react';
55
import { render, screen } from '@testing-library/react';
66
import '@testing-library/jest-dom';
77
import _set from 'lodash/set';
8+
import { MemoryRouter } from 'react-router-dom';
9+
import { Provider } from 'react-redux';
10+
import { createStore } from 'redux';
11+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
812

913
jest.mock('node-fetch', () =>
1014
jest.fn(() =>
@@ -980,14 +984,6 @@ describe('DeepDependencyGraphPage', () => {
980984
});
981985

982986
describe('DeepDependencyGraphPage (default export wrapper)', () => {
983-
const { MemoryRouter } = require('react-router-dom');
984-
985-
const { Provider } = require('react-redux');
986-
987-
const { createStore } = require('redux');
988-
989-
const { QueryClient, QueryClientProvider } = require('@tanstack/react-query');
990-
991987
const mockReduxStore = createStore(() => ({
992988
ddg: {},
993989
router: { location: { search: '?service=test-service' } },

packages/jaeger-ui/src/components/Monitor/index.test.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import store from '../../utils/storage';
1111
import MonitorATMPage from '.';
1212
import metricsReducer from '../../reducers/metrics';
1313
import * as jaegerApiActions from '../../actions/jaeger-api';
14+
import getConfig from '../../utils/config/get-config';
1415

1516
// --- Mock Modules ---
1617
// Mock the actions module
@@ -128,7 +129,6 @@ describe('<MonitorATMPage>', () => {
128129
});
129130

130131
it('renders EmptyState when metricsStorage is disabled in config', () => {
131-
const getConfig = require('../../utils/config/get-config').default;
132132
getConfig.mockImplementation(() => ({
133133
qualityMetrics: { apiEndpoint: '/api/quality-metrics' },
134134
storageCapabilities: { metricsStorage: false },

packages/jaeger-ui/src/components/PlexusDemo/index.test.jsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@
44
import React from 'react';
55
import { render, screen } from '@testing-library/react';
66
import '@testing-library/jest-dom';
7+
import PlexusDemo from './index';
78

89
// Mock the plexus demo module so the test does not depend on the plexus
910
// package build and avoids Web Worker creation in jsdom.
1011
// { virtual: true } is required because the module is resolved by a Vite alias
1112
// at build time but does not exist as a real path in node_modules.
13+
// Note: jest.mock() is always hoisted before imports, so the import above
14+
// picks up the mocked version regardless of source order.
1215
jest.mock(
1316
'@jaegertracing/plexus/demo',
1417
() => ({
@@ -20,9 +23,6 @@ jest.mock(
2023
{ virtual: true }
2124
);
2225

23-
// Import after the mock is registered.
24-
const { default: PlexusDemo } = require('./index');
25-
2626
describe('PlexusDemo', () => {
2727
it('renders without crashing', () => {
2828
expect(() => render(<PlexusDemo />)).not.toThrow();

packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.jsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ import {
6161
} from './SearchForm';
6262
import * as markers from './SearchForm.markers';
6363
import { CHANGE_SERVICE_ACTION_TYPE } from '../../constants/search-form';
64+
import { useServices, useSpanNames } from '../../hooks/useTraceDiscovery';
6465

6566
function makeDateParams(dateOffset = 0) {
6667
const date = new Date();
@@ -580,8 +581,6 @@ describe('<SearchForm>', () => {
580581
});
581582

582583
describe('error handling', () => {
583-
const { useServices, useSpanNames } = require('../../hooks/useTraceDiscovery');
584-
585584
afterEach(() => {
586585
jest.clearAllMocks();
587586
});

packages/jaeger-ui/src/components/SearchTracePage/index.test.jsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import React from 'react';
2020
import { render, screen, waitFor } from '@testing-library/react';
2121
import '@testing-library/jest-dom';
2222
import { Provider } from 'react-redux';
23+
import { createStore } from 'redux';
2324
import { SearchTracePageImpl as SearchTracePage, mapStateToProps } from './index';
2425
import { fetchedState } from '../../constants';
2526
import traceGenerator from '../../demo/trace-generators';
@@ -227,7 +228,7 @@ describe('<SearchTracePage>', () => {
227228

228229
it('hides Upload tab if it is disabled via config', () => {
229230
// Create a custom store with disableFileUploadControl: true
230-
const customStore = require('redux').createStore(() => ({
231+
const customStore = createStore(() => ({
231232
...globalStore.getState(),
232233
config: {
233234
...globalStore.getState().config,

packages/jaeger-ui/src/components/TracePage/TraceFlamegraph/index.test.jsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import React from 'react';
55
import { render, screen } from '@testing-library/react';
66
import '@testing-library/jest-dom';
7-
import { convertJaegerTraceToProfile } from '@pyroscope/flamegraph';
7+
import { convertJaegerTraceToProfile, FlamegraphRenderer } from '@pyroscope/flamegraph';
88
import TraceFlamegraph from './index';
99
import testTrace from './testTrace.json';
1010

@@ -22,10 +22,6 @@ jest.mock('@pyroscope/flamegraph', () => {
2222
};
2323
});
2424

25-
// Re-import FlamegraphRenderer after mocking
26-
27-
const { FlamegraphRenderer } = require('@pyroscope/flamegraph');
28-
2925
const profile = convertJaegerTraceToProfile(testTrace.data);
3026

3127
describe('convertJaegerTraceToProfile', () => {

packages/jaeger-ui/src/components/TracePage/TraceLogsView/index.test.jsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
import React from 'react';
5-
import { render, screen } from '@testing-library/react';
5+
import { render, screen, fireEvent } from '@testing-library/react';
66
import '@testing-library/jest-dom';
77
import TraceLogsView from './index';
88
import transformTraceData from '../../../model/transform-trace-data';
@@ -191,7 +191,6 @@ describe('<TraceLogsView>', () => {
191191
});
192192

193193
it('toggles attribute accordion open and closed on click', () => {
194-
const { fireEvent } = require('@testing-library/react');
195194
const trace = transformTraceData(baseTrace).asOtelTrace();
196195
render(<TraceLogsView trace={trace} useOtelTerms={false} />);
197196

@@ -254,7 +253,6 @@ describe('<TraceLogsView>', () => {
254253
});
255254

256255
it('handles column sorting for service name', () => {
257-
const { fireEvent } = require('@testing-library/react');
258256
const trace = transformTraceData(baseTrace).asOtelTrace();
259257
render(<TraceLogsView trace={trace} useOtelTerms={false} />);
260258

@@ -264,7 +262,6 @@ describe('<TraceLogsView>', () => {
264262
});
265263

266264
it('handles column sorting for operation/span name', () => {
267-
const { fireEvent } = require('@testing-library/react');
268265
const trace = transformTraceData(baseTrace).asOtelTrace();
269266
render(<TraceLogsView trace={trace} useOtelTerms={false} />);
270267

@@ -273,7 +270,6 @@ describe('<TraceLogsView>', () => {
273270
});
274271

275272
it('handles column sorting for event/log name', () => {
276-
const { fireEvent } = require('@testing-library/react');
277273
const trace = transformTraceData(baseTrace).asOtelTrace();
278274
render(<TraceLogsView trace={trace} useOtelTerms={false} />);
279275

packages/jaeger-ui/src/components/TracePage/index.test.jsx

Lines changed: 34 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,36 @@
11
// Copyright (c) 2017 Uber Technologies, Inc.
22
// SPDX-License-Identifier: Apache-2.0
33

4+
import React from 'react';
5+
import { render, screen, act } from '@testing-library/react';
6+
import '@testing-library/jest-dom';
7+
import { MemoryRouter } from 'react-router-dom';
8+
9+
import {
10+
computeAdjustedRange,
11+
makeShortcutCallbacks,
12+
mapDispatchToProps,
13+
mapStateToProps,
14+
shortcutConfig,
15+
TracePageImpl as TracePage,
16+
VIEW_MIN_RANGE,
17+
} from './index';
18+
import * as track from './index.track';
19+
import * as keyboardShortcutsMod from './keyboard-shortcuts';
20+
import { reset as resetShortcuts, merge as mergeShortcuts } from './keyboard-shortcuts';
21+
import * as scrollPageMod from './scroll-page';
22+
import { cancel as cancelScroll } from './scroll-page';
23+
import * as calculateTraceDagEV from './TraceGraph/calculateTraceDagEV';
24+
import { trackSlimHeaderToggle } from './TracePageHeader/TracePageHeader.track';
25+
import * as getUiFindVertexKeys from '../TraceDiff/TraceDiffGraph/traceDiffGraphUtils';
26+
import { fetchedState } from '../../constants';
27+
import traceGenerator from '../../demo/trace-generators';
28+
import transformTraceData from '../../model/transform-trace-data';
29+
import filterSpansSpy from '../../utils/filter-spans';
30+
import updateUiFindSpy from '../../utils/update-ui-find';
31+
import { ETraceViewType } from './types';
32+
import ScrollManager from './ScrollManager';
33+
434
let capturedHeaderProps = {};
535
let capturedArchiveNotifierProps = {};
636

@@ -79,10 +109,10 @@ jest.mock('./ArchiveNotifier', () => props => {
79109
});
80110

81111
jest.mock('./TracePageHeader', () => {
82-
const React = require('react');
112+
const { forwardRef } = require('react');
83113
return {
84114
__esModule: true,
85-
default: React.forwardRef(function MockTracePageHeader(props, ref) {
115+
default: forwardRef(function MockTracePageHeader(props, ref) {
86116
capturedHeaderProps = { ...props, ref };
87117
return (
88118
<div className="TracePageHeader">
@@ -94,34 +124,6 @@ jest.mock('./TracePageHeader', () => {
94124
};
95125
});
96126

97-
import React from 'react';
98-
import { render, screen, act } from '@testing-library/react';
99-
import '@testing-library/jest-dom';
100-
import { MemoryRouter } from 'react-router-dom';
101-
102-
import {
103-
computeAdjustedRange,
104-
makeShortcutCallbacks,
105-
mapDispatchToProps,
106-
mapStateToProps,
107-
shortcutConfig,
108-
TracePageImpl as TracePage,
109-
VIEW_MIN_RANGE,
110-
} from './index';
111-
import * as track from './index.track';
112-
import { reset as resetShortcuts, merge as mergeShortcuts } from './keyboard-shortcuts';
113-
import { cancel as cancelScroll } from './scroll-page';
114-
import * as calculateTraceDagEV from './TraceGraph/calculateTraceDagEV';
115-
import { trackSlimHeaderToggle } from './TracePageHeader/TracePageHeader.track';
116-
import * as getUiFindVertexKeys from '../TraceDiff/TraceDiffGraph/traceDiffGraphUtils';
117-
import { fetchedState } from '../../constants';
118-
import traceGenerator from '../../demo/trace-generators';
119-
import transformTraceData from '../../model/transform-trace-data';
120-
import filterSpansSpy from '../../utils/filter-spans';
121-
import updateUiFindSpy from '../../utils/update-ui-find';
122-
import { ETraceViewType } from './types';
123-
import ScrollManager from './ScrollManager';
124-
125127
const renderWithRouter = (ui, { route = '/' } = {}) => {
126128
return render(<MemoryRouter initialEntries={[route]}>{ui}</MemoryRouter>);
127129
};
@@ -438,8 +440,8 @@ describe('<TracePage>', () => {
438440

439441
ScrollManager.mockImplementation(() => scrollManagerMock);
440442

441-
const resetShortcutsMock = jest.spyOn(require('./keyboard-shortcuts'), 'reset');
442-
const cancelScrollMock = jest.spyOn(require('./scroll-page'), 'cancel');
443+
const resetShortcutsMock = jest.spyOn(keyboardShortcutsMod, 'reset');
444+
const cancelScrollMock = jest.spyOn(scrollPageMod, 'cancel');
443445

444446
const { unmount } = render(<TracePage {...defaultProps} />);
445447
unmount();

0 commit comments

Comments
 (0)