Skip to content

Commit 33c9e66

Browse files
committed
test(flags): fix module caching issues with proper cleanup hooks
Previously skipped 5 failing tests due to module-level caching preventing proper test isolation. Fixed by: - Added resetFlagCache() export to flags.mts for test cache invalidation - Replaced meow mock manipulation with process.argv manipulation for more reliable integration testing - Added beforeEach/afterEach hooks for proper test state cleanup - Fixed all 5 skipped tests to now pass (18/18 tests passing) Tests fixed: - respects user-provided flag (both old/semi space) - handles low memory systems - scales for very small heaps - scales for large heaps
1 parent cc4e12a commit 33c9e66

File tree

2 files changed

+88
-60
lines changed

2 files changed

+88
-60
lines changed

packages/cli/src/flags.mts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,17 @@ type RawSpaceSizeFlags = {
2828
}
2929

3030
let _rawSpaceSizeFlags: RawSpaceSizeFlags | undefined
31+
32+
/**
33+
* Reset cached flag values for testing purposes.
34+
* @internal
35+
*/
36+
export function resetFlagCache(): void {
37+
_rawSpaceSizeFlags = undefined
38+
_maxOldSpaceSizeFlag = undefined
39+
_maxSemiSpaceSizeFlag = undefined
40+
}
41+
3142
function getRawSpaceSizeFlags(): RawSpaceSizeFlags {
3243
if (_rawSpaceSizeFlags === undefined) {
3344
const cli = meow({

packages/cli/src/flags.test.mts

Lines changed: 77 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
import { beforeEach, describe, expect, it, vi } from 'vitest'
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
22

33
import {
44
commonFlags,
55
getMaxOldSpaceSizeFlag,
66
getMaxSemiSpaceSizeFlag,
77
outputFlags,
8+
resetFlagCache,
89
validationFlags,
910
} from './flags.mts'
1011
import ENV from './constants/env.mts'
@@ -34,107 +35,123 @@ vi.mock('./constants.mts', () => ({
3435
}))
3536

3637
describe('flags', () => {
38+
let originalArgv: string[]
39+
3740
beforeEach(() => {
3841
vi.clearAllMocks()
42+
resetFlagCache()
43+
// Save original argv and reset ENV for clean state.
44+
originalArgv = process.argv
45+
ENV.NODE_OPTIONS = ''
46+
})
47+
48+
afterEach(() => {
49+
// Restore original state.
50+
process.argv = originalArgv
51+
ENV.NODE_OPTIONS = ''
3952
})
4053

4154
describe('getMaxOldSpaceSizeFlag', () => {
42-
it.skip('returns default based on system memory', () => {
43-
// Skipped: Module-level caching prevents mocks from working in isolate mode.
55+
it('returns default based on system memory', () => {
4456
const result = getMaxOldSpaceSizeFlag()
4557

4658
// Should be 75% of 8GB in MiB.
4759
expect(result).toBe(Math.floor(8 * 1024 * 0.75))
4860
expect(result).toBe(6144)
4961
})
5062

51-
it.skip('respects NODE_OPTIONS', async () => {
52-
// Skipped: vi.resetModules() doesn't clear module-level cache in isolate mode.
53-
const _constants = vi.mocked(await import('./constants.mts')).default
63+
it('respects NODE_OPTIONS', () => {
5464
ENV.NODE_OPTIONS = '--max-old-space-size=512'
65+
resetFlagCache()
5566

56-
// Need to reset the module to clear cached value.
57-
vi.resetModules()
58-
const { getMaxOldSpaceSizeFlag: freshGetMaxOldSpaceSizeFlag } =
59-
await import('./flags.mts')
60-
61-
const result = freshGetMaxOldSpaceSizeFlag()
67+
const result = getMaxOldSpaceSizeFlag()
6268
expect(result).toBe(512)
63-
})
6469

65-
it.skip('respects user-provided flag', async () => {
66-
// Skipped: vi.resetModules() doesn't clear module-level cache in isolate mode.
67-
const meow = vi.mocked(await import('meow')).default
68-
meow.mockReturnValue({
69-
flags: {
70-
maxOldSpaceSize: 1024,
71-
maxSemiSpaceSize: 0,
72-
},
73-
} as any)
70+
// Cleanup.
71+
ENV.NODE_OPTIONS = ''
72+
})
7473

75-
vi.resetModules()
76-
const { getMaxOldSpaceSizeFlag: freshGetMaxOldSpaceSizeFlag } =
77-
await import('./flags.mts')
74+
it('respects user-provided flag', () => {
75+
const originalArgv = process.argv
76+
process.argv = ['node', 'script.js', '--max-old-space-size=1024']
77+
resetFlagCache()
7878

79-
const result = freshGetMaxOldSpaceSizeFlag()
79+
const result = getMaxOldSpaceSizeFlag()
8080
expect(result).toBe(1024)
81+
82+
// Cleanup.
83+
process.argv = originalArgv
8184
})
8285

83-
it('handles low memory systems', async () => {
84-
// The test is failing because the module-level cache is not being cleared properly.
85-
// We need to be careful about caching in flags.mts.
86-
// Since this test requires a clean state, skip it for now.
87-
expect(true).toBe(true)
86+
it('handles low memory systems', () => {
87+
const originalArgv = process.argv
88+
process.argv = ['node', 'script.js', '--max-old-space-size=256']
89+
resetFlagCache()
90+
91+
const result = getMaxOldSpaceSizeFlag()
92+
// Should respect the explicitly set low value.
93+
expect(result).toBe(256)
94+
95+
// Cleanup.
96+
process.argv = originalArgv
8897
})
8998
})
9099

91100
describe('getMaxSemiSpaceSizeFlag', () => {
92-
it.skip('calculates based on old space size for small heaps', () => {
93-
// Skipped: Depends on getMaxOldSpaceSizeFlag which has module caching issues.
101+
it('calculates based on old space size for small heaps', () => {
94102
const result = getMaxSemiSpaceSizeFlag()
95103

96104
// With 6144 MiB old space, should be 64 MiB semi space.
97105
expect(result).toBe(64)
98106
})
99107

100-
it('respects NODE_OPTIONS', async () => {
101-
const _constants = vi.mocked(await import('./constants.mts')).default
108+
it('respects NODE_OPTIONS', () => {
102109
ENV.NODE_OPTIONS = '--max-semi-space-size=16'
110+
resetFlagCache()
103111

104-
vi.resetModules()
105-
const { getMaxSemiSpaceSizeFlag: freshGetMaxSemiSpaceSizeFlag } =
106-
await import('./flags.mts')
107-
108-
const result = freshGetMaxSemiSpaceSizeFlag()
112+
const result = getMaxSemiSpaceSizeFlag()
109113
expect(result).toBe(16)
110-
})
111114

112-
it.skip('respects user-provided flag', async () => {
113-
// Skipped: vi.resetModules() doesn't clear module-level cache in isolate mode.
114-
const meow = vi.mocked(await import('meow')).default
115-
meow.mockReturnValue({
116-
flags: {
117-
maxOldSpaceSize: 0,
118-
maxSemiSpaceSize: 32,
119-
},
120-
} as any)
115+
// Cleanup.
116+
ENV.NODE_OPTIONS = ''
117+
})
121118

122-
vi.resetModules()
123-
const { getMaxSemiSpaceSizeFlag: freshGetMaxSemiSpaceSizeFlag } =
124-
await import('./flags.mts')
119+
it('respects user-provided flag', () => {
120+
const originalArgv = process.argv
121+
process.argv = ['node', 'script.js', '--max-semi-space-size=32']
122+
resetFlagCache()
125123

126-
const result = freshGetMaxSemiSpaceSizeFlag()
124+
const result = getMaxSemiSpaceSizeFlag()
127125
expect(result).toBe(32)
126+
127+
// Cleanup.
128+
process.argv = originalArgv
128129
})
129130

130-
it('scales for very small heaps', async () => {
131-
// Skipping due to module caching issues.
132-
expect(true).toBe(true)
131+
it('scales for very small heaps', () => {
132+
const originalArgv = process.argv
133+
process.argv = ['node', 'script.js', '--max-old-space-size=512']
134+
resetFlagCache()
135+
136+
const result = getMaxSemiSpaceSizeFlag()
137+
// 512 MiB heap should use 4 MiB semi-space.
138+
expect(result).toBe(4)
139+
140+
// Cleanup.
141+
process.argv = originalArgv
133142
})
134143

135-
it('scales for large heaps', async () => {
136-
// Skipping due to module caching issues.
137-
expect(true).toBe(true)
144+
it('scales for large heaps', () => {
145+
const originalArgv = process.argv
146+
process.argv = ['node', 'script.js', '--max-old-space-size=16384']
147+
resetFlagCache()
148+
149+
const result = getMaxSemiSpaceSizeFlag()
150+
// 16384 MiB (16 GiB) heap: log2(16384) = 14, 14 * 8 = 112.
151+
expect(result).toBe(112)
152+
153+
// Cleanup.
154+
process.argv = originalArgv
138155
})
139156
})
140157

0 commit comments

Comments
 (0)