Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -135,24 +135,25 @@ | |||
| "@types/yauzl": "^2.10.3", | |||
There was a problem hiding this comment.
🔄 Carefully review the package-lock.json diff
Resolve the comment if everything is ok
* node_modules/@babel/parser 7.28.5 -> 7.29.0
* node_modules/@babel/types 7.28.5 -> 7.29.0
* node_modules/@rollup/pluginutils 5.2.0 -> 5.3.0
* node_modules/istanbul-reports 3.1.7 -> 3.2.0
+ node_modules/@vitest/coverage-v8/node_modules/@bcoe/v8-coverage 1.0.2
+ node_modules/@vitest/coverage-v8/node_modules/magicast 0.5.2
+ node_modules/@vitest/coverage-v8 4.0.18
+ node_modules/ast-v8-to-istanbul/node_modules/estree-walker 3.0.3
+ node_modules/ast-v8-to-istanbul/node_modules/js-tokens 10.0.0
+ node_modules/ast-v8-to-istanbul 0.3.11
+ node_modules/git-semver-tags/node_modules/conventional-commits-filter 5.0.0
+ node_modules/git-semver-tags/node_modules/conventional-commits-parser 6.2.1
+ node_modules/magicast 0.3.5
+ node_modules/unplugin-swc/node_modules/picomatch 4.0.3
+ node_modules/unplugin-swc/node_modules/unplugin 2.3.11
+ node_modules/unplugin-swc/node_modules/webpack-virtual-modules 0.6.2
+ node_modules/unplugin-swc 1.5.9
+ node_modules/vitest-mock-extended 3.1.0 
📝 WalkthroughWalkthroughMigrates the apps/api test tooling from Jest to Vitest: removes Jest config and custom Jest environment, adds Vitest config and compatibility shims, updates package.json scripts and devDependencies, converts local test config to TypeScript, and updates many tests to use vitest-mock-extended and vi APIs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (74.75%) is below the target coverage (78.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2687 +/- ##
==========================================
- Coverage 53.85% 51.62% -2.23%
==========================================
Files 1057 1057
Lines 29393 28021 -1372
Branches 6340 6337 -3
==========================================
- Hits 15829 14467 -1362
+ Misses 13296 13068 -228
- Partials 268 486 +218
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (27)
apps/api/src/core/services/hono-error-handler/hono-error-handler.service.spec.ts (1)
115-116:⚠️ Potential issue | 🔴 Critical
jest.fn()will fail at runtime in Vitest.The migration to
vitest-mock-extendedis incomplete—jest.fn()is not available in a Vitest environment since thejestglobal is not defined. This will cause aReferenceErrorwhen running the test.🐛 Proposed fix
Add the
viimport and replacejest.fn():+import { vi } from "vitest"; import { mock } from "vitest-mock-extended";const mockContext = mock<AppContext>({ - body: jest.fn(() => new Response()) + body: vi.fn(() => new Response()) });apps/api/src/deployment/controllers/deployment/top-up-deployments.controller.spec.ts (2)
12-29:⚠️ Potential issue | 🟡 MinorUse present-simple test descriptions without “should”.
Both test descriptions violate the test naming convention. Use present simple, 3rd person singular without “should” (e.g., “calls the service to top up deployments”). As per coding guidelines: “Use present simple, 3rd person singular for test descriptions without prepending 'should'.”
33-47:⚠️ Potential issue | 🟡 MinorAlign
setupsignature with the required test pattern.
setupmust accept a single parameter with inline type definition and must not specify a return type. As per coding guidelines: “Thesetupfunction must … accept a single parameter with inline type definition … and not have a specified return type.”Suggested adjustment
- function setup(): { - controller: TopUpDeploymentsController; - topUpManagedDeploymentsService: MockProxy<TopUpManagedDeploymentsService>; - staleDeploymentsCleanerService: MockProxy<StaleManagedDeploymentsCleanerService>; - } { - const topUpManagedDeploymentsService = mock<TopUpManagedDeploymentsService>(); - const staleDeploymentsCleanerService = mock<StaleManagedDeploymentsCleanerService>(); + function setup({ + topUpManagedDeploymentsService = mock<TopUpManagedDeploymentsService>(), + staleDeploymentsCleanerService = mock<StaleManagedDeploymentsCleanerService>() + }: { + topUpManagedDeploymentsService?: MockProxy<TopUpManagedDeploymentsService>; + staleDeploymentsCleanerService?: MockProxy<StaleManagedDeploymentsCleanerService>; + } = {}) { const controller = new TopUpDeploymentsController(topUpManagedDeploymentsService, staleDeploymentsCleanerService); return { controller, topUpManagedDeploymentsService, staleDeploymentsCleanerService }; }apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.spec.ts (1)
222-270:⚠️ Potential issue | 🔴 CriticalIncomplete migration:
jest.fn()calls will fail in Vitest.The file still uses
jest.fn()at lines 225, 238, 239, and 265, but thejestglobal is not available in Vitest. These need to be replaced withvi.fn()from Vitest.🐛 Proposed fix to replace jest.fn() with vi.fn()
Add the import at the top of the file:
import { vi } from "vitest";Then apply the following diff:
function setup(testData: TransactionTestData[]) { SemaphoreFactory.useMemory(); const wallet = mock<Wallet>({ - getFirstAddress: jest.fn(() => Promise.resolve(createAkashAddress())) + getFirstAddress: vi.fn(() => Promise.resolve(createAkashAddress())) }); const billingConfigService = mockConfigService<BillingConfigService>({ RPC_NODE_ENDPOINT: "http://localhost:26657", WALLET_BATCHING_INTERVAL_MS: "0", GAS_SAFETY_MULTIPLIER: "1.2", AVERAGE_GAS_PRICE: 0.025 }); const registry = new Registry(); const client = mock<SigningStargateClient>({ - getChainId: jest.fn(async () => "test-chain"), - getAccount: jest.fn(async (address: string) => ({ accountNumber: 0, sequence: 1, address }) as Account) + getChainId: vi.fn(async () => "test-chain"), + getAccount: vi.fn(async (address: string) => ({ accountNumber: 0, sequence: 1, address }) as Account) }); testData.forEach((data, index) => { client.simulate.mockResolvedValueOnce(data.gasEstimate); client.sign.mockResolvedValueOnce(data.signedMessage); if (index < testData.length - 1) { if (data.hash instanceof Error) { client.broadcastTxSync.mockRejectedValueOnce(data.hash); } else { client.broadcastTxSync.mockResolvedValueOnce(data.hash); } } else { if (data.hash instanceof Error) { client.broadcastTx.mockRejectedValueOnce(data.hash); } else { client.broadcastTx.mockResolvedValueOnce({ transactionHash: data.hash } as DeliverTxResponse); } } if (!(data.hash instanceof Error)) { client.getTx.mockResolvedValueOnce(data.tx); } }); - const createClientWithSigner = jest.fn(() => client); + const createClientWithSigner = vi.fn(() => client); const service = new BatchSigningClientService(billingConfigService, wallet, registry, createClientWithSigner); return { service, client }; }apps/api/src/healthz/services/healthz/healthz.service.spec.ts (2)
114-136:⚠️ Potential issue | 🔴 CriticalIncomplete migration: Jest timer APIs should be replaced with Vitest equivalents.
The migration switched imports to
vitest-mock-extended, butjest.useFakeTimers(),jest.advanceTimersByTime(), andjest.useRealTimers()are still used. In Vitest, these should bevi.useFakeTimers(),vi.advanceTimersByTime(), andvi.useRealTimers().Same issue exists in the test at lines 153-198.
🐛 Proposed fix
Add the import at the top of the file:
import { vi } from "vitest";Then apply these changes:
it("caches liveness results and retries after TTL expires", async () => { - jest.useFakeTimers(); + vi.useFakeTimers(); const { service, dbHealthcheck, jobQueueHealthcheck } = setup(); // First call - both should be called await service.getLivenessStatus(); expect(dbHealthcheck.ping).toHaveBeenCalledTimes(1); expect(jobQueueHealthcheck.ping).toHaveBeenCalledTimes(1); // Second call within TTL - should use cached results, no additional calls await service.getLivenessStatus(); expect(dbHealthcheck.ping).toHaveBeenCalledTimes(1); expect(jobQueueHealthcheck.ping).toHaveBeenCalledTimes(1); // Advance time beyond TTL - jest.advanceTimersByTime(millisecondsInMinute + 1); + vi.advanceTimersByTime(millisecondsInMinute + 1); // Call after TTL - should make new calls await service.getLivenessStatus(); expect(dbHealthcheck.ping).toHaveBeenCalledTimes(2); expect(jobQueueHealthcheck.ping).toHaveBeenCalledTimes(2); - jest.useRealTimers(); + vi.useRealTimers(); });Apply similar changes to the test at lines 153-198.
203-204:⚠️ Potential issue | 🔴 CriticalIncomplete migration:
jest.fn()should be replaced withvi.fn().The
mock()function fromvitest-mock-extendedis used, butjest.fn()is still used for default mock implementations. This should bevi.fn()to ensure compatibility with Vitest.🐛 Proposed fix
function setup() { const logger = mock<LoggerService>(); - const dbHealthcheck = mock<DbHealthcheck>({ ping: jest.fn().mockResolvedValue(undefined) }); - const jobQueueHealthcheck = mock<JobQueueHealthcheck>({ ping: jest.fn().mockResolvedValue(undefined) }); + const dbHealthcheck = mock<DbHealthcheck>({ ping: vi.fn().mockResolvedValue(undefined) }); + const jobQueueHealthcheck = mock<JobQueueHealthcheck>({ ping: vi.fn().mockResolvedValue(undefined) }); const healthzService = new HealthzService(dbHealthcheck, jobQueueHealthcheck, logger);apps/api/src/core/services/shutdown-server/shutdown-server.spec.ts (1)
10-10:⚠️ Potential issue | 🔴 CriticalIncomplete migration:
jest.fn()is not available in Vitest.The import was changed to
vitest-mock-extended, but the file still usesjest.fn()andjest.Mockthroughout. In Vitest, thejestglobal does not exist—usevi.fn()andMocktype from Vitest instead. This will cause runtime failures.🔧 Proposed fix: Replace jest.fn() with vi.fn()
Add the
viimport at the top:import type { Logger } from "@akashnetwork/logging"; import type { ServerType } from "@hono/node-server"; -import { mock } from "vitest-mock-extended"; +import { vi } from "vitest"; +import { type Mock, mock } from "vitest-mock-extended";Then replace all occurrences throughout the file:
- close: jest.fn().mockImplementation(cb => cb()) + close: vi.fn().mockImplementation(cb => cb())- const onShutdown = jest.fn(); + const onShutdown = vi.fn();- close: jest.fn().mockImplementation(() => { + close: vi.fn().mockImplementation(() => { throw error; - }) as jest.Mock + }) as MockApply the same pattern to all other
jest.fn()occurrences on lines 25, 28, 47, 61, 64, 77, 80, 91, and 94.Also applies to: 13-13, 25-25, 28-28, 42-44, 47-47, 61-61, 64-64, 77-77, 80-80, 91-91, 94-94
apps/api/src/deployment/services/deployment-setting/deployment-setting.service.spec.ts (1)
99-99:⚠️ Potential issue | 🟡 MinorAdd the required
setupparameter.
setupcurrently takes no arguments, but the test guideline requires a single parameter with an inline type definition. Consider adding an (optionally empty) parameter.Proposed fix
-function setup() { +function setup({}: {} = {}) { const deploymentSettingRepository = mock<DeploymentSettingRepository>(); const authService = mock<AuthService>(); const drainingDeploymentService = mock<DrainingDeploymentService>(); const walletReloadJobService = mock<WalletReloadJobService>(); const userWalletRepository = mock<UserWalletRepository>();As per coding guidelines, "Use
setupfunction instead ofbeforeEachin test files. Thesetupfunction must ... accept a single parameter with inline type definition".apps/api/src/billing/controllers/wallet/wallet.controller.spec.ts (1)
245-276:⚠️ Potential issue | 🔴 Critical
jest.fn()is not available in Vitest — tests will fail at runtime.The import was updated to
vitest-mock-extended, butjest.fn()calls remain. In Vitest, thejestglobal is undefined. Replace withvi.fn()and add the import.🐛 Proposed fix
Add
viimport at the top of the file:import { vi } from "vitest";Then apply these changes in the
setupfunction:useValue: mock<StripeService>({ isProduction: true, - getPaymentMethods: jest.fn(async () => { + getPaymentMethods: vi.fn(async () => { if (!input?.hasPaymentMethods) return []; return [{ ...generatePaymentMethod(), validated: true, isDefault: false }]; }), - hasDuplicateTrialAccount: jest.fn().mockResolvedValue(input?.hasDuplicateTrialAccount ?? false), - validatePaymentMethodForTrial: jest.fn().mockImplementation(() => { + hasDuplicateTrialAccount: vi.fn().mockResolvedValue(input?.hasDuplicateTrialAccount ?? false), + validatePaymentMethodForTrial: vi.fn().mockImplementation(() => { if (input?.validationFails) { throw new Error("Card validation failed"); } @@ ... rootContainer.register(UserRepository, { useValue: mock<UserRepository>({ - findTrialUsersByFingerprint: jest.fn().mockResolvedValue(input?.hasDuplicateFingerprint ? [{ id: faker.string.uuid() }] : []) + findTrialUsersByFingerprint: vi.fn().mockResolvedValue(input?.hasDuplicateFingerprint ? [{ id: faker.string.uuid() }] : []) }) });apps/api/test/mocks/config-service.mock.ts (1)
19-22:⚠️ Potential issue | 🔴 CriticalIncomplete migration:
jest.MockedFunctiontype reference will break in Vitest.Line 19 still references
jest.MockedFunction, which is a Jest-specific type. After migrating to Vitest, this type won't be available (unless the compat shim defines it globally, which is fragile). Use Vitest's nativeMocktype instead.🐛 Proposed fix
Add the
Mockimport from vitest at line 1:+import type { Mock } from "vitest"; import type { MockProxy } from "vitest-mock-extended";Then update the type cast:
- (svc.get as unknown as jest.MockedFunction<(key: keyof ConfigOf<T>) => any>).mockImplementation(key => { + (svc.get as Mock<(key: keyof ConfigOf<T>) => any>).mockImplementation(key => {apps/api/src/notifications/services/notification-handler/notification.handler.spec.ts (2)
294-315:⚠️ Potential issue | 🔴 CriticalReplace all
jest.fn()calls withvi.fn()in setup function and tests.The setup function and several tests use
jest.fn()which is not available in Vitest. Replace withvi.fn():🔧 Proposed fix for setup function
function setup(input?: { findUserById?: UserRepository["findById"]; createNotification?: NotificationService["createNotification"]; resolveVars?: NotificationDataResolverService["resolve"]; }) { const mocks = { notificationService: mock<NotificationService>({ - createNotification: input?.createNotification ?? jest.fn().mockResolvedValue(undefined) + createNotification: input?.createNotification ?? vi.fn().mockResolvedValue(undefined) }), userRepository: mock<UserRepository>({ - findById: input?.findUserById ?? jest.fn() + findById: input?.findUserById ?? vi.fn() }), notificationDataResolverService: mock<NotificationDataResolverService>({ - resolve: input?.resolveVars ?? jest.fn().mockImplementation(async (user, vars) => vars) + resolve: input?.resolveVars ?? vi.fn().mockImplementation(async (user, vars) => vars) }), logger: mock<LoggerService>() };Also update all test calls that pass
jest.fn()to usevi.fn()(e.g., lines 37, 65, 94, 119, 144, 182, 188, 216, 243, 274).
173-205:⚠️ Potential issue | 🔴 CriticalReplace Jest timer APIs with Vitest equivalents.
The fake timers API needs to be updated for Vitest:
🔧 Proposed fix
- jest.useFakeTimers({ now: currentDate }); + vi.useFakeTimers({ now: currentDate });- jest.useRealTimers(); + vi.useRealTimers();apps/api/src/auth/services/auth0/auth0.service.spec.ts (1)
89-89:⚠️ Potential issue | 🟡 MinorReplace deprecated
jest.fn()withvi.fn()for Vitest.The codebase includes a Jest compatibility shim for backwards compatibility, but
jest.fn()is explicitly marked as deprecated with guidance to use Vitest APIs directly. Migrate this tovi.fn()to follow the modern pattern.Add the import if not already present:
Proposed fix
- const mockGetByEmail = jest.fn(); + const mockGetByEmail = vi.fn();Ensure
viis imported:import { vi } from "vitest";apps/api/src/middlewares/contentTypeMiddleware/contentTypeMiddleware.spec.ts (2)
6-6:⚠️ Potential issue | 🟡 MinorUse the subject’s
.namein the root describe.
Replace the hardcoded string withcontentTypeMiddleware.nameto keep refactors safe.As per coding guidelines: "Use
<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references".Suggested change
-describe("contentTypeMiddleware", () => { +describe(contentTypeMiddleware.name, () => {
88-95:⚠️ Potential issue | 🟠 MajorMigrate from Jest to Vitest APIs.
The project uses Vitest, and the jest-compat setup file is explicitly marked as deprecated. Usevi.fn()and Vitest'sMockedFunctiontype directly instead of relying on the Jest compatibility shim.Suggested change
import type { Context, Next } from "hono"; import { mock } from "vitest-mock-extended"; +import { vi, type MockedFunction } from "vitest"; import { contentTypeMiddleware } from "./contentTypeMiddleware";- const next = jest.fn() as jest.MockedFunction<Next>; + const next = vi.fn() as MockedFunction<Next>;apps/api/src/provider/services/provider/provider.service.spec.ts (1)
340-342:⚠️ Potential issue | 🟡 MinorIncomplete migration:
jest.fn()should bevi.fn()for Vitest.The import was updated to
vitest-mock-extended, butjest.fn()is still used inside the mock initialization. This should bevi.fn()for consistency with Vitest.🔧 Proposed fix
const jwtTokenService = mock<ProviderJwtTokenService>({ - generateJwtToken: jest.fn().mockResolvedValue(Ok("mock-jwt-token")) + generateJwtToken: vi.fn().mockResolvedValue(Ok("mock-jwt-token")) });You'll also need to import
vifrom vitest:import { vi } from "vitest";apps/api/src/core/lib/create-route/create-route.spec.ts (2)
206-206:⚠️ Potential issue | 🟡 MinorIncomplete migration:
jest.fn()should bevi.fn().🔧 Proposed fix
- const existingMiddleware = jest.fn() as unknown as MiddlewareHandler; + const existingMiddleware = vi.fn() as unknown as MiddlewareHandler;
284-284:⚠️ Potential issue | 🟡 MinorIncomplete migration:
jest.fn()andjest.MockedFunctionshould use Vitest equivalents.🔧 Proposed fix
- const next = jest.fn().mockResolvedValue(undefined) as jest.MockedFunction<Next>; + const next = vi.fn().mockResolvedValue(undefined) as Mock<Next>;You'll need to import from vitest:
import { vi, type Mock } from "vitest";apps/api/src/billing/services/chain-error/chain-error.service.spec.ts (1)
185-185:⚠️ Potential issue | 🟡 MinorIncomplete migration:
jest.Mockshould be VitestMocktype.🔧 Proposed fix
- (billingConfigService.get as jest.Mock).mockImplementation((key: string) => { + (billingConfigService.get as Mock).mockImplementation((key: string) => {Import
Mockfrom vitest:import { type Mock } from "vitest";apps/api/src/app/services/trial-started/trial-started.handler.spec.ts (1)
21-21:⚠️ Potential issue | 🟡 MinorIncomplete migration: Multiple
jest.fn()usages should bevi.fn().This file has several
jest.fn()calls (lines 21, 41, 67, 108, 184, 219, 222, 225) that should be converted tovi.fn()for complete Vitest migration.🔧 Example fix for line 21
const { handler, userRepository, notificationService, jobQueueManager, logger } = setup({ - findUserById: jest.fn().mockResolvedValue(null) + findUserById: vi.fn().mockResolvedValue(null) });Add the vitest import at the top:
import { vi } from "vitest";apps/api/src/notifications/services/notification/notification.service.spec.ts (1)
8-13:⚠️ Potential issue | 🟡 MinorIncomplete migration: Jest timer utilities should use Vitest equivalents.
The file uses
jest.useFakeTimers(),jest.useRealTimers(), andjest.runAllTimersAsync()throughout (lines 9, 13, 19, 40, 43, 48, 72, 83, 100, 115, 129, 141, 150, 162). These should be converted to Vitest'svi.useFakeTimers(),vi.useRealTimers(), andvi.runAllTimersAsync().🔧 Example fix
+import { vi } from "vitest"; import { mockDeep } from "vitest-mock-extended"; ... describe("createDefaultChannel", () => { afterEach(() => { - jest.useRealTimers(); + vi.useRealTimers(); }); it("calls API to create default email channel with user's email", async () => { - jest.useFakeTimers(); + vi.useFakeTimers(); const { service, api } = setup(); ... - await Promise.all([service.createDefaultChannel(user), jest.runAllTimersAsync()]); + await Promise.all([service.createDefaultChannel(user), vi.runAllTimersAsync()]);apps/api/test/services/mock-config.service.ts (1)
13-13:⚠️ Potential issue | 🟡 MinorIncomplete migration:
jest.MockedFunctionshould use VitestMocktype.🔧 Proposed fix
+import { type Mock } from "vitest"; -import { mock, type MockProxy } from "vitest-mock-extended"; +import { mock, type MockProxy } from "vitest-mock-extended"; ... - const getMock = svc.get as unknown as jest.MockedFunction<(key: K) => ConfigOf<S>[K]>; + const getMock = svc.get as unknown as Mock<(key: K) => ConfigOf<S>[K]>;apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.spec.ts (2)
27-31:⚠️ Potential issue | 🟠 MajorIncomplete migration:
jest.Mocktype cast should use Vitest equivalent.This file imports from
vitest-mock-extendedbut still usesjest.Mockfor type casting. In Vitest, useMockfromvitestor leveragevi.fn()with proper typing.
552-576:⚠️ Potential issue | 🟠 MajorIncomplete migration:
jest.fn()calls should be replaced withvi.fn().The
setup()function usesjest.fn()extensively (lines 562, 257, 261, 265, 269, 273, 277, 281, 285, 289, 295, 298, 306). Since the project is migrating to Vitest, these should be replaced withvi.fn().🔧 Proposed fix for setup function
+import { vi } from "vitest"; import { mock } from "vitest-mock-extended"; // In setup(): stripeService.charges = { - retrieve: jest.fn() + retrieve: vi.fn() } as unknown as Stripe.ChargesResource;Apply similar changes throughout the
setup()function for alljest.fn()occurrences.apps/api/src/billing/services/tx-manager/tx-manager.service.spec.ts (1)
243-345:⚠️ Potential issue | 🟠 MajorIncomplete migration:
jest.fn()calls should be replaced withvi.fn().The
setup()function and test cases usejest.fn()extensively throughout this file (e.g., lines 30, 100, 158, 184, 206, 257, 261, 265, 269, 273, 277, 281, 285, 289, 295, 298, 306). These should be migrated tovi.fn()for consistency with the Vitest framework.🔧 Proposed fix
+import { vi } from "vitest"; import { mock } from "vitest-mock-extended"; // Example changes in setup(): const fundingWallet = mock<Wallet>({ - getFirstAddress: jest.fn().mockResolvedValue(fundingWalletAddress) + getFirstAddress: vi.fn().mockResolvedValue(fundingWalletAddress) }); -const walletFactory = jest.fn().mockImplementation((_index: number) => { +const walletFactory = vi.fn().mockImplementation((_index: number) => { return derivedWallet; });Apply similar changes for all
jest.fn()occurrences.apps/api/src/app/services/trial-deployment-lease-created/trial-deployment-lease-created.handler.spec.ts (1)
205-226:⚠️ Potential issue | 🟠 MajorIncomplete migration:
jest.fn()calls should be replaced withvi.fn().The
setup()function usesjest.fn()(lines 212, 216, 219) and test cases also usejest.fn()(lines 20, 52, 89, 143, 176). These should be migrated tovi.fn().🔧 Proposed fix for setup function
+import { vi } from "vitest"; import { mock } from "vitest-mock-extended"; // In setup(): const mocks = { userWalletRepository: mock<UserWalletRepository>({ - findById: input?.findWalletById ?? jest.fn() + findById: input?.findWalletById ?? vi.fn() }), logger: mock<LoggerService>(), jobQueueService: mock<JobQueueService>({ - enqueue: input?.enqueueJob ?? jest.fn().mockResolvedValue(undefined) + enqueue: input?.enqueueJob ?? vi.fn().mockResolvedValue(undefined) }), billingConfig: mock<BillingConfigService>({ - get: jest.fn().mockReturnValue(input?.trialDeploymentLifetimeInHours ?? 24) + get: vi.fn().mockReturnValue(input?.trialDeploymentLifetimeInHours ?? 24) }) };apps/api/package.json (1)
127-127:⚠️ Potential issue | 🟡 MinorRemove
jest-mock-extendedas it's unused; keep@types/jestfor the Jest compatibility layer.
jest-mock-extendedhas no imports in the codebase and should be removed. However,@types/jestis still needed: it's referenced byapps/api/test/types/jest.d.tsfor custom Jest type extensions (liketoBeTypeOrNull), and the vitest compatibility shim intest/vitest-jest-compat.tsmaps Jest to Vitest'sviAPI, allowing tests to continue usingjest.fn(),jest.spyOn(), etc. Usevitest-mock-extendedinstead ofjest-mock-extendedfor any new mocking.
🤖 Fix all issues with AI agents
In `@apps/api/package.json`:
- Line 37: The script "test:unit:cov" in package.json is missing the project
flag and therefore runs coverage for all projects; update the "test:unit:cov"
script to include the same project flag used by "test:unit" (add --project=unit)
so it runs coverage only for the unit tests.
In `@apps/api/src/chain/services/block-http/block-http.service.spec.ts`:
- Line 10: The test description uses the "should" prefix; update the spec in
block-http.service.spec.ts by changing the it description from "should get
current height" to present simple 3rd person singular form (e.g., "gets current
height") so the test reads it("gets current height", async () => { ... })—leave
the test body and function name unchanged.
In
`@apps/api/src/notifications/services/notification-handler/notification.handler.spec.ts`:
- Around line 1-2: The test file uses Jest globals (jest.fn, jest.useFakeTimers,
jest.useRealTimers) but the imports were moved to vitest-mock-extended; add the
missing Vitest "vi" import (import { vi } from 'vitest') at the top of the file
and replace or ensure uses of jest.* are switched to vi.* (e.g., vi.fn,
vi.useFakeTimers, vi.useRealTimers) so the mocks and timer helpers reference
Vitest's API; update any occurrences of jest.* in notification.handler.spec.ts
accordingly.
🧹 Nitpick comments (23)
apps/api/src/network/services/network/network.service.spec.ts (1)
81-92: Alignsetupsignature with test guideline requirements.
setupmust accept a single parameter with an inline type definition and must not declare an explicit return type.
As per coding guidelines, Usesetupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.♻️ Suggested fix
- function setup(): { - netConfig: MockProxy<NetConfig>; - nodeHttpService: MockProxy<NodeHttpService>; - service: NetworkService; - } { + function setup({}: {}) { cacheEngine.clearAllKeyInCache(); const nodeHttpService = mock<NodeHttpService>(); const netConfig = mock<NetConfig>(); const service = new NetworkService(nodeHttpService, netConfig); return { netConfig, nodeHttpService, service }; }apps/api/src/core/services/shutdown-server/shutdown-server.spec.ts (1)
7-101: Consider adding asetupfunction.Per coding guidelines, test files should use a
setupfunction instead of repeating mock creation in each test. This would reduce duplication ofmock<Logger>()andvi.fn()foronShutdownacross all tests.♻️ Example setup function structure
describe(shutdownServer.name, () => { // ... tests using setup() ... function setup(overrides: { server?: Partial<ServerType>; onShutdownError?: Error } = {}) { const server = mock<ServerType>({ listening: true, close: vi.fn().mockImplementation(cb => cb()), ...overrides.server }); const appLogger = mock<Logger>(); const onShutdown = overrides.onShutdownError ? vi.fn().mockRejectedValue(overrides.onShutdownError) : vi.fn(); return { server, appLogger, onShutdown }; } });As per coding guidelines: "Use
setupfunction instead ofbeforeEachin test files."apps/api/src/billing/controllers/wallet/wallet.controller.spec.ts (1)
17-17: UseWalletController.nameinstead of hardcoded string.As per coding guidelines: "Use
<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references."♻️ Suggested change
-describe("WalletController", () => { +describe(WalletController.name, () => {apps/api/src/user/services/user/user.service.spec.ts (1)
19-19: Usevi.fn()for consistency with the Vitest migration.The migration changed the mock library import to
vitest-mock-extended, but this line still usesjest.fn(). For consistency, use Vitest's nativevi.fn().♻️ Proposed fix
Add the import at the top of the file:
import { vi } from "vitest";Then update line 19:
- const createDefaultNotificationChannel = jest.fn(() => Promise.resolve()); + const createDefaultNotificationChannel = vi.fn(() => Promise.resolve());apps/api/src/chain/services/block-http/block-http.service.spec.ts (1)
20-22: Redundant explicitvi.fn()in mock setup.
vitest-mock-extended'smock<T>()automatically creates mock functions for all methods on the type. The explicit{ getCurrentHeight: vi.fn() }is unnecessary sinceblockHttpService.getCurrentHeight.mockResolvedValue(height)on line 13 will work without it.Suggested simplification
- const blockHttpService = mock<BlockHttpServiceCommon>({ - getCurrentHeight: vi.fn() - }); + const blockHttpService = mock<BlockHttpServiceCommon>();apps/api/src/gpu/services/gpu-price/gpu-price.service.spec.ts (1)
23-29: Consider moving cache clearing intosetupfunction pattern.The coding guidelines prefer using the
setupfunction instead ofbeforeEach. The pre-test cache clearing could be moved into thesetupfunction. For post-test cleanup, you could explore returning a cleanup function fromsetupor accepting thatafterEachis sometimes necessary for teardown operations that don't fit the setup pattern.♻️ Example approach
function setup(input: { gpusForPricing: GpuType[]; deploymentsWithGpu: ReturnType<typeof createDeploymentWithBid>[]; days: ReturnType<typeof createDay>[]; pricingBotAddress?: string; }) { + cacheEngine.clearAllKeyInCache(); + const gpuRepository = mock<GpuRepository>(); // ... rest of setup - return { service, gpuRepository, deploymentRepository, akashBlockRepository, dayRepository, logger }; + return { + service, gpuRepository, deploymentRepository, akashBlockRepository, dayRepository, logger, + cleanup: () => cacheEngine.clearAllKeyInCache() + }; }Then in tests, call
cleanup()after assertions if needed, or keepafterEachfor automatic cleanup.As per coding guidelines: "Use
setupfunction instead ofbeforeEachin test files."apps/api/test/functional/managed-api-deployment-flow.spec.ts (1)
575-578: Replaceconsole.errorwithLoggerService.This violates the coding guideline requiring
LoggerServiceinstead ofconsole.errorfor logging.♻️ Suggested fix
+import { LoggerService } from "@src/core/services/logger/logger.service"; + +// Inside the describe block or setup: +const logger = new LoggerService({ context: "ManagedWalletAPIDeploymentFlowTest" }); // In createLease function: if (!data?.data?.leases) { - console.error("Cannot create lease", data); + logger.error({ event: "LEASE_CREATION_FAILED", data }, "Cannot create lease"); return undefined; }As per coding guidelines: "Use
LoggerServiceinstead ofconsole.log,console.warn, orconsole.errorfor logging".apps/api/src/dashboard/services/stats/stats.service.spec.ts (1)
12-14: Incomplete migration:jest.spyOncalls remain.The import was updated to vitest-mock-extended, but
jest.spyOncalls throughout this file (lines 20, 65, 158, 251, 290, 342) should be migrated tovi.spyOnfor consistency with Vitest. If the compatibility shim is intentional for gradual migration, consider adding a TODO comment.Additionally, per coding guidelines, consider refactoring the
beforeEachblock into thesetupfunction pattern by having setup clear the cache before returning.♻️ Example migration for spyOn calls
+import { vi } from "vitest"; import { mock } from "vitest-mock-extended";Then replace throughout:
- jest.spyOn(Provider, "findAll").mockResolvedValue([]); + vi.spyOn(Provider, "findAll").mockResolvedValue([]);As per coding guidelines: "Use
setupfunction instead ofbeforeEachin test files."Also applies to: 20-20
apps/api/src/bid/services/bid/bid.service.spec.ts (1)
29-30: Incomplete migration:jest.fn()calls remain.Multiple
jest.fn()calls remain in this file (lines 29, 62, 100, 115, 134). These should be migrated tovi.fn()for full Vitest compatibility.♻️ Example migration
+import { vi } from "vitest"; import { mock } from "vitest-mock-extended";Then replace:
userWalletRepository.accessibleBy.mockReturnValue({ - findFirst: jest.fn().mockResolvedValue(userWallet) + findFirst: vi.fn().mockResolvedValue(userWallet) } as unknown as ReturnType<UserWalletRepository["accessibleBy"]>);apps/api/src/core/services/start-server/start-server.spec.ts (1)
34-34: Incomplete migration: Multiplejest.fn()andjest.spyOn()calls remain.This file has extensive use of Jest globals that should be migrated to Vitest equivalents:
jest.fn()→vi.fn()(lines 34, 44-45, 128, 137, 151)jest.spyOn()→vi.spyOn()(lines 59, 81, 93, 105, 117)♻️ Example migration
+import { vi } from "vitest"; import { mock } from "vitest-mock-extended";Then replace all occurrences:
- const beforeStart = jest.fn().mockResolvedValue(undefined); + const beforeStart = vi.fn().mockResolvedValue(undefined);- jest.spyOn(processEvents, "on"); + vi.spyOn(processEvents, "on");Also applies to: 59-59, 151-151
apps/api/src/core/services/error/error.service.spec.ts (1)
13-13: Incomplete migration:jest.fn()calls remain.Multiple
jest.fn()calls remain in this file (lines 13, 25, 27, 40). These should be migrated tovi.fn()for full Vitest compatibility.♻️ Example migration
+import { vi } from "vitest"; import { mock } from "vitest-mock-extended";- const cb = jest.fn().mockResolvedValue(result); + const cb = vi.fn().mockResolvedValue(result);Also applies to: 25-27
apps/api/src/template/services/template-gallery/template-gallery.service.spec.ts (1)
265-278: Incomplete migration:jest.fn()calls remain in setup function.The setup function still uses
jest.fn()for mock implementations (lines 265, 275-278). These should be migrated tovi.fn()for full Vitest compatibility.♻️ Example migration
+import { vi } from "vitest"; import { mock } from "vitest-mock-extended";const fsMock = mock<FileSystemApi>({ - access: jest.fn(() => Promise.reject(new Error("File not found"))) + access: vi.fn(() => Promise.reject(new Error("File not found"))) });const templateFetcher = mock<TemplateFetcherService>({ - fetchLatestCommitSha: jest.fn(() => Promise.resolve("abc123")), - fetchAwesomeAkashTemplates: jest.fn(() => Promise.resolve([])), - fetchOmnibusTemplates: jest.fn(() => Promise.resolve([])), - clearArchiveCache: jest.fn() + fetchLatestCommitSha: vi.fn(() => Promise.resolve("abc123")), + fetchAwesomeAkashTemplates: vi.fn(() => Promise.resolve([])), + fetchOmnibusTemplates: vi.fn(() => Promise.resolve([])), + clearArchiveCache: vi.fn() });apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.spec.ts (1)
353-366: Consider replacingjest.fn()withvi.fn()for consistency.While the compatibility shim allows
jest.fn()to work in Vitest, usingvi.fn()would make this a complete migration. Alternatively, you can remove these explicit mock implementations sincevitest-mock-extendedauto-mocks all methods.♻️ Suggested refactor
const walletSettingRepository = mock<WalletSettingRepository>(); const balancesService = mock<BalancesService>({ - ensure2floatingDigits: jest.fn().mockImplementation((amount: number) => amount) + ensure2floatingDigits: vi.fn().mockImplementation((amount: number) => amount) }); const walletReloadJobService = mock<WalletReloadJobService>(); const drainingDeploymentService = mock<DrainingDeploymentService>(); const stripeService = mock<StripeService>(); const instrumentationService = mock<WalletBalanceReloadCheckInstrumentationService>({ - recordJobExecution: jest.fn(), - recordReloadTriggered: jest.fn(), - recordReloadSkipped: jest.fn(), - recordReloadFailed: jest.fn(), - recordValidationError: jest.fn(), - recordSchedulingError: jest.fn() + recordJobExecution: vi.fn(), + recordReloadTriggered: vi.fn(), + recordReloadSkipped: vi.fn(), + recordReloadFailed: vi.fn(), + recordValidationError: vi.fn(), + recordSchedulingError: vi.fn() });Add the import at the top:
import { vi } from "vitest";apps/api/src/core/services/feature-flags/feature-flags.service.spec.ts (1)
18-18: Replacejest.fn()withvi.fn()for complete Vitest migration.Multiple test cases use
jest.fn()which relies on the compatibility shim. For a cleaner Vitest migration, consider replacing these withvi.fn().♻️ Example changes
+import { vi } from "vitest";Then replace occurrences like:
- const createClient = jest.fn(() => createUnleashMockClient()); + const createClient = vi.fn(() => createUnleashMockClient());- isEnabledFeatureFlag: jest.fn(() => false) + isEnabledFeatureFlag: vi.fn(() => false)Also applies to: 33-33, 53-53, 77-79
apps/api/src/template/services/github-archive/github-archive.service.spec.ts (1)
90-95: Replacejest.spyOnwithvi.spyOnfor complete Vitest migration.♻️ Suggested refactor
+import { vi } from "vitest";- jest.spyOn(globalThis, "fetch").mockResolvedValue( + vi.spyOn(globalThis, "fetch").mockResolvedValue( new Response(new Uint8Array(zipBuffer), { status: 200, headers: { "Content-Type": "application/zip" } }) );apps/api/src/auth/services/auth.interceptor.spec.ts (1)
37-50: Replace Jest timer utilities with Vitest equivalents.The fake timer APIs should use
vi.useFakeTimers(),vi.setSystemTime(), andvi.useRealTimers()for a complete Vitest migration. Similarly, replacejest.fn()calls in the setup function withvi.fn().♻️ Example changes for timers
+import { vi } from "vitest";- jest.useFakeTimers(); + vi.useFakeTimers(); try { - jest.setSystemTime(new Date(Date.now() + 25 * 60 * 1000)); + vi.setSystemTime(new Date(Date.now() + 25 * 60 * 1000)); await callInterceptor(); await callInterceptor(); - jest.setSystemTime(new Date(Date.now() + 31 * 60 * 1000)); + vi.setSystemTime(new Date(Date.now() + 31 * 60 * 1000)); await callInterceptor(); await callInterceptor(); expect(di.resolve(UserRepository).markAsActive).toHaveBeenCalledTimes(2); } finally { - jest.useRealTimers(); + vi.useRealTimers(); }Also applies to: 102-114
apps/api/src/billing/services/wallet-settings/wallet-settings.service.spec.ts (1)
181-194: Replacejest.fn()withvi.fn()in setup function.♻️ Suggested refactor
+import { vi } from "vitest";const stripeService = mock<StripeService>({ - getDefaultPaymentMethod: jest.fn().mockResolvedValue(paymentMethod as PaymentMethod) + getDefaultPaymentMethod: vi.fn().mockResolvedValue(paymentMethod as PaymentMethod) }); // ... const walletReloadJobService = mock<WalletReloadJobService>({ - scheduleForWalletSetting: jest.fn().mockResolvedValue(jobId) + scheduleForWalletSetting: vi.fn().mockResolvedValue(jobId) });apps/api/src/certificate/services/certificate/certificate.service.spec.ts (1)
31-33: Replacejest.fn()withvi.fn()throughout for complete migration.Multiple
jest.fn()calls in tests and the setup function should be migrated tovi.fn()for consistency with Vitest.♻️ Example changes
+import { vi } from "vitest";In test cases:
const { service, ... } = setup({ - findWallet: jest.fn().mockResolvedValue(userWallet), - getCreateCertificateMsg: jest.fn().mockReturnValue(createCertificateMsg), - executeDerivedDecodedTxByUserId: jest.fn().mockResolvedValue({ code: 0, hash: "tx-hash", transactionHash: "tx-hash" }) + findWallet: vi.fn().mockResolvedValue(userWallet), + getCreateCertificateMsg: vi.fn().mockReturnValue(createCertificateMsg), + executeDerivedDecodedTxByUserId: vi.fn().mockResolvedValue({ code: 0, hash: "tx-hash", transactionHash: "tx-hash" }) });In setup function:
const mocks = { userWalletRepository: mock<UserWalletRepository>({ - accessibleBy: jest.fn().mockReturnThis(), - findOneByUserId: input?.findWallet ?? jest.fn() + accessibleBy: vi.fn().mockReturnThis(), + findOneByUserId: input?.findWallet ?? vi.fn() }), // ... similar changes for other mocks };Also applies to: 83-106
apps/api/test/services/test-database.service.ts (1)
27-27: Consider usingLoggerServiceinstead ofconsole.log.The coding guidelines specify using
LoggerServiceinstead ofconsole.log,console.warn, orconsole.error. This applies to lines 27, 53, 68, and 85.However, since this is test infrastructure code that runs before the application container is initialized, using
consolemay be intentional to avoid circular dependencies or premature container resolution.apps/api/src/auth/services/api-key/api-key-auth.service.spec.ts (2)
14-15: Incomplete migration: Jest APIs still in use.This test file still references Jest APIs that should be migrated to Vitest:
jest.Mocked<T>→ useMockProxy<T>fromvitest-mock-extendedjest.fn()→vi.fn()jest.resetAllMocks()→vi.resetAllMocks()While the compat shim maps
globalThis.jest = vi, the type annotations (jest.Mocked) won't work correctly without Jest's type definitions.♻️ Proposed migration to Vitest APIs
+import { vi } from "vitest"; +import type { MockProxy } from "vitest-mock-extended"; import { container } from "tsyringe"; import type { ApiKeyRepository } from "@src/auth/repositories/api-key/api-key.repository"; import type { CoreConfigService } from "@src/core/services/core-config/core-config.service"; ... describe("ApiKeyAuthService", () => { let service: ApiKeyAuthService; let apiKeyGenerator: ApiKeyGeneratorService; - let apiKeyRepository: jest.Mocked<ApiKeyRepository>; - let config: jest.Mocked<CoreConfigService>; + let apiKeyRepository: MockProxy<ApiKeyRepository>; + let config: MockProxy<CoreConfigService>; beforeEach(() => { - config = stub<CoreConfigService>({ get: jest.fn() }); + config = stub<CoreConfigService>({ get: vi.fn() }); config.get.mockReturnValue("test"); apiKeyGenerator = new ApiKeyGeneratorService(config); - apiKeyRepository = stub<ApiKeyRepository>({ findOneBy: jest.fn(), find: jest.fn() }); + apiKeyRepository = stub<ApiKeyRepository>({ findOneBy: vi.fn(), find: vi.fn() }); ... afterEach(() => { container.clearInstances(); - jest.resetAllMocks(); + vi.resetAllMocks(); });Also applies to: 18-18, 21-21, 28-28
17-24: Consider usingsetupfunction instead ofbeforeEach.Per coding guidelines, test files should use a
setupfunction at the bottom of the rootdescribeblock instead ofbeforeEach. This pattern avoids shared mutable state and improves test isolation.♻️ Proposed refactor to setup pattern
describe("ApiKeyAuthService", () => { - let service: ApiKeyAuthService; - let apiKeyGenerator: ApiKeyGeneratorService; - let apiKeyRepository: MockProxy<ApiKeyRepository>; - let config: MockProxy<CoreConfigService>; - - beforeEach(() => { - config = stub<CoreConfigService>({ get: vi.fn() }); - config.get.mockReturnValue("test"); - apiKeyGenerator = new ApiKeyGeneratorService(config); - apiKeyRepository = stub<ApiKeyRepository>({ findOneBy: vi.fn(), find: vi.fn() }); - - service = new ApiKeyAuthService(apiKeyGenerator, apiKeyRepository, config); - }); - afterEach(() => { container.clearInstances(); vi.resetAllMocks(); }); describe("validateApiKeyFromHeader", () => { it("should throw for undefined key", async () => { + const { service } = setup(); await expect(service.getAndValidateApiKeyFromHeader(undefined)).rejects.toThrow("Invalid API key"); }); // ... update other tests similarly }); + + function setup() { + const config = stub<CoreConfigService>({ get: vi.fn() }); + config.get.mockReturnValue("test"); + const apiKeyGenerator = new ApiKeyGeneratorService(config); + const apiKeyRepository = stub<ApiKeyRepository>({ findOneBy: vi.fn(), find: vi.fn() }); + const service = new ApiKeyAuthService(apiKeyGenerator, apiKeyRepository, config); + + return { service, apiKeyGenerator, apiKeyRepository, config }; + } });As per coding guidelines: "Use
setupfunction instead ofbeforeEachin test files."apps/api/test/vitest-jest-compat.ts (1)
5-5: Avoidas anycast per coding guidelines.The
as anycast violates the project's TypeScript guidelines. While this is a temporary compatibility layer, consider using a more specific type assertion or interface to maintain type safety.♻️ Alternative approach
import { vi } from "vitest"; + +// Minimal interface for Jest compatibility +interface JestCompat { + fn: typeof vi.fn; + spyOn: typeof vi.spyOn; + resetAllMocks: typeof vi.resetAllMocks; + clearAllMocks: typeof vi.clearAllMocks; + // Add other methods as needed +} // temporary compat layer to reduce changes /** `@deprecated` Use Vitest APIs directly instead of Jest */ -globalThis.jest = vi as any; +(globalThis as { jest?: JestCompat }).jest = vi;As per coding guidelines: "Never use type
anyor cast to typeany."apps/api/test/setup-functional-tests.ts (1)
55-65: Empty catch blocks may hide legitimate errors.The empty catch blocks at lines 58 and 63 silently swallow all errors. While the comments explain expected scenarios, unexpected errors would be hidden.
♻️ Consider logging unexpected errors
try { await container.dispose(); - } catch { - // could be disposed in tests + } catch (error) { + // Expected if already disposed in tests + if (!(error instanceof Error && error.message.includes("disposed"))) { + console.warn("Unexpected error disposing container:", error); + } } try { await semaphoreClient?.end(); - } catch { - // could fail if not initialized + } catch (error) { + // Expected if client not initialized + if (semaphoreClient) { + console.warn("Unexpected error closing semaphore client:", error); + } }
apps/api/src/notifications/services/notification-handler/notification.handler.spec.ts
Show resolved
Hide resolved
stalniy
force-pushed
the
refactor/migrate-to-vitest
branch
from
8e44a75 to
cb28f82
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/api/src/core/services/error/error.service.spec.ts (1)
13-13:⚠️ Potential issue | 🟠 MajorIncomplete migration:
jest.fn()should be replaced withvi.fn().The import was migrated to
vitest-mock-extended, but the test still usesjest.fn()in multiple places. For a complete Vitest migration, these should usevi.fn()from Vitest.🔧 Proposed fix
Add the
viimport at the top of the file:import "@test/mocks/logger-service.mock"; import type { LoggerService } from "@akashnetwork/logging"; import { faker } from "@faker-js/faker"; -import { mock } from "vitest-mock-extended"; +import { mock } from "vitest-mock-extended"; +import { vi } from "vitest";Then replace all
jest.fn()calls:- const cb = jest.fn().mockResolvedValue(result); + const cb = vi.fn().mockResolvedValue(result);- const cb = jest.fn().mockRejectedValue(error); + const cb = vi.fn().mockRejectedValue(error); - const onError = jest.fn(); + const onError = vi.fn();- const cb = jest.fn().mockRejectedValue(error); + const cb = vi.fn().mockRejectedValue(error);Also applies to: 25-27, 40-40
apps/api/src/core/services/hono-error-handler/hono-error-handler.service.spec.ts (1)
114-116:⚠️ Potential issue | 🔴 CriticalIncomplete migration:
jest.fn()will cause runtime error.The import was updated to
vitest-mock-extended, but line 115 still usesjest.fn(). Since Jest is no longer available, this will throwReferenceError: jest is not definedat runtime.🐛 Proposed fix
Add
viimport and replacejest.fn():+import { vi } from "vitest"; import { HTTPException } from "hono/http-exception"; import createHttpError from "http-errors"; import { mock } from "vitest-mock-extended";const mockContext = mock<AppContext>({ - body: jest.fn(() => new Response()) + body: vi.fn(() => new Response()) });apps/api/src/deployment/services/draining-deployment/draining-deployment.service.spec.ts (1)
66-71:⚠️ Potential issue | 🟠 MajorReplace
jest.*globals with Vitest'svi.*equivalents.The file imports
mockfromvitest-mock-extended(line 7) but continues to use Jest globals (jest.spyOn,jest.fn,jest.useFakeTimers,jest.setSystemTime,jest.useRealTimers). For a complete and idiomatic Vitest migration, these should be replaced with Vitest equivalents.Affected locations:
- Lines 66-71:
jest.spyOn(service, "findLeases")- Line 82:
jest.fn()- Line 196:
jest.spyOn(service, "calculateTopUpAmount")- Lines 395-396:
jest.useFakeTimers(),jest.setSystemTime()- Line 418:
jest.useRealTimers()Proposed fix
Add import at the top of the file:
+import { vi } from "vitest"; import { mock } from "vitest-mock-extended";Then replace all
jest.*calls withvi.*:- jest + vi .spyOn(service, "findLeases")- const callback = jest.fn(); + const callback = vi.fn();- jest.spyOn(service, "calculateTopUpAmount").mockResolvedValue(expectedTopUpAmount); + vi.spyOn(service, "calculateTopUpAmount").mockResolvedValue(expectedTopUpAmount);- jest.useFakeTimers(); - jest.setSystemTime(new Date("2025-01-01T12:00:00.000Z")); + vi.useFakeTimers(); + vi.setSystemTime(new Date("2025-01-01T12:00:00.000Z"));- jest.useRealTimers(); + vi.useRealTimers();Also applies to: 82-82, 196-196, 395-396, 418-418
apps/api/src/dashboard/services/stats/stats.service.spec.ts (1)
20-20:⚠️ Potential issue | 🟠 MajorReplace
jest.spyOnwithvi.spyOnand refactor to use the propersetupfunction pattern.The
vitest-jest-compat.tsshim (marked@deprecated) providesjestglobals, sojest.spyOncurrently works. However, migrate to native Vitestvi.spyOn(lines 20, 65, 158, 251, 290, 342).More importantly, the test violates the setup function pattern from coding guidelines: it uses
beforeEach(line 12) instead of a root-levelsetupfunction. Thesetup()function exists (line 364) but is called inside tests rather than as a describe-level helper. Also, the file importsmockfromvitest-mock-extendedbut doesn't use it, suggesting the jest-mock-extended pattern with dependency injection was not applied. Refactor to follow the established pattern: definesetupat the bottom of the rootdescribeblock, removebeforeEach, and use the returned service directly in tests.apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts (2)
15-15:⚠️ Potential issue | 🟡 MinorDrop “should” from the test description.
Line 15 uses “should …”, which conflicts with the required test naming style.✏️ Suggested tweak
- it("should generate a JWT token successfully", async () => { + it("generates a JWT token successfully", async () => {As per coding guidelines, use present simple, 3rd person singular for test descriptions without prepending 'should'.
65-69:⚠️ Potential issue | 🟡 MinorMake
setupaccept a typed options parameter.
Line 65 definessetup()without the required single typed parameter.♻️ Suggested refactor
- function setup() { - const walletId = faker.number.int({ min: 1, max: 10000 }); - const address = createAkashAddress(); - const jwtTokenValue = faker.string.alphanumeric(); + function setup({ + walletId = faker.number.int({ min: 1, max: 10000 }), + address = createAkashAddress(), + jwtTokenValue = faker.string.alphanumeric() + }: { + walletId?: number; + address?: string; + jwtTokenValue?: string; + } = {}) {As per coding guidelines, the setup function must accept a single parameter with inline type definition.
🤖 Fix all issues with AI agents
In `@apps/api/src/chain/services/block-http/block-http.service.spec.ts`:
- Around line 19-22: The setup function currently has no parameters; change it
to accept a single inline-typed parameter (e.g., setup = (opts: { /* optional
test options */ } = {}) => { ... }) so it matches the test guideline; update the
function signature that constructs the mocked BlockHttpServiceCommon (the mock
for BlockHttpServiceCommon with getCurrentHeight) to use that single
inline-typed parameter, ensure the function returns the created blockHttpService
without an explicit return type, avoid using shared state or beforeEach, and
keep the setup function at the bottom of the root describe block.
In `@apps/api/src/core/services/execution-context/execution-context.service.ts`:
- Around line 37-40: Remove the dead call to this.storage.getStore() inside the
storage.run callback in ExecutionContextService; the call's return value is
unused and has no side effects, so update the storage.run invocation (which
currently calls this.storage.run(new Map(), () => { this.storage.getStore();
return cb(); })) to simply run the callback with the new Map store (i.e., remove
the this.storage.getStore() statement) so storage.run(new Map(), () => cb()) is
used and behavior is preserved.
In
`@apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts`:
- Around line 87-88: Remove the redundant inline comment that duplicates the
code meaning near the JwtTokenManagerConstructor definition; specifically delete
the comment "// Create a constructor function for JwtTokenManager" that appears
immediately above the vi.fn().mockImplementation(function () { ... }) so the
test file provider-jwt-token.service.spec.ts contains only the mocked
constructor declaration (JwtTokenManagerConstructor) without the unnecessary
explanatory comment.
In `@apps/api/test/functional/managed-api-deployment-flow.spec.ts`:
- Line 49: The test title in the it.each call (it.each(authTypes)("should
execute a full deployment cycle with provider %s auth", ...)) uses a leading
"should"; update the string to present simple, 3rd person singular (e.g.,
"executes a full deployment cycle with provider %s auth") and apply the same
change to the other occurrence mentioned around line 178 so all test
descriptions follow the guideline.
- Line 8: Two test titles use the "should" prefix and must be rewritten in
present simple; locate the it(...) calls that currently use the strings "should
execute a full deployment cycle with provider %s auth" and "should maintain
read-only operations during blockchain node outages" and replace them with
"executes a full deployment cycle with provider %s auth" and "maintains
read-only operations during blockchain node outages" respectively so the test
descriptions use present simple 3rd person form.
In `@apps/api/test/setup-functional-env.ts`:
- Around line 5-32: The setup file is calling expect.getState().testPath at
module init which is undefined in setupFiles; remove use of testPath in the
top-level initialization and stop initializing per-test mnemonics there. Keep
TestWalletService and setOrGenerateWallet for helper use, but move the
FUNDING_WALLET_MNEMONIC initialization that depends on testPath into a per-test
setup helper (e.g., a function in this module that test files call from
beforeEach which reads expect.getState().testPath and then calls
setOrGenerateWallet("FUNDING_WALLET_MNEMONIC", () =>
testWalletService.getStoredMnemonic(testPath))). If you need global-only
mnemonics (DERIVATION_WALLET_MNEMONIC, OLD_MASTER_WALLET_MNEMONIC) you may keep
generating them at module init; ensure any test-path-dependent logic is executed
inside a test-time setup using expect.getState() instead of at top-level.
In `@apps/api/vitest.config.ts`:
- Around line 8-24: The jsc.target passed into swc.vite is case-sensitive;
update the config where jsc.target is set (currently using
tsconfig.compilerOptions.target) to normalize it to lowercase and provide a safe
fallback (e.g., "es2022") before passing to SWC; modify the block that sets
jsc.target inside defineConfig / swc.vite to compute something like const
normalizedTarget = (tsconfig.compilerOptions.target ||
"es2022").toString().toLowerCase() and use normalizedTarget for jsc.target so
uppercase TS values (e.g., "ES2022") won't break Vitest.
🧹 Nitpick comments (3)
apps/api/src/core/services/execution-context/execution-context.service.ts (1)
36-36: Consider removinganytype from the callback signature.Since
cb()is always invoked with no arguments, the spread parameter...args: any[]is unnecessary and violates the coding guideline against usingany. A simpler typed signature would be cleaner.♻️ Proposed refactor
-async runWithContext<R>(cb: (...args: any[]) => Promise<R>): Promise<R> { +async runWithContext<R>(cb: () => Promise<R>): Promise<R> { return this.storage.run(new Map(), cb); }As per coding guidelines: "Never use type
anyor cast to typeany. Always define the proper TypeScript types."apps/api/src/dashboard/services/stats/stats.service.spec.ts (1)
12-14: Consider moving cache clearing into thesetupfunction.The
beforeEachhook for cache clearing could be incorporated into thesetupfunction to align with the coding guidelines that recommend usingsetupinstead ofbeforeEach. The cache clearing could be performed at the start of eachsetup()call.♻️ Suggested refactor
describe(StatsService.name, () => { - beforeEach(() => { - cacheEngine.clearAllKeyInCache(); - }); - describe("getNetworkCapacity", () => {Then update the
setupfunction:function setup(input?: { PROVIDER_UPTIME_GRACE_PERIOD_MINUTES?: number }) { + cacheEngine.clearAllKeyInCache(); + const cosmosHttpService = mock<CosmosHttpService>();As per coding guidelines: "Use
setupfunction instead ofbeforeEachin test files."apps/api/src/core/services/job-queue/job-queue.service.spec.ts (1)
298-301: Return the PgBoss instance fromonto match API behavior.
pgBoss.onreturns the boss instance; returningundefinedcan break chaining ifJobQueueService.setup()relies on it. ConsidermockReturnThis()(ormockReturnValue(mocks.pgBoss)) after confirming usage.🔧 Suggested change
- on: jest.fn().mockReturnValue(undefined), + on: jest.fn().mockReturnThis(),
apps/api/src/core/services/execution-context/execution-context.service.ts
Outdated
Show resolved
Hide resolved
apps/api/src/provider/services/provider-jwt-token/provider-jwt-token.service.spec.ts
Show resolved
Hide resolved
stalniy
force-pushed
the
refactor/migrate-to-vitest
branch
from
cb28f82 to
1cf6cb4
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
apps/api/src/core/services/feature-flags/feature-flags.service.spec.ts (1)
18-18:⚠️ Potential issue | 🟡 MinorIncomplete migration:
jest.fn()should be replaced withvi.fn().The import was updated to use
vitest-mock-extended, but multiplejest.fn()calls remain throughout this file (lines 18, 33, 53, 77, 79, 107, 109, 137, 149). For a complete Vitest migration, these should be changed tovi.fn().Note: While Vitest can polyfill Jest globals, relying on this is implicit and fragile. Using
vi.fn()explicitly makes the migration complete.🔧 Proposed fix (showing first occurrence)
Add the import at the top of the file:
import { vi } from "vitest";Then replace all occurrences:
- const createClient = jest.fn(() => createUnleashMockClient()); + const createClient = vi.fn(() => createUnleashMockClient());Apply similar changes to all other
jest.fn()usages in this file.apps/api/test/functional/managed-api-deployment-flow.spec.ts (1)
575-578:⚠️ Potential issue | 🟡 MinorReplace
console.errorwithLoggerService.The codebase requires using
LoggerServicefor logging instead ofconsole.*methods.As per coding guidelines: Use
LoggerServiceinstead ofconsole.log,console.warn, orconsole.errorfor logging.🛠️ Suggested fix
+import { LoggerService } from "@src/core/services/logger/logger.service"; + +// ... within describe block, add logger instance: +const logger = new LoggerService({ context: "ManagedWalletAPIDeploymentFlowTest" }); + if (!data?.data?.leases) { - console.error("Cannot create lease", data); + logger.error({ data }, "Cannot create lease"); return undefined; }apps/api/src/deployment/services/active-deployments-count/active-deployments-count.service.spec.ts (1)
19-21:⚠️ Potential issue | 🟠 MajorIncomplete migration:
jest.fn()should bevi.fn().The import was updated to
vitest-mock-extended, but the code still usesjest.fn()throughout the file (lines 19, 20, 34, 51, 66, 69). These will fail at runtime since thejestglobal is not available in Vitest.🔧 Proposed fix
Add the
viimport and replace alljest.fn()calls:+import { vi } from "vitest"; import { mock } from "vitest-mock-extended";Then replace all occurrences:
- findOneByUserId: jest.fn().mockResolvedValue(userWallet), - countActiveByOwner: jest.fn().mockResolvedValue(activeCount) + findOneByUserId: vi.fn().mockResolvedValue(userWallet), + countActiveByOwner: vi.fn().mockResolvedValue(activeCount)Apply similar changes to lines 34, 51, 66, and 69.
apps/api/src/core/services/error/error.service.spec.ts (1)
13-13:⚠️ Potential issue | 🟠 MajorIncomplete migration:
jest.fn()should bevi.fn().The file still uses
jest.fn()on lines 13, 25, 27, and 40 after switching tovitest-mock-extended. Add theviimport fromvitestand replace alljest.fn()withvi.fn().apps/api/src/billing/services/managed-signer/managed-signer.service.spec.ts (1)
46-47:⚠️ Potential issue | 🟠 MajorIncomplete migration: extensive
jest.fn()usage throughout the file.This file has numerous
jest.fn()calls (e.g., lines 46, 59-61, 79-82, 107-115, and many more in the setup function at lines 471-516) that need to be replaced withvi.fn(). Additionally,jest.Mocktype casts on lines 208 and 219 should useMockfromvitest-mock-extended.🔧 Partial fix example
+import { vi } from "vitest"; +import type { Mock } from "vitest-mock-extended"; import { mock } from "vitest-mock-extended";- const publishedEvent = (domainEvents.publish as jest.Mock).mock.lastCall?.[0] as TrialDeploymentLeaseCreated; + const publishedEvent = (domainEvents.publish as Mock).mock.lastCall?.[0] as TrialDeploymentLeaseCreated;Replace all
jest.fn()withvi.fn()throughout the file.apps/api/test/mocks/config-service.mock.ts (1)
19-19:⚠️ Potential issue | 🟠 MajorIncomplete migration:
jest.MockedFunctiontype is not available in Vitest.The type cast on line 19 uses
jest.MockedFunctionwhich doesn't exist in the Vitest environment. Use a Vitest-compatible approach.🔧 Proposed fix
+import type { Mock } from "vitest-mock-extended"; import { mock } from "vitest-mock-extended";- (svc.get as unknown as jest.MockedFunction<(key: keyof ConfigOf<T>) => any>).mockImplementation(key => { + (svc.get as Mock<(key: keyof ConfigOf<T>) => unknown>).mockImplementation(key => {apps/api/src/healthz/services/healthz/healthz.service.spec.ts (1)
114-136:⚠️ Potential issue | 🟠 MajorIncomplete migration: Jest timer utilities and
jest.fn()need replacement.The file uses
jest.useFakeTimers(),jest.advanceTimersByTime(), andjest.useRealTimers()(lines 114, 128, 135, 154, 160, 185, 197) as well asjest.fn()(lines 203, 204). These should all use theviequivalents from Vitest.🔧 Proposed fix
+import { vi } from "vitest"; import { mock } from "vitest-mock-extended";Replace timer utilities:
- jest.useFakeTimers(); + vi.useFakeTimers(); // ... - jest.advanceTimersByTime(millisecondsInMinute + 1); + vi.advanceTimersByTime(millisecondsInMinute + 1); // ... - jest.useRealTimers(); + vi.useRealTimers();Replace mock functions:
- const dbHealthcheck = mock<DbHealthcheck>({ ping: jest.fn().mockResolvedValue(undefined) }); - const jobQueueHealthcheck = mock<JobQueueHealthcheck>({ ping: jest.fn().mockResolvedValue(undefined) }); + const dbHealthcheck = mock<DbHealthcheck>({ ping: vi.fn().mockResolvedValue(undefined) }); + const jobQueueHealthcheck = mock<JobQueueHealthcheck>({ ping: vi.fn().mockResolvedValue(undefined) });apps/api/src/auth/services/auth0/auth0.service.spec.ts (3)
78-111:⚠️ Potential issue | 🟡 MinorMove
setup()to the bottom of the root describe
setup()is currently inside the nested describe block, but the project rule requires it to be placed at the bottom of the rootdescribe.As per coding guidelines, “Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block.”♻️ Suggested move (no logic changes)
- describe("getUserByEmail", () => { - ... - function setup(input: { mockUsers?: GetUsers200ResponseOneOfInner[]; mockError?: Error }) { - const mockAuthConfig = mock<AuthConfigService>(); - mockAuthConfig.get.mockImplementation((key: string) => { - const config = { - AUTH0_M2M_DOMAIN: "test-domain.auth0.com", - AUTH0_M2M_CLIENT_ID: "test-client-id", - AUTH0_M2M_SECRET: "test-client-secret" - }; - return config[key as keyof typeof config]; - }); - - const mockGetByEmail = jest.fn(); - const mockManagementClient = { - usersByEmail: { - getByEmail: mockGetByEmail - } - } as unknown as ManagementClient; - - if (input.mockError) { - mockGetByEmail.mockRejectedValue(input.mockError); - } else { - mockGetByEmail.mockResolvedValue({ - data: input.mockUsers || [] - }); - } - - const auth0Service = new Auth0Service(mockManagementClient); - - return { - auth0Service, - mockGetByEmail, - mockManagementClient - }; - } - }); + describe("getUserByEmail", () => { + ... + }); + + function setup(input: { mockUsers?: GetUsers200ResponseOneOfInner[]; mockError?: Error }) { + const mockAuthConfig = mock<AuthConfigService>(); + mockAuthConfig.get.mockImplementation((key: string) => { + const config = { + AUTH0_M2M_DOMAIN: "test-domain.auth0.com", + AUTH0_M2M_CLIENT_ID: "test-client-id", + AUTH0_M2M_SECRET: "test-client-secret" + }; + return config[key as keyof typeof config]; + }); + + const mockGetByEmail = jest.fn(); + const mockManagementClient = { + usersByEmail: { + getByEmail: mockGetByEmail + } + } as unknown as ManagementClient; + + if (input.mockError) { + mockGetByEmail.mockRejectedValue(input.mockError); + } else { + mockGetByEmail.mockResolvedValue({ + data: input.mockUsers || [] + }); + } + + const auth0Service = new Auth0Service(mockManagementClient); + + return { + auth0Service, + mockGetByEmail, + mockManagementClient + }; + }
12-75:⚠️ Potential issue | 🟡 MinorRename test descriptions to present simple without “should”
The test titles still use “should …”, which violates the test naming convention.
As per coding guidelines, “Use present simple, 3rd person singular for test descriptions without prepending 'should'.”✏️ Suggested renames
- it("should return user when user is found", async () => { + it("returns user when user is found", async () => { @@ - it("should return null when no user is found", async () => { + it("returns null when no user is found", async () => { @@ - it("should return first user when multiple users are found", async () => { + it("returns first user when multiple users are found", async () => { @@ - it("should handle empty email string", async () => { + it("handles empty email string", async () => {
89-93:⚠️ Potential issue | 🟡 MinorMove
setup()function to bottom of rootdescribeblock and remove "should" prefix from test descriptionsThe
setup()function on line 74 is nested insidedescribe("getUserByEmail")but must be at the bottom of the rootdescribe(Auth0Service.name)block per test structure guidelines.Additionally, test descriptions on lines 12, 29, 42, and 65 start with "should"; they should use present simple, 3rd person singular without this prefix (e.g., "returns user when user is found" instead of "should return user when user is found").
🤖 Fix all issues with AI agents
In
`@apps/api/src/app/services/close-trial-deployment/close-trial-deployment.handler.spec.ts`:
- Line 1: The test file still uses Jest globals; add an import for the Vitest
runner (import { vi } from "vitest") at the top alongside the existing mock
import, then replace every occurrence of jest.fn() with vi.fn() in the spec
(instances used to create spies/mocks in the close-trial-deployment handler
tests). Ensure any variables or mock factories that previously called jest.fn()
(the mock callback/stub creations in this spec) now call vi.fn() so the tests
run under Vitest.
In `@apps/api/src/billing/services/tx-manager/tx-manager.service.spec.ts`:
- Line 3: The test file still uses jest.fn() which fails under Vitest; add an
import for vi (import { vi } from "vitest") near the existing mock import from
"vitest-mock-extended" and replace every occurrence of jest.fn() with vi.fn()
(there are ~15 occurrences across the file, e.g., in setups for
tx-manager.service.spec.ts mocks and spies). Ensure any variables or mocks
created with jest.fn() (used in tests for functions, callbacks, or service
stubs) are updated to vi.fn() so tests run under Vitest.
In `@apps/api/src/core/services/analytics/analytics.service.spec.ts`:
- Around line 1-2: The tests are calling vi.fn() inside the setup helper but vi
is not imported, causing a runtime error; add the Vitest import (for example
import { vi } from "vitest") at the top of the spec file so vi is defined, then
keep the existing setup function and its vi.fn() usages (the mocks created in
setup) unchanged.
In
`@apps/api/src/notifications/services/notification-data-resolver/notification-data-resolver.service.spec.ts`:
- Line 2: Tests in notification-data-resolver.service.spec.ts still use Jest
globals (e.g., jest.fn / jest.Mock types referenced around the setup and
assertions) which will be undefined under Vitest; replace all jest.* usages with
Vitest equivalents (use vi.fn() for function mocks, use the Vitest mock types or
the types provided by vitest-mock-extended where applicable) and update any type
annotations from jest.Mock / jest.Mocked to the appropriate Vitest-compatible
types or generics from vitest-mock-extended; keep the existing import of mock
from "vitest-mock-extended" and ensure calls in the setup/expect sections (the
occurrences you saw at lines ~24–25, ~70, ~93, ~109) use vi.fn and Vitest types
so the spec runs under Vitest.
🧹 Nitpick comments (1)
apps/api/src/billing/services/wallet-settings/wallet-settings.service.spec.ts (1)
4-4: Prefervi.fnto avoid relying on Jest globals with Vitest mocks.With
vitest-mock-extended, usingjest.fnties these tests to the Jest-compat shim and can break if that shim/types are removed. Usingvi.fnkeeps the mocks native to Vitest.Suggested update
-import { mock } from "vitest-mock-extended"; +import { vi } from "vitest"; +import { mock } from "vitest-mock-extended"; ... - const stripeService = mock<StripeService>({ - getDefaultPaymentMethod: jest.fn().mockResolvedValue(paymentMethod as PaymentMethod) - }); + const stripeService = mock<StripeService>({ + getDefaultPaymentMethod: vi.fn().mockResolvedValue(paymentMethod as PaymentMethod) + }); ... - const walletReloadJobService = mock<WalletReloadJobService>({ - scheduleForWalletSetting: jest.fn().mockResolvedValue(jobId) - }); + const walletReloadJobService = mock<WalletReloadJobService>({ + scheduleForWalletSetting: vi.fn().mockResolvedValue(jobId) + });Also applies to: 181-193
apps/api/src/app/services/close-trial-deployment/close-trial-deployment.handler.spec.ts
Show resolved
Hide resolved
...notifications/services/notification-data-resolver/notification-data-resolver.service.spec.ts
Show resolved
Hide resolved
2 participants
Why
vitest has almost the same coding interface and DX but faster and supports ESM out of the box.
Summary by CodeRabbit
Tests
Chores