Skip to content

refactor: replace vi.doMock with vi.mock + vi.hoisted #8319

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

Merged
DrJKL merged 1 commit into from
Jan 26, 2026
Merged

refactor: replace vi.doMock with vi.mock + vi.hoisted #8319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 14 additions & 0 deletions eslint.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,20 @@ export default defineConfig([
]
}
},
{
files: ['**/*.test.ts'],
rules: {
'no-restricted-properties': [
'error',
{
object: 'vi',
property: 'doMock',
message:
'Use vi.mock() with vi.hoisted() instead of vi.doMock(). See docs/testing/vitest-patterns.md'
}
]
}
},
{
files: ['scripts/**/*.js'],
languageOptions: {
Expand Down
36 changes: 21 additions & 15 deletions src/stores/firebaseAuthStore.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,21 @@ vi.mock('@/stores/toastStore', () => ({
// Mock useDialogService
vi.mock('@/services/dialogService')

const mockDistributionTypes = vi.hoisted(() => ({
isCloud: false,
isDesktop: false
}))

vi.mock('@/platform/distribution/types', () => mockDistributionTypes)

const mockApiKeyStore = vi.hoisted(() => ({
getAuthHeader: vi.fn().mockReturnValue(null)
}))

vi.mock('@/stores/apiKeyAuthStore', () => ({
useApiKeyAuthStore: () => mockApiKeyStore
}))

Comment on lines +85 to +99
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Reset hoisted distribution flags between tests to avoid leakage.

mockDistributionTypes is a shared object; setting isCloud/isDesktop to true persists beyond the token-refresh tests because vi.resetAllMocks() doesn’t reset plain object state. That can make later tests order-dependent. Please reset these flags in the outer beforeEach (or add an afterEach in the nested describe).

🛠 Suggested fix 
  beforeEach(() => {
    vi.resetAllMocks()
+   mockDistributionTypes.isCloud = false
+   mockDistributionTypes.isDesktop = false

Based on learnings, keep module mocks contained and avoid global mutable state in tests.

Also applies to: 165-180

🤖 Prompt for AI Agents 
In `@src/stores/firebaseAuthStore.test.ts` around lines 85 - 99, The shared
hoisted mock object mockDistributionTypes (with properties isCloud and
isDesktop) retains mutations between tests causing order-dependent failures;
reset those flags in test setup/teardown by adding a beforeEach or afterEach
that sets mockDistributionTypes.isCloud = false and
mockDistributionTypes.isDesktop = false (or restores original values) so each
test starts with a clean distribution state; update the nested describe(s)
around the token-refresh tests as well to ensure isolation for
mockDistributionTypes.

describe('useFirebaseAuthStore', () => {
let store: ReturnType<typeof useFirebaseAuthStore>
let authStateCallback: (user: any) => void
Expand Down Expand Up @@ -147,12 +162,9 @@ describe('useFirebaseAuthStore', () => {
})

describe('token refresh events', () => {
beforeEach(async () => {
vi.resetModules()
vi.doMock('@/platform/distribution/types', () => ({
isCloud: true,
isDesktop: true
}))
beforeEach(() => {
mockDistributionTypes.isCloud = true
mockDistributionTypes.isDesktop = true

vi.mocked(firebaseAuth.onIdTokenChanged).mockImplementation(
(_auth, callback) => {
Expand All @@ -164,8 +176,7 @@ describe('useFirebaseAuthStore', () => {
vi.mocked(vuefire.useFirebaseAuth).mockReturnValue(mockAuth as any)

setActivePinia(createPinia())
const storeModule = await import('@/stores/firebaseAuthStore')
store = storeModule.useFirebaseAuthStore()
store = useFirebaseAuthStore()
})

it("should not increment tokenRefreshTrigger on the user's first ID token event", () => {
Expand Down Expand Up @@ -442,13 +453,8 @@ describe('useFirebaseAuthStore', () => {
// This test reproduces the issue where getAuthHeader fails due to network errors
// when Firebase Auth tries to refresh tokens offline

// Mock useApiKeyAuthStore to return null (no API key fallback)
const mockApiKeyStore = {
getAuthHeader: vi.fn().mockReturnValue(null)
}
vi.doMock('@/stores/apiKeyAuthStore', () => ({
useApiKeyAuthStore: () => mockApiKeyStore
}))
// Configure mockApiKeyStore to return null (no API key fallback)
mockApiKeyStore.getAuthHeader.mockReturnValue(null)

// Setup user with network error on token refresh
mockUser.getIdToken.mockReset()
Expand Down
92 changes: 27 additions & 65 deletions src/workbench/extensions/manager/utils/versionUtil.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import { describe, expect, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

import {
checkVersionCompatibility,
getFrontendVersion
} from '@/workbench/extensions/manager/utils/versionUtil'

// Mock config module
const mockConfig = vi.hoisted((): { app_version: string | undefined } => ({
app_version: '1.24.0-1'
}))

vi.mock('@/config', () => ({
default: {
app_version: '1.24.0-1'
}
default: mockConfig
}))

describe('versionUtil', () => {
Expand Down Expand Up @@ -266,79 +267,40 @@ describe('versionUtil', () => {
})

describe('getFrontendVersion', () => {
let originalEnv: string | undefined

beforeEach(() => {
originalEnv = import.meta.env.VITE_APP_VERSION
mockConfig.app_version = '1.24.0-1'
})

afterEach(() => {
if (originalEnv !== undefined) {
import.meta.env.VITE_APP_VERSION = originalEnv
} else {
delete import.meta.env.VITE_APP_VERSION
}
})

it('should return app_version from config when available', () => {
const version = getFrontendVersion()
expect(version).toBe('1.24.0-1')
})

it('should fallback to VITE_APP_VERSION when app_version is not available', async () => {
// Save original environment
const originalEnv = import.meta.env.VITE_APP_VERSION

// Mock config without app_version
vi.doMock('@/config', () => ({
default: {}
}))

// Set VITE_APP_VERSION
it('should fallback to VITE_APP_VERSION when app_version is not available', () => {
mockConfig.app_version = undefined
import.meta.env.VITE_APP_VERSION = '2.0.0'

// Clear module cache to force re-import
vi.resetModules()

// Import fresh module
const versionUtil =
await import('@/workbench/extensions/manager/utils/versionUtil')

const version = versionUtil.getFrontendVersion()
const version = getFrontendVersion()
expect(version).toBe('2.0.0')

// Restore original env
import.meta.env.VITE_APP_VERSION = originalEnv

// Reset mocks for next test
vi.resetModules()
vi.doMock('@/config', () => ({
default: {
app_version: '1.24.0-1'
}
}))
})

it('should return undefined when no version is available', async () => {
// Save original environment
const originalEnv = import.meta.env.VITE_APP_VERSION

// Mock config without app_version
vi.doMock('@/config', () => ({
default: {}
}))

// Clear VITE_APP_VERSION
it('should return undefined when no version is available', () => {
mockConfig.app_version = undefined
delete import.meta.env.VITE_APP_VERSION

// Clear module cache to force re-import
vi.resetModules()

// Import fresh module
const versionUtil =
await import('@/workbench/extensions/manager/utils/versionUtil')

const version = versionUtil.getFrontendVersion()
const version = getFrontendVersion()
expect(version).toBeUndefined()

// Restore original env
if (originalEnv !== undefined) {
import.meta.env.VITE_APP_VERSION = originalEnv
}

// Reset mocks for next test
vi.resetModules()
vi.doMock('@/config', () => ({
default: {
app_version: '1.24.0-1'
}
}))
})
})
})