chore: migrates deploy-web tests to vitest#2785
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2785 +/- ##
==========================================
+ Coverage 52.08% 53.63% +1.54%
==========================================
Files 1043 1019 -24
Lines 27453 23591 -3862
Branches 6357 5754 -603
==========================================
- Hits 14299 12653 -1646
+ Misses 12727 9533 -3194
- Partials 427 1405 +978
🚀 New features to boost your workflow:
|
251fece to
1ccb8e7
Compare
📝 WalkthroughWalkthroughMass migration of the deploy-web test suite from Jest to Vitest: Jest config and test tsconfig removed, Vitest config and setup added, package.json scripts/deps updated, ~100+ spec files and global test infra converted to Vitest APIs, plus two minor runtime/type and styling tweaks. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (30)
apps/deploy-web/src/services/feature-flag/feature-flag.service.spec.ts (1)
69-80:⚠️ Potential issue | 🟡 MinorIncorrect type for
isEnabledparameter.The
isEnabledparameter is typed asMockbut used as a boolean (!!options?.isEnabled). This type is incorrect and the parameter appears unused by any test.Proposed fix
- function setup(options?: { enableAll?: boolean; isEnabled?: Mock }): { + function setup(options?: { enableAll?: boolean; isEnabled?: boolean }): {Alternatively, if this parameter is no longer needed, consider removing it entirely:
- function setup(options?: { enableAll?: boolean; isEnabled?: Mock }): { + function setup(options?: { enableAll?: boolean }): { service: FeatureFlagService; unleash: typeof unleashModule; flagsClient: MockProxy<UnleashClient>; } { ... - flagsClient.isEnabled.mockReturnValue(!!options?.isEnabled); + flagsClient.isEnabled.mockReturnValue(false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/services/feature-flag/feature-flag.service.spec.ts` around lines 69 - 80, The setup helper's options incorrectly type isEnabled as Mock while it is used as a boolean and appears unused elsewhere; update the setup signature (function setup) to accept options?: { enableAll?: boolean; isEnabled?: boolean } and change the flagsClient.isEnabled.mockReturnValue call to use options?.isEnabled ?? false (or remove the isEnabled option entirely and always mock a default boolean), ensuring any tests that rely on isEnabled are updated to pass a boolean when calling setup.apps/deploy-web/src/hoc/registered-users-only/registered-users-only.hoc.spec.tsx (1)
59-59: 🛠️ Refactor suggestion | 🟠 MajorAvoid
anytype in props definition.The
Record<string, any>type violates the coding guideline to never useany. Define a specific type for the props being tested.Suggested fix
- function setup({ isRegistered = true, props = {} }: { isRegistered?: boolean; props?: Record<string, any>; fallbackComponent?: React.ComponentType }) { + function setup({ isRegistered = true, props = {} }: { isRegistered?: boolean; props?: { testProp?: string }; fallbackComponent?: React.ComponentType }) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hoc/registered-users-only/registered-users-only.hoc.spec.tsx` at line 59, The test helper setup function currently types props as Record<string, any>, which violates the no-any rule; update the signature of setup to use a concrete props type (e.g., define an interface or type alias like TestProps with the actual prop keys used in these tests) or, if the shape varies, use Record<string, unknown> and narrow/cast where needed; update references in the tests to match the new TestProps type so TypeScript enforces correct prop shapes for the setup function.apps/deploy-web/src/lib/auth0/setSession/setSession.spec.ts (1)
12-22:⚠️ Potential issue | 🟡 MinorRefactor to required
setuphelper and update test description tense.
Line 13 uses “can set session” (not present simple 3rd person singular), and the test should use asetuphelper at the bottom of the root describe.🔧 Suggested change
describe(setSession.name, () => { - it("can set session", async () => { - const req = mock<NextApiRequest>(); - const res = mock<NextApiResponse>(); - const session = new Session({ - someClaim: "someValue" - }); + it("sets session", async () => { + const { req, res, session } = setup({}); await setSession(req, res, session); expect(await getSession(req, res)).toEqual(session); }); + + function setup(_: {}) { + const req = mock<NextApiRequest>(); + const res = mock<NextApiResponse>(); + const session = new Session({ + someClaim: "someValue" + }); + return { req, res, session }; + } });As per coding guidelines, “Use
setupfunction 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,” and “Use present simple, 3rd person singular for test descriptions without prepending ‘should’.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/lib/auth0/setSession/setSession.spec.ts` around lines 12 - 22, Update the test to use a bottom-placed setup helper and change the test description to present simple 3rd person singular; replace the inline creation of req/res/session in the spec with a setup function at the bottom of the root describe that accepts a single inline-typed parameter, creates fresh mock NextApiRequest/NextApiResponse and a new Session instance (references: setSession, getSession, Session), returns an object containing { req, res, session }, and does not declare an explicit return type; then update the it(...) description from "can set session" to "sets session" and call setup from the test to get req/res/session before invoking setSession and asserting getSession(req, res) equals session.apps/deploy-web/src/utils/getValidInternalReturnToUrl/getValidInternalReturnToUrl.spec.ts (1)
53-53: 🛠️ Refactor suggestion | 🟠 MajorCall setup() inside each test to avoid shared state.
The
setup()function is currently called at the nested describe block scope (lines 53, 84, 130, 149, 151, 156), creating shared constants that are reused across multiple tests. This violates the guideline to "avoid shared state" and can lead to test interdependencies.Each test should call
setup()independently to ensure test isolation:♻️ Example fix for one describe block
describe("same-origin URLs", () => { - const { window: mockWindow } = setup("https://console.akash.network"); - it("returns the full URL for same-origin absolute URL", () => { + const { window: mockWindow } = setup("https://console.akash.network"); expect(getValidInternalReturnToUrl("https://console.akash.network/dashboard", mockWindow)).toBe("https://console.akash.network/dashboard"); }); it("returns the full URL with path and query", () => { + const { window: mockWindow } = setup("https://console.akash.network"); expect(getValidInternalReturnToUrl("https://console.akash.network/dashboard?tab=settings", mockWindow)).toBe( "https://console.akash.network/dashboard?tab=settings" ); }); // Apply same pattern to remaining tests... });Apply this pattern to all describe blocks that currently call
setup()at the block scope (lines 53, 84, 130, 149, 151, 156). As per coding guidelines: "Thesetupfunction must... avoid shared state".Also applies to: 84-84, 130-130, 149-149, 151-151, 156-156, 173-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/utils/getValidInternalReturnToUrl/getValidInternalReturnToUrl.spec.ts` at line 53, The tests currently call setup() once at the describe block scope creating shared constants like mockWindow; change each test so it calls setup() inside its own it/test body to avoid shared state—remove any describe-level const { window: mockWindow } = setup(...) and instead invoke const { window: mockWindow } = setup(...) at the start of each test that needs it (for all describe blocks that currently call setup() at block scope), ensuring each test gets a fresh mockWindow and any other values returned by setup().apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx (2)
31-38:⚠️ Potential issue | 🟡 MinorUse inline type definition for
setupparameter.The
setupfunction acceptsDailyUsageBarChartPropsas a named type, but the test guideline requires an inline type definition.Suggested fix
- async function setup(props: DailyUsageBarChartProps) { + async function setup(props: { + isFetching: DailyUsageBarChartProps["isFetching"]; + data: DailyUsageBarChartProps["data"]; + }) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx` around lines 31 - 38, The test's setup function currently types its parameter using the named type DailyUsageBarChartProps; change it to an inline type definition instead by replacing the signature of setup to accept an object shaped like DailyUsageBarChartProps inline (e.g., setup(props: { /* same fields as DailyUsageBarChartProps */ })) so the parameter uses an inline type; update any references inside the function accordingly (setup, DailyUsageBarChart, MockComponents, DEPENDENCIES remain the same).
10-12:⚠️ Potential issue | 🟡 MinorUse
queryByRoleinstead ofgetByRolein deploy-web test expectations.This violates the coding guideline for deploy-web specs. Change
getByRole("status")toqueryByRole("status").Suggested fix
- expect(screen.getByRole("status")).toBeInTheDocument(); + expect(screen.queryByRole("status")).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx` around lines 10 - 12, In the "shows a spinner when fetching" test in DailyUsageBarChart.spec.tsx (the test that calls setup({ isFetching: true, data: [] })), replace the use of screen.getByRole("status") with screen.queryByRole("status") in the expect assertion so the spec follows the deploy-web testing guideline; keep the rest of the test (including setup and matcher toBeInTheDocument()) unchanged.apps/deploy-web/src/queries/useManagedWalletQuery.spec.tsx (2)
14-15:⚠️ Potential issue | 🟡 MinorUse present simple, third-person test descriptions (no “should”).
Please update the
it(...)strings to match the project’s test naming convention.✏️ Suggested change
- it("should fetch wallet when userId is provided", async () => { + it("fetches wallet when userId is provided", async () => { @@ - it("should not fetch when userId is not provided", () => { + it("does not fetch when userId is not provided", () => { @@ - it("should create wallet and update query cache", async () => { + it("creates wallet and updates query cache", async () => {As per coding guidelines: Use present simple, 3rd person singular for test descriptions without prepending 'should'.
Also applies to: 34-35, 48-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useManagedWalletQuery.spec.tsx` around lines 14 - 15, Update the test titles in useManagedWalletQuery.spec.tsx to use present simple, third-person singular and remove the word "should"; e.g., change the it(...) strings in the test cases (including the one around the current snippet for "it('should fetch wallet when userId is provided')" and the other occurrences at lines 34-35 and 48-49) to concise forms like "fetches wallet when userId is provided" or "returns null when userId is missing" so they follow the project's naming convention for tests.
27-31:⚠️ Potential issue | 🟠 MajorRemove "should" from test descriptions and use
waitForfrom@testing-library/reactinstead ofvi.waitFor.Test descriptions should use present simple, 3rd person singular without "should" prefix (e.g., "fetches wallet when userId is provided" instead of "should fetch wallet when userId is provided"). Additionally,
vi.waitFordoesn't automatically wrap assertions in React'sact()call. UsewaitForfrom@testing-library/reactinstead, which handles act-wrapping correctly for React state updates.🔧 Suggested changes
-import { act } from "@testing-library/react"; +import { waitFor } from "@testing-library/react"; @@ - it("should fetch wallet when userId is provided", async () => { + it("fetches wallet when userId is provided", async () => { const mockData = { userId: faker.string.uuid(), address: faker.finance.ethereumAddress() }; const managedWalletService = mock<ManagedWalletHttpService>({ getWallet: vi.fn().mockResolvedValue(mockData) }); const { result } = setupQuery(() => useManagedWalletQuery(mockData.userId), { services: { managedWalletService: () => managedWalletService } }); - await vi.waitFor(() => { + await waitFor(() => { expect(managedWalletService.getWallet).toHaveBeenCalledWith(mockData.userId); expect(result.current.isSuccess).toBe(true); expect(result.current.data).toEqual(mockData); }); }); - it("should not fetch when userId is not provided", () => { + it("does not fetch when userId is not provided", () => {- it("should create wallet and update query cache", async () => { + it("creates wallet and updates query cache", async () => { // ... await act(async () => result.current.mutation.mutateAsync(mockData.userId)); - await vi.waitFor(() => { + await waitFor(() => { expect(mockManagedWalletService.createWallet).toHaveBeenCalledWith(mockData.userId); expect(result.current.mutation.isSuccess).toBe(true); expect(result.current.queryClient.getQueryData(["MANAGED_WALLET", mockData.userId])).toEqual(mockData); }); });Also applies to: 45, 59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useManagedWalletQuery.spec.tsx` around lines 27 - 31, Tests use "should" in their descriptions and call vi.waitFor which doesn't wrap assertions with React's act; rename test descriptions to present simple third-person (e.g., "fetches wallet when userId is provided") and replace uses of vi.waitFor with waitFor imported from '@testing-library/react' in the spec file (update all occurrences referenced, e.g., the assertions around managedWalletService.getWallet and result.current checks and the other two blocks at the same pattern) so assertions are act-wrapped properly.apps/deploy-web/src/components/shared/RemoteApiError/RemoteApiError.spec.tsx (2)
11-12:⚠️ Potential issue | 🟡 MinorUse
queryByTextin expectations per test guidelines.Replace
getByTextwithqueryByTextin these expectations to match the repo’s testing conventions.🔧 Suggested change
- const { getByText } = setup({ error: new Error("Test error") }); - expect(getByText(/unexpected error/i)).toBeInTheDocument(); + const { queryByText } = setup({ error: new Error("Test error") }); + expect(queryByText(/unexpected error/i)).toBeInTheDocument();- const { getByText } = setup({ error }); - expect(getByText(/Error message from API/i)).toBeInTheDocument(); + const { queryByText } = setup({ error }); + expect(queryByText(/Error message from API/i)).toBeInTheDocument();Based on learnings "Use
queryBymethods instead ofgetBymethods in test expectations".Also applies to: 23-24
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/shared/RemoteApiError/RemoteApiError.spec.tsx` around lines 11 - 12, The test uses getByText from setup but our convention is to use queryByText in expectations; update the destructuring in the failing specs to use queryByText instead of getByText (e.g., change "const { getByText } = setup(...)" to "const { queryByText } = setup(...)" and update the expect calls that use getByText(/unexpected error/i) to queryByText(/unexpected error/i)); apply the same change for the other occurrence referenced (lines 23-24) so all expectations use queryByText.
21-21:⚠️ Potential issue | 🟠 MajorReplace
anycast with proper Axios config type.
config: {} as anyviolates the no-anyrule. The response config field should useInternalAxiosRequestConfig(the normalized config type after Axios processing).🔧 Suggested change
-import { AxiosError } from "axios"; +import { AxiosError, type InternalAxiosRequestConfig } from "axios"; ... - config: {} as any + config: {} as InternalAxiosRequestConfig🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/shared/RemoteApiError/RemoteApiError.spec.tsx` at line 21, Replace the unsafe cast "config: {} as any" with the proper Axios normalized config type: import and use InternalAxiosRequestConfig and write "config: {} as InternalAxiosRequestConfig"; update the test file RemoteApiError.spec.tsx to import { InternalAxiosRequestConfig } from 'axios' (or from the axios types export in your project) and apply that type to the config object so the no-any rule is satisfied.apps/deploy-web/src/services/analytics/analytics.service.spec.ts (3)
117-200:⚠️ Potential issue | 🟡 MinorRemove “should” from test descriptions.
Multiple
it("should …")cases in thetracksuite should be converted to present simple (e.g.,it("tracks events in both GA and Amplitude when no target specified", …)).As per coding guidelines, "Use present simple, 3rd person singular for test descriptions without prepending 'should'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/services/analytics/analytics.service.spec.ts` around lines 117 - 200, Rename the test description strings in the track test suite by removing the leading "should" and converting them to present simple 3rd person singular (e.g., change it("should track events in both GA and Amplitude when no target specified", ...) to it("tracks events in both GA and Amplitude when no target specified", ...)); update all similar it(...) descriptions in this file's track suite (including the cases referencing service.track, the GA transform case, the navigate_tab case, and the target-specific case) so they follow the guideline.
77-98:⚠️ Potential issue | 🟡 MinorRemove “should” from test descriptions.
Use present simple, 3rd person singular (e.g.,
it("identifies user in both GA and Amplitude", …)).As per coding guidelines, "Use present simple, 3rd person singular for test descriptions without prepending 'should'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/services/analytics/analytics.service.spec.ts` around lines 77 - 98, Update the two test description strings in the specs that start with "should" to use present simple third-person singular without "should" (e.g., change it("should identify user in both GA and Amplitude", ...) to it("identifies user in both GA and Amplitude", ...) and similarly change it("should only identify in enabled services", ...) to it("identifies only in enabled services", ...)); leave the test bodies, mocks (identify, setUserId, gtag), the setup(...) call and service.identify(...) unchanged.
15-31:⚠️ Potential issue | 🟡 MinorRename nested describe label to method name or “when …”.
describe("initialization", …)doesn’t follow the required naming convention. Considerdescribe("when initializing", …)or a method name if it exists.As per coding guidelines, "Use either a method name or a condition starting with 'when' for nested suite descriptions in tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/services/analytics/analytics.service.spec.ts` around lines 15 - 31, Rename the nested test suite label that currently reads describe("initialization", ...) to follow the project's naming convention—use either a method name or a condition starting with "when", e.g. describe("when initializing", ...) or describe("identify", ...) depending on intent; update the string only (leave the tests and setup/teardown intact) so the suite name for the block containing the Amplitude initialization tests matches the required pattern.apps/deploy-web/src/queries/useLeaseQuery.spec.tsx (4)
123-123:⚠️ Potential issue | 🟡 MinorUse
<Subject>.namefor the root describe.Switch the root suite to use the subject’s
.nameso renames are refactor-safe.Suggested change
-describe("useLeaseQuery", () => { +describe(useLeaseQuery.name, () => {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.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useLeaseQuery.spec.tsx` at line 123, The root test suite description is hardcoded as "useLeaseQuery"; change it to use the subject's name so renames are refactor-safe by replacing the string literal with useLeaseQuery.name in the top-level describe call (the suite that currently reads describe("useLeaseQuery", () => {)). Update the describe that references useLeaseQuery to call describe(useLeaseQuery.name, ...) so the test suite tracks symbol renames automatically.
358-385:⚠️ Potential issue | 🟡 MinorAlign helper with the required
setuppattern and placement.The helper is named
setupLeaseStatusand is nested inside a childdescribe. The guideline requires asetupfunction at the bottom of the root describe block. Consider renaming tosetup, moving it to the bottom of the rootdescribe, and keeping the inline parameter type.As per coding guidelines: Use
setupfunction 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.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useLeaseQuery.spec.tsx` around lines 358 - 385, Rename the nested helper setupLeaseStatus to setup, move it to the bottom of the root describe block, and change its signature to accept a single inline-typed parameter (same shape as the current optional input) while returning the created test harness object without an explicit return type; inside the new setup use the same construction that references USE_LEASE_STATUS_DEPENDENCIES, useProviderCredentials, useLeaseStatus and setupQuery (keeping the inline providerCredentials default), ensure services providers (providerProxy, certificatesService, etc.) are provided from the single parameter, and avoid any shared mutable state so the helper creates and returns a fresh object each call.
125-235:⚠️ Potential issue | 🟡 MinorRename test titles to present simple without “should.”
Multiple
it(...)titles still start with “should …”. Please update them to present simple (e.g., “returns null…”, “fetches leases…”).Example pattern
- it("should return null when deployment is not provided", async () => { + it("returns null when deployment is not provided", async () => {As per coding guidelines: Use present simple, 3rd person singular for test descriptions without prepending 'should'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useLeaseQuery.spec.tsx` around lines 125 - 235, Rename the test titles in this spec from imperative "should ..." to present simple 3rd-person singular; update each it(...) description strings in useLeaseQuery.spec.tsx (e.g., the blocks for useDeploymentLeaseList and useAllLeases) so "should return null when deployment is not provided" -> "returns null when deployment is not provided", "should fetch leases when deployment is provided" -> "fetches leases when deployment is provided", "should provide a remove function that clears the query" -> "provides a remove function that clears the query", "should return null when address is not provided" -> "returns null when address is not provided", "should fetch all leases when address is provided" -> "fetches all leases when address is provided", and "should use the correct query key" -> "uses the correct query key", leaving test logic and identifiers (useDeploymentLeaseList, useAllLeases, QueryKeys.getLeasesKey, leaseToDto) unchanged.
232-232:⚠️ Potential issue | 🟡 MinorRemove
anycast from the leaseToDto call.
undefined as anyviolates the coding guideline against usinganytypes. TheleaseToDtofunction signature (deployment: Pick<RpcDeployment, "groups">) doesn't mark the second parameter as optional, yet its implementation handles falsy values. Either update the function signature to make the parameter optional (deployment?: Pick<RpcDeployment, "groups">) or create a properly typed mock object instead of casting toany.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useLeaseQuery.spec.tsx` at line 232, The test is using an `undefined as any` cast when calling leaseToDto; remove the `any` cast and fix the typing properly by either making leaseToDto's second parameter optional in its signature (change from `deployment: Pick<RpcDeployment, "groups">` to `deployment?: Pick<RpcDeployment, "groups">`) since the implementation already accepts falsy values, or construct and pass a real mock that matches `Pick<RpcDeployment, "groups">` instead of `undefined as any` in the test (update the call in the spec where you invoke leaseToDto(mockLeases[0], ...) to use the chosen approach).apps/deploy-web/src/components/auth/SignUpButton/SignUpButton.spec.tsx (1)
12-22:⚠️ Potential issue | 🟡 MinorUse
queryBy*in expectations instead ofgetBy*.The expectations at Line 15 and Line 21 use
getByText, which violates the deploy-web test guideline.
As per coding guidelines, “UsequeryBymethods instead ofgetBymethods in test expectations”.Suggested change
- expect(screen.getByText("Sign up")).toBeInTheDocument(); + expect(screen.queryByText("Sign up")).toBeInTheDocument(); - expect(screen.getByText("Custom Sign Up Text")).toBeInTheDocument(); + expect(screen.queryByText("Custom Sign Up Text")).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/auth/SignUpButton/SignUpButton.spec.tsx` around lines 12 - 22, Change the test assertions to use queryByText instead of getByText: in the "renders with default text when no children provided" test replace screen.getByText("Sign up") with screen.queryByText("Sign up"), and in the "renders with custom children content" test replace screen.getByText("Custom Sign Up Text") with screen.queryByText("Custom Sign Up Text"); keep the existing toBeInTheDocument and negative query assertion as-is so the expectations follow the deploy-web guideline and still assert presence/absence correctly for setup().apps/deploy-web/src/queries/useBitBucketQuery.spec.tsx (1)
22-22:⚠️ Potential issue | 🟡 MinorUse
<Subject>.namefor the root describe label.The root suite uses a hardcoded string, which breaks the test guideline that enables automated refactors. Consider dropping the wrapper and using each hook’s
.nameat the top level (or otherwise ensure the root describe uses.name).
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”.🔧 Example direction (apply pattern to all hook suites)
-describe("useBitBucketQuery", () => { - describe("useBitFetchAccessToken", () => { +describe(useBitFetchAccessToken.name, () => { it("fetches access token and update token state", async () => { // ... }); }); -});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useBitBucketQuery.spec.tsx` at line 22, The root test suite uses a hardcoded string instead of the function's .name which breaks automated refactors; update the top-level describe to use useBitBucketQuery.name (i.e., replace the string "useBitBucketQuery" with useBitBucketQuery.name) so the suite label is tied to the actual exported hook name; apply the same pattern for any other hook suites.apps/deploy-web/src/queries/useTemplateQuery.spec.tsx (2)
125-130:⚠️ Potential issue | 🟡 MinorConsolidate to a single
setupat the bottom of the root describe.There are multiple nested
setuphelpers; the guideline requires a singlesetupfunction at the bottom of the rootdescribe, with inline-typed parameters and no shared state. Consider one root-levelsetupthat accepts options (services, wrapper, templateId, etc.) and reuse it across suites. 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.Also applies to: 166-170, 224-228, 265-270, 308-313, 351-356, 421-425
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useTemplateQuery.spec.tsx` around lines 125 - 130, Consolidate the multiple nested setup helpers into a single root-level setup function placed at the bottom of the root describe that creates and returns the object under test; implement it to call setupQuery(() => useUserFavoriteTemplates(), ...) and accept a single inline-typed options parameter (e.g., { services?: ServicesProviderProps["services"]; user?: UserProfile; wrapper?: React.FC; templateId?: string }) rather than using shared state or beforeEach, ensure it constructs the wrapper using UserProvider (defaulting user to { email: "test@akash.network" } if not provided) and forwards services from options, and then update all test suites (references around lines ~125, 166, 224, 265, 308, 351, 421) to invoke this single setup with appropriate options instead of their local setup helpers.
58-58:⚠️ Potential issue | 🟡 MinorUpdate hardcoded describe block labels to use
.namefrom hook exports.The root
describe("useTemplateQuery", ...)and several nested describe blocks use hardcoded strings instead of.namereferences. Update the following lines:
- Line 58: Root describe should reference an exported hook (e.g.,
useUserTemplates.nameif this is the primary subject) or keep it as a grouping describe only if intentional- Line 91: Use
useUserFavoriteTemplates.nameinstead of"useUserFavoriteTemplates"- Line 273: Use
useAddFavoriteTemplate.nameinstead of"useAddFavoriteTemplate"- Line 316: Use
useRemoveFavoriteTemplate.nameinstead of"useRemoveFavoriteTemplate"- Line 359: Use
useTemplates.nameinstead of"useTemplates"This enables automated refactoring tools to track all references when hooks are renamed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useTemplateQuery.spec.tsx` at line 58, Replace hardcoded describe labels with the exported hook .name values so refactors follow renames: change the root describe to reference the primary hook export (e.g., useUserTemplates.name) or leave as a generic grouping if intentional, and replace the nested describe strings with useUserFavoriteTemplates.name, useAddFavoriteTemplate.name, useRemoveFavoriteTemplate.name, and useTemplates.name respectively; update the describe calls that reference those hooks so they use the .name property of the exported functions (e.g., useUserFavoriteTemplates.name) instead of raw string literals.apps/deploy-web/src/components/alerts/NotificationChannelsListView/NotificationChannelsListView.spec.tsx (1)
99-113:⚠️ Potential issue | 🟡 MinorInline the
setupparameter type.
The setup helper should accept a single parameter with an inline type definition.✅ Suggested fix
-function setup(props: Partial<NotificationChannelsListViewProps> = {}) { +function setup( + props: { + pagination?: NotificationChannelsListViewProps["pagination"]; + data?: NotificationChannelsListViewProps["data"]; + isLoading?: NotificationChannelsListViewProps["isLoading"]; + onRemove?: NotificationChannelsListViewProps["onRemove"]; + removingIds?: NotificationChannelsListViewProps["removingIds"]; + onPaginationChange?: NotificationChannelsListViewProps["onPaginationChange"]; + isError?: NotificationChannelsListViewProps["isError"]; + } = {} +) {As per coding guidelines: “Use
setupfunction instead ofbeforeEach… Thesetupfunction must… accept a single parameter with inline type definition.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelsListView/NotificationChannelsListView.spec.tsx` around lines 99 - 113, The setup helper currently types its parameter as Partial<NotificationChannelsListViewProps>; replace that with an inline type definition for the single props parameter so it conforms to the guideline. Update the setup signature (function setup(...)) to accept props: Partial<{ pagination: { page: number; limit: number; total: number; totalPages: number }; data: ReturnType<typeof buildNotificationChannel>[]; isLoading: boolean; onRemove: jest.Mock | typeof vi.fn; removingIds: Set<string | number>; onPaginationChange: jest.Mock | typeof vi.fn; isError: boolean }> = {} and keep the rest of the function body unchanged (merge defaults with props and return render helpers) so references to setup and NotificationChannelsListViewProps are preserved but the parameter type is inlined.apps/deploy-web/src/components/billing-usage/PaymentMethodsView/PaymentMethodsView.spec.tsx (1)
75-110:⚠️ Potential issue | 🟡 MinorRename nested
describeblocks to match suite naming rules.Nested suite descriptions like "Loading State" and "Empty State" should be phrased as a method name or start with "when" (e.g., "when loading", "when empty").
As per coding guidelines: `**/*.spec.{ts,tsx}`: Use either a method name or a condition starting with 'when' for nested suite descriptions in tests.📝 Example rename
-describe("Loading State", () => { +describe("when loading", () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/billing-usage/PaymentMethodsView/PaymentMethodsView.spec.tsx` around lines 75 - 110, The nested test suite titles "Loading State" and "Empty State" violate naming rules; rename these describe blocks to be a method name or start with "when" (e.g., change describe("Loading State") to describe("when loading") and describe("Empty State") to describe("when empty")), ensure any other nested suites like describe("Payment Methods Display") follow the same convention, and update any related test descriptions if needed so they read naturally under the new suite names.apps/deploy-web/src/components/billing-usage/PaymentPopup/PaymentPopup.spec.tsx (1)
582-587:⚠️ Potential issue | 🟡 MinorSwitch all
getBy*methods toqueryBy*in test expectations to align with the project's coding guidelines forapps/deploy-web/**/*.spec.tsxfiles. This includes all instances withinvi.waitForblocks.Affected locations: 582–587, 614–616, 624–626, 652–654, 662–664, 690–692, 700–702.
Example adjustment
await vi.waitFor(() => { - expect(screen.getByTestId("alert")).toBeInTheDocument(); - expect(screen.getByText("Payment Error")).toBeInTheDocument(); - expect(screen.getByText("Your card was declined.")).toBeInTheDocument(); - expect(screen.getByText("Please try a different card.")).toBeInTheDocument(); + expect(screen.queryByTestId("alert")).toBeInTheDocument(); + expect(screen.queryByText("Payment Error")).toBeInTheDocument(); + expect(screen.queryByText("Your card was declined.")).toBeInTheDocument(); + expect(screen.queryByText("Please try a different card.")).toBeInTheDocument(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/billing-usage/PaymentPopup/PaymentPopup.spec.tsx` around lines 582 - 587, In PaymentPopup.spec.tsx update the assertions inside the vi.waitFor blocks to use queryBy* instead of getBy* (e.g., replace screen.getByTestId("alert"), screen.getByText("Payment Error"), screen.getByText("Your card was declined."), screen.getByText("Please try a different card.") and all other occurrences) so they return null when not present and avoid throwing; search for all vi.waitFor usages in the file (including the blocks that currently assert for error/success messages) and change getByTestId/getByText/getByRole to queryByTestId/queryByText/queryByRole respectively, keeping the existing expect(...).toBeInTheDocument()/toBeNull() assertions as-is.apps/deploy-web/src/components/alerts/NotificationChannelCreateContainer/NotificationChannelFormContainer.spec.tsx (1)
22-41:⚠️ Potential issue | 🟡 MinorUse
queryByTestIdinstead ofgetByTestIdin test expectations.This spec uses
getByTestIdin expectations, but the project's official rule requiresqueryBy*methods for deploy-web spec files.queryByis safer for testing both presence and absence of elements.🔁 Suggested changes
- expect(screen.getByTestId("notification-channel-create-success-notification")).toBeInTheDocument(); + expect(screen.queryByTestId("notification-channel-create-success-notification")).toBeInTheDocument();- expect(screen.getByTestId("notification-channel-create-error-notification")).toBeInTheDocument(); + expect(screen.queryByTestId("notification-channel-create-error-notification")).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelCreateContainer/NotificationChannelFormContainer.spec.tsx` around lines 22 - 41, The test uses screen.getByTestId("notification-channel-create-success-notification") which violates the project's spec rule; change this assertion to use screen.queryByTestId("notification-channel-create-success-notification") and adjust the expectation to check for non-null/defined presence (or truthiness) as appropriate, keeping the surrounding vi.waitFor block and the requestFn assertions intact so the rest of the test (including the expect(requestFn).toHaveBeenCalledWith(...) checks) remains unchanged.apps/deploy-web/src/utils/domUtils.spec.ts (1)
32-66:⚠️ Potential issue | 🟡 MinorMove
setupto the bottom of the rootdescribe.The
setuphelper is nested insidedescribe("downloadCsv"), but the test guideline requiressetupto live at the bottom of the rootdescribeblock. Please lift it to the root and pass options as needed.
As per coding guidelines, “Usesetupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock…”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/utils/domUtils.spec.ts` around lines 32 - 66, The setup helper is declared inside describe("downloadCsv") but must be moved to the bottom of the root describe block; lift the function declaration out of the inner describe to the bottom of the top-level describe, keep its return shape (createObjectURLMock, setAttributeMock, appendChildMock, removeChildMock, createElementMock) and update each test in describe("downloadCsv") to call setup() at the start (passing any options if needed) instead of relying on the inner-scoped helper; ensure references to global.URL.createObjectURL, document.createElement spy, and document.body append/remove spies remain intact and that the tests import/use the moved setup function within the same file.apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsx (2)
23-33:⚠️ Potential issue | 🟡 MinorUse
queryByTestIdinstead ofgetByTestIdin test expectations.Per the coding guidelines for
apps/deploy-web/**/*.spec.tsxfiles, usequeryBymethods instead ofgetBymethods in test expectations.queryBymethods returnnullif an element is not found, making them safer for testing both presence and absence of elements.Locations to fix
Line 32:
- expect(screen.getByTestId("alert-remove-success-notification")).toBeInTheDocument(); + expect(screen.queryByTestId("alert-remove-success-notification")).toBeInTheDocument();Line 46:
- expect(screen.getByTestId("alert-remove-error-notification")).toBeInTheDocument(); + expect(screen.queryByTestId("alert-remove-error-notification")).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsx` around lines 23 - 33, Replace the use of screen.getByTestId with screen.queryByTestId in AlertsListContainer.spec.tsx for the alerts removal test (the assertions after calling child.onRemove in the test that uses setup(), mockData, requestFn and child.onRemove), updating both occurrences mentioned; keep the same assertion matcher (e.g., expect(screen.queryByTestId("alert-remove-success-notification")).toBeInTheDocument()) so the test uses the safer queryBy* API while preserving the intent to assert presence/absence.
79-90:⚠️ Potential issue | 🟠 MajorImport
RequestFnfrom the correct package and fix query selector violations.The proposed fix uses the wrong import source. Other similar container spec files in
apps/deploy-web/src/components/alerts/importRequestFnfrom@openapi-qraft/tanstack-query-react-types, not@openapi-qraft/react. Additionally, per coding guidelines, replacegetByTestIdwithqueryByTestIdin test expectations for deploy-web specs.Corrected fix
-import type { RequestFnResponse } from "@openapi-qraft/react"; +import type { RequestFn, RequestFnResponse } from "@openapi-qraft/tanstack-query-react-types"; @@ - const requestFn = vi.fn( + const requestFn: RequestFn<any, Error> = vi.fn( () => Promise.resolve({ data: mockData }) as Promise<RequestFnResponse<typeof mockData, unknown>> ); @@ - createAPIClient({ - requestFn: requestFn as any, + createAPIClient({ + requestFn: requestFn as RequestFn<any, Error>, baseUrl: "", queryClient }) @@ - expect(screen.getByTestId("alert-remove-success-notification")).toBeInTheDocument(); + expect(screen.queryByTestId("alert-remove-success-notification")).toBeInTheDocument(); @@ - expect(screen.getByTestId("alert-remove-error-notification")).toBeInTheDocument(); + expect(screen.queryByTestId("alert-remove-error-notification")).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsx` around lines 79 - 90, Update the test to import RequestFn from the correct package and use query selector-safe assertions: change the RequestFn import to come from "@openapi-qraft/tanstack-query-react-types" (not "@openapi-qraft/react") and ensure the mocked function signature remains compatible (the existing requestFn mock can stay but type it as RequestFn). Also replace any getByTestId usages in AlertsListContainer.spec.tsx with queryByTestId in test expectations (and adjust assertions to handle null/undefined results) so the tests follow deploy-web selector guidelines; reference the requestFn mock and the services object when making these changes.apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.spec.tsx (1)
350-359:⚠️ Potential issue | 🟡 MinorReplace
anytypes in setup input with explicit types.
paymentMethods?: any[]andsetupIntent?: anyviolate the no-anyrule and prevent refactoring tools from tracking type changes. SincePaymentMethodis available from@akashnetwork/http-sdk, import it and define a type forSetupIntentbased on the structure used in your mock setup.Proposed fix
+ import type { PaymentMethod } from "@akashnetwork/http-sdk"; + + type TestSetupIntent = { id: string; client_secret: string }; + function setup( input: { - paymentMethods?: any[]; - setupIntent?: any; + paymentMethods?: PaymentMethod[]; + setupIntent?: TestSetupIntent; hasManagedWallet?: boolean; isWalletLoading?: boolean; isConnectingWallet?: boolean; isRemoving?: boolean; onComplete?: Mock;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.spec.tsx` around lines 350 - 359, Change the `setup` helper signature to remove `any` by importing `PaymentMethod` from `@akashnetwork/http-sdk` and using `paymentMethods?: PaymentMethod[]`, and define a local `SetupIntent` interface (matching the fields used in tests, e.g., id, client_secret, status or whatever properties mocks expect) then use `setupIntent?: SetupIntent` in the `setup` input type; update the import list to include `PaymentMethod` and replace all occurrences in the spec that rely on the mocked setupIntent to conform to the new `SetupIntent` shape so type-checking passes (targets: the `setup` function and the `paymentMethods` / `setupIntent` parameters).apps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.spec.tsx (1)
120-149:⚠️ Potential issue | 🟡 MinorRemove
anytypes from setup function and mockSnackbar.Define explicit types for
userand snackbar props. UseCustomUserProfilefrom@src/types/userfor the user type, or define a minimal test-specific type with the required fields.Suggested approach
+import type { CustomUserProfile } from "@src/types/user"; + function setup(input: { user?: CustomUserProfile; onComplete?: Mock } = {}) { const mockSendVerificationEmail = vi.fn(); const mockCheckSession = vi.fn(); const mockEnqueueSnackbar = vi.fn(); const mockAnalyticsService = { track: vi.fn() }; const mockUseCustomUser = vi.fn().mockReturnValue({ - user: input.user || { id: "test-user", emailVerified: false }, + user: input.user || { id: "test-user", emailVerified: false } as CustomUserProfile, checkSession: mockCheckSession }); const mockUseSnackbar = vi.fn().mockReturnValue({ enqueueSnackbar: mockEnqueueSnackbar }); const mockUseServices = vi.fn().mockReturnValue({ analyticsService: mockAnalyticsService, auth: { sendVerificationEmail: mockSendVerificationEmail } }); - const mockSnackbar = ({ title, subTitle, iconVariant }: any) => ( + const mockSnackbar = ({ title, subTitle, iconVariant }: { title: string; subTitle: string; iconVariant: string }) => ( <div data-testid="snackbar" data-title={title} data-subtitle={subTitle} data-icon-variant={iconVariant} /> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.spec.tsx` around lines 120 - 149, Remove the use of `any` in the `setup` function and `mockSnackbar` by giving explicit types: import and use `CustomUserProfile` from `@src/types/user` (or declare a minimal test-specific type with at least `id: string` and `emailVerified: boolean`) for the `user` parameter and mock return of `mockUseCustomUser`, and type the `mockSnackbar` props (e.g., `{ title: string; subTitle?: string; iconVariant?: string }`); update `setup` signature, `mockUseCustomUser.mockReturnValue`, and `mockSnackbar` parameter annotation accordingly so all mocks (`mockUseCustomUser`, `mockUseServices`, `mockUseSnackbar`) return properly typed values.
apps/deploy-web/src/components/billing-usage/PaymentMethodsView/PaymentMethodsView.spec.tsx
Show resolved
Hide resolved
apps/deploy-web/src/components/billing-usage/PaymentPopup/PaymentPopup.spec.tsx
Show resolved
Hide resolved
apps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.spec.tsx
Show resolved
Hide resolved
apps/deploy-web/src/hoc/registered-users-only/registered-users-only.hoc.spec.tsx
Show resolved
Hide resolved
apps/deploy-web/src/hooks/useThrottledEffect/useThrottledEffect.spec.tsx
Show resolved
Hide resolved
apps/deploy-web/src/utils/getValidInternalReturnToUrl/getValidInternalReturnToUrl.spec.ts
Show resolved
Hide resolved
1ccb8e7 to
0742988
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (17)
apps/deploy-web/src/services/feature-flag/feature-flag.service.spec.ts (1)
69-87:⚠️ Potential issue | 🟡 MinorMake
setupthe last helper and remove its explicit return type.The project rule requires
setupto be the last function in the rootdescribeand to omit an explicit return type. Please move it belowcreateCtxand let TS infer the 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 change (remove explicit return type)
- function setup(options?: { enableAll?: boolean; isEnabled?: Mock }): { - service: FeatureFlagService; - unleash: typeof unleashModule; - flagsClient: MockProxy<UnleashClient>; - } { + function setup(options?: { enableAll?: boolean; isEnabled?: Mock }) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/services/feature-flag/feature-flag.service.spec.ts` around lines 69 - 87, Move the setup helper so it appears after createCtx as the last helper in the root describe, and remove its explicit return type so TypeScript can infer it; keep the function signature accepting a single inline-typed parameter (options?: { enableAll?: boolean; isEnabled?: Mock }) and preserve its internals (mocking unleash module, flagsClient, config, instantiating FeatureFlagService, and spying on service.extractSessionId), ensuring no shared state and that setup returns the created object implicitly.apps/deploy-web/src/components/providers/NetworkCapacity/NetworkCapacity.spec.tsx (1)
217-265:⚠️ Potential issue | 🟡 MinorAlign
setupwith the test setup guideline (inline param type + bottom of rootdescribe).Line 217 uses a named
TestInputtype and Line 262+ places additional declarations aftersetup, sosetupis not the last item in the rootdescribe. The guideline requires a single inline-typed parameter andsetupto be at the bottom of the rootdescribeblock.Proposed fix
- function setup(input?: TestInput) { + function setup(input?: { stats?: Props["stats"]; theme?: "light" | "dark" }) { type PieData = { id: string; label: string; value: number; color: string }; type PieProps = { data: PieData[]; tooltip: unknown; innerRadius: number; padAngle: number; cornerRadius: number; activeOuterRadiusOffset: number; borderWidth: number; enableArcLinkLabels: boolean; arcLabelsSkipAngle: number; }; const mockPieProps: PieProps[] = []; const mockTooltipLabel = vi.fn(); const MockResponsivePie = vi.fn((props: PieProps) => { mockPieProps.push(props); return null; }); const dependencies: typeof DEPENDENCIES = { ResponsivePie: MockResponsivePie as unknown as typeof DEPENDENCIES.ResponsivePie, TooltipLabel: mockTooltipLabel as unknown as typeof DEPENDENCIES.TooltipLabel }; const stats = input?.stats ?? createStats(); const theme = input?.theme ?? "light"; const result = render( <ThemeProvider defaultTheme={theme} attribute="class"> <IntlProvider locale="en-US" defaultLocale="en-US"> <NetworkCapacity stats={stats} dependencies={dependencies} /> </IntlProvider> </ThemeProvider> ); return { ...result, pieProps: mockPieProps, mockTooltipLabel, MockResponsivePie }; } - interface TestInput { - stats?: Props["stats"]; - theme?: "light" | "dark"; - } });As per coding guidelines: “Use
setupfunction 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.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/providers/NetworkCapacity/NetworkCapacity.spec.tsx` around lines 217 - 265, Move the setup function to the bottom of the root describe block and convert its parameter to a single inline-typed parameter instead of the named TestInput interface; remove the interface declaration and any extra declarations that follow setup so setup is the last item in the describe. Ensure setup has no explicit return type, does not rely on shared state, and still constructs/mock values (MockResponsivePie, mockTooltipLabel, mockPieProps, dependencies, stats, theme) inside itself and returns the render result plus pieProps/mockTooltipLabel/MockResponsivePie as before.apps/deploy-web/src/components/turnstile/Turnstile.spec.tsx (2)
162-193:⚠️ Potential issue | 🟡 MinorKeep
setupas the last helper in the rootdescribeblock.
setupcurrently appears beforeuseForwardedRef, but test guidelines require it to be the last helper in the root suite.
As per coding guidelines:**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock.🔧 Suggested reordering
- async function setup(input?: { enabled?: boolean; siteKey?: string; onDismissed?: () => void; components?: Partial<typeof COMPONENTS> }) { - const turnstileRef = { current: null as TurnstileRef | null }; - ... - return { ...result, turnstileRef }; - } - function useForwardedRef<T>(ref: React.ForwardedRef<T>, instance: T = mock<T>()) { useEffect(() => { if (typeof ref === "function") { ref(instance); } else if (ref) { ref.current = instance; } }, []); } + + async function setup(input?: { enabled?: boolean; siteKey?: string; onDismissed?: () => void; components?: Partial<typeof COMPONENTS> }) { + const turnstileRef = { current: null as TurnstileRef | null }; + ... + return { ...result, turnstileRef }; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/turnstile/Turnstile.spec.tsx` around lines 162 - 193, Move the async helper function setup so it appears after useForwardedRef and is the last helper declared inside the root describe block; specifically relocate the setup(...) definition to follow the useForwardedRef(...) definition so the test file follows the guideline that the setup helper is the final helper in the root suite. Ensure references to Turnstile, TurnstileRef, and useForwardedRef remain intact when you move the function.
55-59:⚠️ Potential issue | 🟠 MajorAdd explicit button prop types to avoid implicit
anyinforwardRefcallbacks.Both Button mocks (lines 55-59 and 82-86) infer
propsandrefasany, which violates TypeScript typing rules and fails under strict mode (noImplicitAnyis enabled in tsconfig).Suggested fix
- Button: forwardRef((props, ref) => ( + Button: forwardRef<HTMLButtonElement, React.ComponentPropsWithoutRef<"button">>((props, ref) => ( <button type="button" {...props} ref={ref} onClick={props.onClick}> {props.children} </button> ))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/turnstile/Turnstile.spec.tsx` around lines 55 - 59, The mocked Button components in Turnstile.spec.tsx use forwardRef((props, ref) => ...) with implicit any for props and ref; update both mocks (the Button forwardRef callbacks at lines ~55-59 and ~82-86) to declare explicit types: type the props as React.ButtonHTMLAttributes<HTMLButtonElement> (or a narrow interface if needed) and children as React.ReactNode, and type the ref as React.Ref<HTMLButtonElement> (or React.ForwardedRef<HTMLButtonElement>) so the forwardRef signature is fully typed and satisfies noImplicitAny.apps/deploy-web/src/utils/domUtils.spec.ts (1)
33-66:⚠️ Potential issue | 🟡 MinorAlign
setup()with repository test guidelines.
Line 33-66:setupis nested inside a childdescribeand takes no parameter; guidelines require it at the bottom of the rootdescribeand to accept a single inline-typed parameter. Please refactor accordingly.As per coding guidelines: Use
setupfunction 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.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/utils/domUtils.spec.ts` around lines 33 - 66, Move the nested setup function to the bottom of the root describe and refactor its signature to accept a single inline-typed parameter (e.g., opts: { /* optional flags */ }) instead of no args; keep no explicit return type and have setup create and return the test doubles and SUT (createObjectURLMock, revokeObjectURLMock, setAttributeMock, appendChildMock, removeChildMock, createElementMock, linkMock) without sharing state across tests, and preserve the document.createElement spy/restore behavior inside that function so callers receive the initialized mocks and spies from a single returned object.apps/deploy-web/src/hooks/usePricing/usePricing.spec.ts (2)
57-89:⚠️ Potential issue | 🟡 MinorPlace
setupat the bottom of the rootdescribe.
setupis followed bybuildMarketData, which breaks the required ordering. MovebuildMarketDataabovesetupor movesetupafter it.♻️ Suggested reordering
- function setup(input?: { marketData?: MarketData | null; isLoadingMarketData?: boolean }) { - const result = setupQuery(() => usePricing(), { - services: { - publicConsoleApiHttpClient: () => - mock<HttpClient>({ - get: async () => { - if (input?.isLoadingMarketData) return new Promise(() => {}); - return { - data: input?.marketData === undefined ? buildMarketData() : input.marketData - }; - } - } as unknown as HttpClient), - networkStore: () => - mock<NetworkStore>({ - useSelectedNetworkId: () => SANDBOX_ID - }) - } - }); - - return result; - } - function buildMarketData(overrides?: Partial<MarketData>): MarketData { return { price: 1.25, volume: 1_000_000, marketCap: 500_000_000, marketCapRank: 100, priceChange24h: 0.01, priceChangePercentage24: 0.5, ...overrides }; } + + function setup(input?: { marketData?: MarketData | null; isLoadingMarketData?: boolean }) { + const result = setupQuery(() => usePricing(), { + services: { + publicConsoleApiHttpClient: () => + mock<HttpClient>({ + get: async () => { + if (input?.isLoadingMarketData) return new Promise(() => {}); + return { + data: input?.marketData === undefined ? buildMarketData() : input.marketData + }; + } + } as unknown as HttpClient), + networkStore: () => + mock<NetworkStore>({ + useSelectedNetworkId: () => SANDBOX_ID + }) + } + }); + + return result; + }As per coding guidelines, "Use
setupfunction 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."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/usePricing/usePricing.spec.ts` around lines 57 - 89, The setup helper is placed before buildMarketData which violates the rule that setup must be at the bottom of the root describe; relocate functions so that either buildMarketData is defined above setup or move setup to after buildMarketData (i.e., place setup as the last helper inside the root describe). Ensure the setup function signature remains a single parameter with inline type, returns the result of setupQuery, and does not introduce shared state.
4-24:⚠️ Potential issue | 🟠 MajorReplace
vi.waitForwithwaitForfrom Testing Library.
vi.waitFordoes not exist in Vitest 4.0.18. Use thewaitForutility from@testing-library/reactinstead. Also move thesetupfunction to the bottom of the describe block after helper functions, and update the unused import on line 12.Required changes
-import {} from "@testing-library/react"; +import { waitFor } from "@testing-library/react"; ... -import { vi } from "vitest"; +import { describe, expect, it } from "vitest";- await vi.waitFor(() => { + await waitFor(() => { expect(result.current.isLoaded).toBe(true); });Move
setupfunction afterbuildMarketDataso it is at the bottom of the describe block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/usePricing/usePricing.spec.ts` around lines 4 - 24, Replace the nonexistent vi.waitFor call with waitFor imported from `@testing-library/react` (update the import to include waitFor), and move the setup function to the bottom of the describe block after buildMarketData per the review; update the unused empty import line to import { waitFor } from "@testing-library/react" and ensure tests call waitFor(...) instead of vi.waitFor(...), keeping the helper functions buildMarketData and setup present and relocating setup below buildMarketData.apps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.spec.tsx (1)
120-161:⚠️ Potential issue | 🟠 MajorAddress both
anytypes in the setup function.The setup function has two violations of the TypeScript guideline:
- Line 120:
user?: anyin the input parameter- Line 144:
({ title, subTitle, iconVariant }: any)in the mockSnackbar destructuringDefine explicit types for both instead of using
any.✅ Suggested fixes
- function setup(input: { user?: any; onComplete?: Mock } = {}) { + function setup( + input: { + user?: { id?: string; emailVerified?: boolean }; + onComplete?: Mock; + } = {} + ) {- const mockSnackbar = ({ title, subTitle, iconVariant }: any) => ( + const mockSnackbar = ({ title, subTitle, iconVariant }: { title?: string; subTitle?: string; iconVariant?: string }) => (Per coding guidelines
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.spec.tsx` around lines 120 - 161, The setup function currently uses two `any` types: the input parameter's `user?: any` and the `mockSnackbar` props `({ title, subTitle, iconVariant }: any)`; replace these with explicit types (e.g., declare or import a User type/interface and a SnackbarProps/interface) and update references: change the setup signature to `input: { user?: User; onComplete?: Mock }`, ensure `mockUseCustomUser`'s mockReturnValue matches the User shape, and change `mockSnackbar` to accept `(props: SnackbarProps)` or destructure with `({ title, subTitle, iconVariant }: SnackbarProps)` so TypeScript enforces proper typings; add the minimal type definitions or imports at the top of the test file if they don't already exist.apps/deploy-web/src/components/alerts/NotificationChannelsListView/NotificationChannelsListView.spec.tsx (1)
99-114:⚠️ Potential issue | 🟡 MinorMake the
setupparameter inline-typed as required by test rules.
setupcurrently acceptsPartial<NotificationChannelsListViewProps>, which isn’t an inline type definition. Please switch to an inline object type.As per coding guidelines, `**/*.spec.{ts,tsx} : Use setup function instead of beforeEach in test files. The setup function must be at the bottom of the root describe block, 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 change
-function setup(props: Partial<NotificationChannelsListViewProps> = {}) { +function setup( + props: { + pagination?: NotificationChannelsListViewProps["pagination"]; + data?: NotificationChannelsListViewProps["data"]; + isLoading?: NotificationChannelsListViewProps["isLoading"]; + onRemove?: NotificationChannelsListViewProps["onRemove"]; + removingIds?: NotificationChannelsListViewProps["removingIds"]; + onPaginationChange?: NotificationChannelsListViewProps["onPaginationChange"]; + isError?: NotificationChannelsListViewProps["isError"]; + } = {} +) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelsListView/NotificationChannelsListView.spec.tsx` around lines 99 - 114, The setup function's parameter uses a named type Partial<NotificationChannelsListViewProps>; change it to an inline object type instead so tests follow the rule requiring inline typing. Update the signature of setup (the helper that builds defaultProps and returns the component under test) to accept a single parameter typed inline (e.g. { pagination?: ..., data?: ..., isLoading?: ..., onRemove?: ..., removingIds?: ..., onPaginationChange?: ..., isError?: ... }) and merge that inline-typed param into defaultProps exactly as before; keep the function name setup, retain defaultProps construction and spreading behavior, and do not add an explicit return type.apps/deploy-web/src/components/alerts/NotificationChannelEditContainer/NotificationChannelEditContainer.spec.tsx (2)
82-115:⚠️ Potential issue | 🟡 Minor
setupmust accept a single inline‑typed parameter.
Line 82 definessetup()without parameters, which violates the test guideline. Add a single inline‑typed parameter (with defaults if needed).Proposed fix
- async function setup() { + async function setup( + { inputOverrides = {} }: { inputOverrides?: Partial<{ id: string; name: string; emails: string[] }> } = {} + ) { const input = { id: faker.string.uuid(), name: faker.lorem.word(), - emails: [faker.internet.email()] + emails: [faker.internet.email()], + ...inputOverrides };As per coding guidelines:
**/*.spec.{ts,tsx}: 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.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelEditContainer/NotificationChannelEditContainer.spec.tsx` around lines 82 - 115, The setup function must accept a single inline-typed parameter; change the signature of setup to accept one options parameter (e.g. setup = async (opts: { id?: string; name?: string; emails?: string[] } = {}) => { ... }) and use opts.id/opts.name/opts.emails to build the input (falling back to the existing faker defaults), leaving the rest of the function (requestFn, services, childCapturer, render of NotificationChannelEditContainer and returned object including requestFn, input, child) unchanged so tests keep their behavior.
23-46:⚠️ Potential issue | 🟡 MinorUse
queryByTestIdin expectations.
Line 45 and Line 78 usegetByTestIdinside expectations; this should bequeryByTestIdper the test guideline.Proposed fix
- expect(screen.getByTestId("notification-channel-edit-success-notification")).toBeInTheDocument(); + expect(screen.queryByTestId("notification-channel-edit-success-notification")).toBeInTheDocument(); @@ - expect(screen.getByTestId("notification-channel-edit-error-notification")).toBeInTheDocument(); + expect(screen.queryByTestId("notification-channel-edit-error-notification")).toBeInTheDocument();As per coding guidelines:
{apps/deploy-web,apps/provider-console}/**/*.spec.tsx: UsequeryBymethods instead ofgetBymethods in test expectations.Also applies to: 56-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelEditContainer/NotificationChannelEditContainer.spec.tsx` around lines 23 - 46, In NotificationChannelEditContainer.spec.tsx, update assertions inside vi.waitFor that currently call screen.getByTestId to use screen.queryByTestId instead (e.g., replace screen.getByTestId("notification-channel-edit-success-notification") with screen.queryByTestId(...)) so the expectation uses the queryBy pattern per test guidelines; keep the rest of the assertion (expect(...).toBeInTheDocument()) intact and make the same change for other occurrences in the file (the block asserting requestFn calls and the later assertions around lines 56-79).apps/deploy-web/src/context/PaymentPollingProvider/PaymentPollingProvider.spec.tsx (1)
209-293:⚠️ Potential issue | 🟡 MinorAdd
afterEachhook to restore timers and prevent test isolation issues.
setup()enables fake timers (line 210), but 5 of the 9 tests don't callcleanup(): "provides polling context to children", "prevents multiple polling instances", "shows loading snackbar when polling starts", "stops polling when stopPolling is called", and "verifies analytics service is properly configured for trial users". Fake timers will remain enabled for subsequent tests, causing test isolation issues. Add anafterEachhook to guarantee cleanup.Proposed fix
-import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; ... describe(PaymentPollingProvider.name, () => { + afterEach(() => { + vi.useRealTimers(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/context/PaymentPollingProvider/PaymentPollingProvider.spec.tsx` around lines 209 - 293, The tests call vi.useFakeTimers() inside setup() but don't always restore them; add an afterEach hook in PaymentPollingProvider.spec.tsx that calls vi.useRealTimers() (or invokes the setup cleanup) to guarantee timers are restored between tests so fake timers from setup() don't leak into other tests; reference the setup function and vi.useFakeTimers()/vi.useRealTimers() when adding this hook.apps/deploy-web/src/components/billing-usage/PaymentPopup/PaymentPopup.spec.tsx (2)
770-789:⚠️ Potential issue | 🟡 MinorMove
setupinside the rootdescribeblock (last statement).
setupis currently declared outside the root suite; the guideline requires it to be placed at the bottom of the rootdescribeblock.
As per coding guidelines:**/*.spec.{ts,tsx}: 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.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/billing-usage/PaymentPopup/PaymentPopup.spec.tsx` around lines 770 - 789, The setup function is declared outside the root describe; move the entire setup(...) declaration to be the last statement inside the root describe block so it lives within that suite (keep the same inline parameter type and signature, do not add an explicit return type), ensure it constructs and returns the test object locally (no shared module-level state) and update any references in the file to still call setup() as before.
582-587:⚠️ Potential issue | 🟡 MinorUse
queryBy*for expectations in deploy-web specs.These assertions still use
getBy*. The deploy-web spec guideline requiresqueryBy*in expectations; please swap them in thesewaitForblocks (and any similar ones in this file).♻️ Suggested change (apply similarly to the other blocks)
- expect(screen.getByTestId("alert")).toBeInTheDocument(); - expect(screen.getByText("Payment Error")).toBeInTheDocument(); - expect(screen.getByText("Your card was declined.")).toBeInTheDocument(); - expect(screen.getByText("Please try a different card.")).toBeInTheDocument(); + expect(screen.queryByTestId("alert")).toBeInTheDocument(); + expect(screen.queryByText("Payment Error")).toBeInTheDocument(); + expect(screen.queryByText("Your card was declined.")).toBeInTheDocument(); + expect(screen.queryByText("Please try a different card.")).toBeInTheDocument();As per coding guidelines:
{apps/deploy-web,apps/provider-console}/**/*.spec.tsx: UsequeryBymethods instead ofgetBymethods in test expectations.Also applies to: 614-617, 652-654, 690-692
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/billing-usage/PaymentPopup/PaymentPopup.spec.tsx` around lines 582 - 587, The assertions inside the vi.waitFor blocks in PaymentPopup.spec.tsx use screen.getByTestId/getByText which will throw if the element is absent; replace those calls with screen.queryByTestId and screen.queryByText in the waitFor at lines shown (the block that checks "alert", "Payment Error", "Your card was declined.", "Please try a different card.") and apply the same change to the other similar waitFor blocks mentioned (around the blocks at 614-617, 652-654, 690-692) so expectations use queryBy* methods instead of getBy*.apps/deploy-web/src/hooks/useUser.spec.tsx (1)
35-44:⚠️ Potential issue | 🟠 MajorRemove the unused
mock()call—it has no effect and doesn't actually mock theuseCustomUserhook.Lines 36-41 create a mock object but never assign or use the result. This is dead code. The
mock()function fromvitest-mock-extendedcreates mock objects; it doesn't intercept module imports. The tests currently work becauseUserProvider(line 49) provides the test data via props, not because of this ineffective mock.Either remove the mock code entirely, or properly mock
useCustomUserusing dependency injection or module-level mocking if needed. The current code violates the pattern of usingjest-mock-extendedto create mocks and pass them as dependencies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/useUser.spec.tsx` around lines 35 - 44, The setup function is creating a mock object via mock() for useCustomUser but never uses or assigns it, so remove the ineffective mock() call (or replace it with a proper module-level mock) to eliminate dead code; update the setup helper (function setup) to stop calling mock() and rely on the UserProvider props (or, if you intend to mock the hook, use vi.mock or proper dependency injection to mock useCustomUser from useCustomUserModule) and ensure buildUser, useCustomUserModule.useCustomUser, and checkSession references remain consistent.apps/deploy-web/src/components/auth/SignUpButton/SignUpButton.spec.tsx (1)
59-72:⚠️ Potential issue | 🟡 MinorReturn an object from
setupto follow test guidelines.
setup()renders but returns void; the guideline expects it to create and return the object under test. Returning the render result (and router) keeps compliance without affecting callers.As per coding guidelines: `**/*.spec.{ts,tsx}`: Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block, 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 adjustment
- render( + const renderResult = render( <TestContainerProvider> <SignUpButton {...props} dependencies={{ ...DEPENDENCIES, useRouter: () => router }} /> </TestContainerProvider> ); + return { router, renderResult };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/auth/SignUpButton/SignUpButton.spec.tsx` around lines 59 - 72, The setup() helper currently renders SignUpButton but returns void; update setup (the function named setup in SignUpButton.spec.tsx) to return the rendered result and the mocked router so tests follow the spec guideline: capture the value returned by render(...) and the router variable, and return an object like { ...renderResult, router } (no explicit return type), preserving the existing call to <TestContainerProvider> and dependencies (DEPENDENCIES, useRouter: () => router) so callers can access query utilities and the mocked NextRouter.push.apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsx (1)
79-90:⚠️ Potential issue | 🟠 MajorImport
RequestFnand useRequestFn<any, Error>instead of the rawas anycast.The
as anycast violates the no-anyrule. Update the import to includeRequestFnfrom@openapi-qraft/tanstack-query-react-types(matching the pattern inDeploymentAlertsContainer,NotificationChannelCreateContainer, andNotificationChannelsListContainertest files) and cast toRequestFn<any, Error>instead:-import type { RequestFnResponse } from "@openapi-qraft/react"; +import type { RequestFn, RequestFnResponse } from "@openapi-qraft/tanstack-query-react-types";Then update line 89:
- requestFn: requestFn as any, + requestFn: requestFn as RequestFn<any, Error>,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsx` around lines 79 - 90, Import RequestFn from "@openapi-qraft/tanstack-query-react-types" at the top of the spec and change the test helper `requestFn` typing so it uses RequestFn<any, Error> instead of casting to `any`; specifically update the `requestFn` declaration and the cast passed into `createAPIClient` (replace `as any` with `as RequestFn<any, Error>`), mirroring the pattern used in DeploymentAlertsContainer, NotificationChannelCreateContainer, and NotificationChannelsListContainer tests.
🧹 Nitpick comments (13)
apps/deploy-web/src/lib/websocket/WebsocketSession.spec.ts (1)
8-10: Consider removing unnecessaryafterEachblock.The
vi.useRealTimers()call resets fake timers, but no tests in this file usevi.useFakeTimers(). This cleanup appears to be unnecessary and could be removed to reduce noise.🧹 Proposed fix
describe(WebsocketSession.name, () => { - afterEach(() => { - vi.useRealTimers(); - }); - describe("send", () => {Also update the import on line 1:
-import { afterEach, describe, expect, it, vi } from "vitest"; +import { describe, expect, it, vi } from "vitest";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/lib/websocket/WebsocketSession.spec.ts` around lines 8 - 10, Remove the unnecessary afterEach cleanup in WebsocketSession.spec.ts: delete the afterEach(() => { vi.useRealTimers(); }) block since no tests use vi.useFakeTimers(), and also remove the now-unused vi import at the top of the file (the import on line 1) if it's only present for that call.apps/deploy-web/src/queries/useGrantsQuery.spec.tsx (2)
11-11: Remove empty import (migration leftover).This empty import serves no purpose and appears to be a leftover from the Jest migration.
🧹 Proposed fix
-import {} from "@testing-library/react";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useGrantsQuery.spec.tsx` at line 11, Remove the empty import statement `import {} from "@testing-library/react";` from useGrantsQuery.spec.tsx — it's a migration leftover and unused; simply delete that import line (leaving any other valid imports intact) and run tests to ensure nothing else depends on it.
174-182: Avoidas anycasts.The
as anycasts on lines 182 and 209 violate the coding guideline that prohibits using typeany. Consider using a more specific type or adjusting the mock setup to avoid the type escape hatch.As per coding guidelines: "Never use type
anyor cast to typeany. Always define the proper TypeScript types."Also applies to: 201-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useGrantsQuery.spec.tsx` around lines 174 - 182, Summary: Tests use "as any" to silence typing for the chainApiHttpClient mocks; replace those casts with properly typed mock objects. Fix: import the vitest Mocked type (or use Partial) and declare the mock as vi.Mocked<FallbackableHttpClient> (or Partial<FallbackableHttpClient> with a concrete assertion) instead of using "as any"; e.g. define const chainApiHttpClient: vi.Mocked<FallbackableHttpClient> = { isFallbackEnabled: false, get: vi.fn().mockResolvedValue({ data: { allowances: mockData, pagination: { next_key: null, total: mockData.length } } }) }; do the same for the other mock at the later occurrence so both mocks satisfy the FallbackableHttpClient shape without using any.apps/deploy-web/src/queries/useManagedWalletQuery.spec.tsx (2)
12-13: Redundant nested describe with duplicate name.The inner describe at line 13 duplicates the outer describe name. As per coding guidelines, nested suite descriptions should use a method name or a condition starting with 'when'. Consider removing the redundant nesting or renaming the inner describe to describe a specific scenario.
♻️ Proposed fix to remove redundant nesting
describe(useManagedWalletQuery.name, () => { - describe(useManagedWalletQuery.name, () => { it("should fetch wallet when userId is provided", async () => { // ... }); it("should not fetch when userId is not provided", () => { // ... }); - }); describe(useCreateManagedWalletMutation.name, () => { // ... }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useManagedWalletQuery.spec.tsx` around lines 12 - 13, The test file contains a redundant nested describe using the same title as the outer suite: remove the inner describe(useManagedWalletQuery.name, ...) or rename it to a scenario-style description (e.g., describe('when ...') or describe('with X condition')), so keep the outer describe(useManagedWalletQuery.name, ...) and either delete the duplicate inner describe block or change its title to a specific scenario; ensure any nested tests are moved under the corrected/named describe and that references to useManagedWalletQuery.name remain only where appropriate.
14-14: Test descriptions use "should" prefix.Per coding guidelines, test descriptions should use present simple, 3rd person singular without prepending "should". This applies to lines 14, 34, and 48.
♻️ Proposed fix for test descriptions
- it("should fetch wallet when userId is provided", async () => { + it("fetches wallet when userId is provided", async () => {- it("should not fetch when userId is not provided", () => { + it("does not fetch when userId is not provided", () => {- it("should create wallet and update query cache", async () => { + it("creates wallet and updates query cache", async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useManagedWalletQuery.spec.tsx` at line 14, Update the test descriptions in useManagedWalletQuery.spec.tsx to remove the "should" prefix and use present simple, 3rd person singular; e.g., change the it("should fetch wallet when userId is provided", ...) description to it("fetches wallet when userId is provided", ...), and apply the same transformation to the other two it(...) descriptions (lines showing "should ..." in the file) so each reads like "fetches ...", "returns ...", etc., in present simple 3rd-person form.apps/deploy-web/src/components/new-deployment/NewDeploymentContainer/NewDeploymentContainer.spec.tsx (2)
27-29: Consider usingwaitForfrom@testing-library/reactfor DOM assertions.
vi.waitForis a valid Vitest utility, but for DOM testing with React,waitForfrom@testing-library/reactintegrates better with React'sact()wrapper and has default configurations optimized for DOM testing. This applies to allvi.waitForusages throughout the file.♻️ Alternative approach using testing-library's waitFor
-import { render, screen } from "@testing-library/react"; +import { render, screen, waitFor } from "@testing-library/react";Then replace
vi.waitForcalls:- await vi.waitFor(() => { + await waitFor(() => { expect(screen.getByTestId("template-list")).toBeInTheDocument(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/new-deployment/NewDeploymentContainer/NewDeploymentContainer.spec.tsx` around lines 27 - 29, Replace uses of vi.waitFor in NewDeploymentContainer.spec.tsx with waitFor imported from `@testing-library/react`: update the import list to include waitFor and change calls like vi.waitFor(() => expect(screen.getByTestId("template-list")).toBeInTheDocument()) to waitFor(...). Ensure other vi.waitFor occurrences in this spec file are similarly replaced so DOM assertions use testing-library's waitFor which integrates with React's act().
336-340: Minor:as never[]type assertions are unusual.While functional,
as never[]is an unconventional pattern. Consider using explicit empty array types for clarity.♻️ Suggested type-safe alternative
const templatesValue = { isLoading: input.isLoadingTemplates ?? false, - templates: [] as never[], - categories: [] as never[] + templates: [] as TemplateOutput[], + categories: [] as string[] };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/new-deployment/NewDeploymentContainer/NewDeploymentContainer.spec.tsx` around lines 336 - 340, The test uses unusual "as never[]" assertions for templatesValue.templates and templatesValue.categories; replace those with explicit empty-array types instead: update the templatesValue object (the templates and categories properties) to be typed as the actual expected array types (e.g., Template[] and Category[] or Array<YourType>) or use unknown[]/any[] if concrete types aren't available, so the test is type-safe and clear rather than asserting never[].apps/deploy-web/src/queries/useSaveSettings.spec.tsx (3)
19-19: Test description grammar: use 3rd person singular for all verbs.The description contains "call" and "show" which should be "calls" and "shows" to follow 3rd person singular convention.
- it("saves settings successfully, call checkSession and show success snackbar", async () => { + it("saves settings successfully, calls checkSession and shows success snackbar", async () => {As per coding guidelines: "Use present simple, 3rd person singular for test descriptions without prepending 'should'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useSaveSettings.spec.tsx` at line 19, Update the test description string in the it(...) block for the useSaveSettings spec to use present simple 3rd person singular: change "saves settings successfully, call checkSession and show success snackbar" to "saves settings successfully, calls checkSession and shows success snackbar" so verbs ("call" and "show") match the project's test description grammar convention; locate the it(...) in useSaveSettings.spec.tsx and edit the description accordingly.
82-82: Remove "should" prefix from test description.The coding guideline specifies using present simple, 3rd person singular for test descriptions without prepending "should".
- it("should fetch deposit params successfully", async () => { + it("fetches deposit params successfully", async () => {As per coding guidelines: "Use present simple, 3rd person singular for test descriptions without prepending 'should'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useSaveSettings.spec.tsx` at line 82, Replace the test description string in the it(...) call that currently reads "should fetch deposit params successfully" with a present simple, 3rd person singular phrasing (e.g., "fetches deposit params successfully") so the test in useSaveSettings.spec.tsx follows the guideline; locate the it("should fetch deposit params successfully", async () => { ... }) and update the description text accordingly.
44-44: Test description grammar: "show" should be "shows".- it("handles error when saving settings and show error snackbar", async () => { + it("handles error when saving settings and shows error snackbar", async () => {As per coding guidelines: "Use present simple, 3rd person singular for test descriptions without prepending 'should'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useSaveSettings.spec.tsx` at line 44, The test description in the spec it("handles error when saving settings and show error snackbar", ...) uses incorrect grammar; update the string to present simple 3rd person singular by changing "show" to "shows" (i.e., it("handles error when saving settings and shows error snackbar", ...)) so the test name for the useSaveSettings.spec.tsx case follows the project's test-description guideline.apps/deploy-web/src/queries/queryKeys.spec.ts (1)
1-5: Vitest migration is correct; consider usingQueryKeys.namein describe.The Vitest import is appropriate for the migration. However, per coding guidelines, the root describe block should use
QueryKeys.nameinstead of a hardcoded string to enable automated refactoring tools to find all references.♻️ Suggested fix
-describe("QueryKeys", () => { +describe(QueryKeys.name, () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/queryKeys.spec.ts` around lines 1 - 5, Replace the hardcoded describe title with a dynamic reference by using QueryKeys.name: locate the root describe block in the test file (the one that currently reads describe("QueryKeys", ...)) and change it to use QueryKeys.name so the block references the actual QueryKeys symbol (ensuring the import of QueryKeys from "./queryKeys" remains correct).apps/deploy-web/src/hooks/useUser.spec.tsx (1)
12-12: UseuseIsRegisteredUser.namein describe block.Per coding guidelines, the root describe should use
useIsRegisteredUser.nameinstead of a hardcoded string.♻️ Suggested fix
-describe("useIsRegisteredUser", () => { +describe(useIsRegisteredUser.name, () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/useUser.spec.tsx` at line 12, Update the test suite's root describe to use the function's name property instead of a string literal: replace the hardcoded "useIsRegisteredUser" describe argument with useIsRegisteredUser.name so the describe references the actual hook identifier; locate the describe in useUser.spec.tsx and change its first argument to useIsRegisteredUser.name to follow the project's naming convention.apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsx (1)
69-70: Match the requiredsetupsignature shape.The test guideline expects a single parameter with an inline type definition. Add a (possibly unused) parameter to comply.
🔧 Proposed fix
- async function setup() { + async function setup(_input: {} = {}) {As per coding guidelines:
**/*.spec.{ts,tsx}: Thesetupfunction must accept a single parameter with inline type definition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsx` around lines 69 - 70, The test's setup function must match the required spec signature shape: update the async function setup() declaration to accept a single parameter with an inline type definition (e.g. async function setup(_: { /* options? */ } = {})) so it conforms to the expected `setup` signature; keep the parameter possibly unused (underscore name or default value) and preserve the existing body of setup.
...components/alerts/NotificationChannelEditContainer/NotificationChannelEditContainer.spec.tsx
Show resolved
Hide resolved
apps/deploy-web/src/components/alerts/NotificationChannelForm/NotificationChannelForm.spec.tsx
Show resolved
Hide resolved
apps/deploy-web/src/components/alerts/NotificationChannelForm/NotificationChannelForm.spec.tsx
Show resolved
Hide resolved
apps/deploy-web/src/components/auth/ForgotPasswordForm/ForgotPasswordForm.spec.tsx
Show resolved
Hide resolved
apps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.spec.tsx
Show resolved
Hide resolved
0742988 to
ed27d98
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (16)
apps/deploy-web/src/queries/usePaymentQueries.spec.tsx (1)
49-49:⚠️ Potential issue | 🟡 MinorUse
<Subject>.namefor the root describe.Line 49 hardcodes the suite name; the guideline requires using
<Subject>.name. Consider splitting into top-level describes per hook (e.g.,describe(usePaymentMethodsQuery.name, ...),describe(usePaymentMutations.name, ...)) so each root suite derives its name from the subject.
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".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/usePaymentQueries.spec.tsx` at line 49, Replace the hardcoded root describe string with subject.name-based describes: split the suite into separate top-level describes using the hook function names (e.g., describe(usePaymentMethodsQuery.name, ...) and describe(usePaymentMutations.name, ...)) instead of "usePaymentQueries"; update any nested tests/setup to live under the appropriate hook-named describe blocks so each root suite is derived from the actual subject symbol.apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts (1)
99-117:⚠️ Potential issue | 🟡 MinorPlace
setupat the bottom of the rootdescribe.
Right nowsetupsits outside the rootdescribe; the guideline requires it to be inside the root block and at its bottom.♻️ Proposed fix (move setup inside root describe)
describe("pageGuards", () => { describe("isAuthenticated", () => { ... }); describe("redirectIfAccessTokenExpired", () => { ... }); + + function setup(input?: { enabledFeatures?: string[]; session?: Partial<Session> }) { + return mock<AppTypedContext>({ + getCurrentSession: vi.fn().mockImplementation(async () => { + if (!input?.session) return null; + return { + ...input.session, + accessTokenExpiresAt: input.session.accessTokenExpiresAt ? new Date(input.session.accessTokenExpiresAt).getTime() / 1000 : undefined + }; + }), + services: { + featureFlagService: mock<FeatureFlagService>({ + isEnabledForCtx: vi.fn(async featureName => !!input?.enabledFeatures?.includes(featureName)) + }), + logger: mock<LoggerService>(), + urlService: UrlService + }, + resolvedUrl: faker.internet.url() + }); + } }); - -function setup(input?: { enabledFeatures?: string[]; session?: Partial<Session> }) { - return mock<AppTypedContext>({ - ... - }); -}As per coding guidelines: Use
setupfunction 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.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts` around lines 99 - 117, The setup helper is currently declared outside the root describe; move the entire setup function (the one returning mock<AppTypedContext> and defining getCurrentSession, services.featureFlagService.isEnabledForCtx, logger and urlService, and resolvedUrl) into the root describe block and place it at the very bottom of that root describe. Ensure setup retains its single parameter signature (input?: { enabledFeatures?: string[]; session?: Partial<Session> }) with inline typing, does not declare an explicit return type, does not rely on shared state, and continues returning the mocked AppTypedContext used by tests that call setup.apps/deploy-web/src/services/stripe/stripe.service.spec.ts (1)
8-57:⚠️ Potential issue | 🟡 MinorAlign suite/test names with project conventions.
Root suite should use
StripeService.name, and test titles should be present simple without “should”.
As per coding guidelines, use<Subject>.namein the root describe suite description and use present simple, 3rd person singular for test descriptions without prepending “should”.✅ Suggested updates
-describe("StripeService", () => { +describe(StripeService.name, () => { describe("getStripe", () => { - it("should load Stripe instance when publishable key is configured", async () => { + it("loads Stripe instance when publishable key is configured", async () => { const { stripeService, mockLoadStripe, mockStripeInstance } = setup({ publishableKey: "pk_test_mock_key" }); @@ - it("should return cached instance on subsequent calls", async () => { + it("returns cached instance on subsequent calls", async () => { const { stripeService, mockLoadStripe, mockStripeInstance } = setup({ publishableKey: "pk_test_mock_key" }); @@ - it("should handle loadStripe errors gracefully", async () => { + it("handles loadStripe errors gracefully", async () => { const { stripeService, mockLoadStripe } = setup({ publishableKey: "pk_test_mock_key" }); @@ describe("clearStripeInstance", () => { - it("should clear the cached Stripe instance", async () => { + it("clears the cached Stripe instance", async () => { const { stripeService, mockLoadStripe, mockStripeInstance } = setup({ publishableKey: "pk_test_mock_key" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/services/stripe/stripe.service.spec.ts` around lines 8 - 57, Change the root test suite description to use StripeService.name (use StripeService.name in the outer describe) and rename each it() title to present simple 3rd person singular without a leading “should” (e.g., "loads Stripe instance when publishable key is configured", "returns cached instance on subsequent calls", "handles loadStripe errors gracefully", "clears the cached Stripe instance", etc.) so descriptions follow project conventions and keep inner describe blocks like "getStripe" and "clearStripeInstance" as-is.apps/deploy-web/src/components/alerts/NotificationChannelsListContainer/NotificationChannelsListContainer.spec.tsx (1)
88-88:⚠️ Potential issue | 🟡 MinorRemove
anytype by using the concretemockDatatype.The cast to
RequestFn<any, Error>violates the coding guideline. Replaceanywith the actual data type:♻️ Suggested fix
- requestFn: requestFn as RequestFn<any, Error>, + requestFn: requestFn as RequestFn<typeof mockData, unknown>,Note: The second type parameter should also be
unknownto match the actual return typeRequestFnResponse<typeof mockData, unknown>.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelsListContainer/NotificationChannelsListContainer.spec.tsx` at line 88, The cast uses a broad any; change the type assertion on requestFn from RequestFn<any, Error> to the concrete RequestFn<typeof mockData, unknown> (or RequestFnResponse<typeof mockData, unknown> if that matches the helper signature) so the first type parameter is the actual mockData shape and the second is unknown instead of Error; update the requestFn assertion and any related helper/type imports so requestFn is typed as RequestFn<typeof mockData, unknown>.apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.spec.tsx (1)
164-230:⚠️ Potential issue | 🟡 MinorMove
setupto the bottom of the rootdescribeblock.
setupis not the last helper;genFakeTokenappears after it, which violates the test guideline. Please movegenFakeTokenabovesetupsosetupstays last.🔧 Suggested reordering
- function setup(input?: { - services?: Partial<RenderAppHookOptions["services"]>; - wallet?: Partial<WalletContext>; - custodialWallet?: ReturnType<typeof useSelectedChain>; - initialToken?: string; - }) { - return setupQuery( - () => - useProviderJwt({ - dependencies: { - ...DEPENDENCIES, - useWallet: () => ({ - address: "akash1234567890", - walletName: "test-wallet", - isWalletLoaded: true, - connectManagedWallet: vi.fn(), - logout: vi.fn(), - signAndBroadcastTx: vi.fn(), - isManaged: false, - isWalletConnected: true, - isCustodial: false, - isWalletLoading: false, - isTrialing: false, - isOnboarding: false, - creditAmount: 0, - switchWalletType: vi.fn(), - hasManagedWallet: false, - managedWalletError: undefined, - ...input?.wallet - }), - useSelectedChain: () => - input?.custodialWallet ?? - mock<CustodialWallet>({ - signArbitrary: vi.fn() - }) - } - }), - { - services: { - networkStore: () => - mock<NetworkStore>({ - useSelectedNetworkId: () => "mainnet" - }), - storedWalletsService: () => - mock<StoredWalletsService>({ - getStorageWallets: () => - input?.initialToken ? [{ address: "akash1234567890", token: input.initialToken } as storedWalletsService.LocalWallet] : [] - }), - consoleApiHttpClient: () => mock(), - ...input?.services - } - } - ); - } - function genFakeToken(payload: Partial<JwtTokenPayload> = {}) { return `header.${btoa( JSON.stringify({ version: "v1", iss: "akash1234567890", exp: Date.now() + 3600, iat: Date.now(), leases: { access: "full" }, ...payload }) )}.signature`; } + + function setup(input?: { + services?: Partial<RenderAppHookOptions["services"]>; + wallet?: Partial<WalletContext>; + custodialWallet?: ReturnType<typeof useSelectedChain>; + initialToken?: string; + }) { + return setupQuery( + () => + useProviderJwt({ + dependencies: { + ...DEPENDENCIES, + useWallet: () => ({ + address: "akash1234567890", + walletName: "test-wallet", + isWalletLoaded: true, + connectManagedWallet: vi.fn(), + logout: vi.fn(), + signAndBroadcastTx: vi.fn(), + isManaged: false, + isWalletConnected: true, + isCustodial: false, + isWalletLoading: false, + isTrialing: false, + isOnboarding: false, + creditAmount: 0, + switchWalletType: vi.fn(), + hasManagedWallet: false, + managedWalletError: undefined, + ...input?.wallet + }), + useSelectedChain: () => + input?.custodialWallet ?? + mock<CustodialWallet>({ + signArbitrary: vi.fn() + }) + } + }), + { + services: { + networkStore: () => + mock<NetworkStore>({ + useSelectedNetworkId: () => "mainnet" + }), + storedWalletsService: () => + mock<StoredWalletsService>({ + getStorageWallets: () => + input?.initialToken ? [{ address: "akash1234567890", token: input.initialToken } as storedWalletsService.LocalWallet] : [] + }), + consoleApiHttpClient: () => mock(), + ...input?.services + } + } + ); + }As per coding guidelines: "Use
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/useProviderJwt/useProviderJwt.spec.tsx` around lines 164 - 230, Move the helper genFakeToken so it appears before the setup function within the root describe block; specifically, reorder the two helpers so genFakeToken (the token generator used in tests) is declared above setup (the setup function that calls useProviderJwt and setupQuery) and ensure setup remains the last helper in that describe block.apps/deploy-web/src/hooks/useUser.spec.tsx (1)
36-44:⚠️ Potential issue | 🟠 MajorRemove ineffective mock and follow jest-mock-extended pattern.
The
mock()call creates a mock object but the return value is not stored or passed as a dependency to the hook under test. Without dependency injection orvi.mock()module registration, this mock has no effect—useIsRegisteredUsercalls the realuseCustomUserhook. The tests pass only becauseUserProvideris seeded directly withcustomUser.Per coding guidelines, use
jest-mock-extendedto create mocks and pass them as dependencies. Refactor to either inject the mockeduseCustomUseras a dependency or usevi.mock()to replace the module before calling the hook.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/useUser.spec.tsx` around lines 36 - 44, The current test calls mock() which isn't injected so useIsRegisteredUser still invokes the real useCustomUser; replace this by creating a proper mock of useCustomUser (useCustomUser) via jest-mock-extended and ensure it's injected or module-mocked before the hook is imported—either refactor useIsRegisteredUser to accept a dependency (e.g., pass a mocked useCustomUser) or call vi.mock(...) to replace the module that exports useCustomUser with your jest-mock-extended mock; also remove the ineffective raw mock() call and ensure checkSession is a mocked function on the mocked useCustomUser object so tests exercise the mocked behavior.apps/deploy-web/src/queries/useGitlabQuery.spec.tsx (1)
22-24:⚠️ Potential issue | 🟡 MinorUse
<Subject>.namein the root describe.The root suite uses a hardcoded string, which violates the test naming guideline and breaks refactor tooling. Consider promoting each hook’s suite to root level and use the hook’s
.nameinstead.🔧 Suggested restructure (apply pattern to all hook suites)
-describe("useGitlabQuery", () => { - describe("useGitLabFetchAccessToken", () => { +describe(useGitLabFetchAccessToken.name, () => { it("fetches access token and update token state", async () => { ... }); - }); +}); + +describe(useGitLabUserProfile.name, () => { + ... +});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".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useGitlabQuery.spec.tsx` around lines 22 - 24, Replace the hardcoded root describe string with the actual hook/function .name and promote each nested hook suite to use .name as well; specifically change describe("useGitlabQuery", ...) to describe(useGitlabQuery.name, ...) and change the nested describe("useGitLabFetchAccessToken", ...) to describe(useGitLabFetchAccessToken.name, ...), ensuring the test file references the actual exported function identifiers useGitlabQuery and useGitLabFetchAccessToken so automated refactors can locate them.apps/deploy-web/src/utils/domUtils.spec.ts (1)
33-66:⚠️ Potential issue | 🟡 MinorAlign
setupplacement/signature with test conventions.Line 33-66 defines
setupinside a nesteddescribeand above the tests, and it has no parameter. The repo rule expectssetupat the bottom of the rootdescribeblock and to accept a single inline-typed parameter. Consider making each function its own rootdescribe(or hoisting a single root helper) and movingsetupbelow the tests with the required signature.As per coding guidelines,
**/*.spec.{ts,tsx}: 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.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/utils/domUtils.spec.ts` around lines 33 - 66, The current setup function is defined inside a nested describe, lacks the required single-parameter inline-typed signature, and sits above tests; move the setup helper to the bottom of the root describe (or make each tested function its own root describe) and change its signature to accept one inline-typed parameter (e.g., options: { /* inline type */ }) with no explicit return type, ensure it constructs and returns the object-under-test (mocks like createObjectURLMock, revokeObjectURLMock, setAttributeMock, appendChildMock, removeChildMock, and createElementMock) without sharing external state, and remove any top-level shared variables so tests call setup(...) to get their isolated mocks.apps/deploy-web/src/components/alerts/NotificationChannelCreateContainer/NotificationChannelFormContainer.spec.tsx (4)
73-111:⚠️ Potential issue | 🟡 Minor
setupmust accept a single typed parameter.The current
setupsignature has no parameters, which violates the required setup shape for spec files.✅ Conform to the required setup signature
- async function setup() { + async function setup({ onCreate = vi.fn() }: { onCreate?: () => void } = {}) { @@ - <NotificationChannelCreateContainer onCreate={vi.fn()}>{childCapturer.renderChild}</NotificationChannelCreateContainer> + <NotificationChannelCreateContainer onCreate={onCreate}>{childCapturer.renderChild}</NotificationChannelCreateContainer>As per coding guidelines,
**/*.spec.{ts,tsx}: 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.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelCreateContainer/NotificationChannelFormContainer.spec.tsx` around lines 73 - 111, Change the setup helper to accept a single inline-typed parameter and return the test fixtures without an explicit return type: update the current async function setup() to async function setup(params: { /* optional overrides */ } = {}) and use that params object for any overrides (e.g., input values or services) instead of relying on outer shared state; keep the function at the bottom of the root describe, construct and return the object under test (requestFn, input, child) inside the function, and avoid declaring a return type. Ensure references inside the function (createAPIClient, NotificationChannelCreateContainer, childCapturer, queryClient) remain unchanged while wiring any overrides from the single parameter.
78-98:⚠️ Potential issue | 🟠 MajorRemove
anyfromRequestFntyping.
RequestFn<any, Error>violates the repo rule againstanyand weakens type safety around the API mock.🧩 Typed alternative without `any`
- const requestFn = vi.fn( + type NotificationChannelData = components["schemas"]["NotificationChannelOutput"]["data"]; + const requestFn = vi.fn<RequestFn<NotificationChannelData, Error>>( () => Promise.resolve({ @@ - }) as Promise<RequestFnResponse<components["schemas"]["NotificationChannelOutput"]["data"], unknown>> + }) as Promise<RequestFnResponse<NotificationChannelData, unknown>> ); @@ - requestFn: requestFn as RequestFn<any, Error>, + requestFn,As per coding guidelines,
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelCreateContainer/NotificationChannelFormContainer.spec.tsx` around lines 78 - 98, The test is casting the mock to RequestFn<any, Error>, which uses banned any; instead tighten the generic types for requestFn/createAPIClient to the actual response and error shapes used in the test (the RequestFnResponse generic is already using components["schemas"]["NotificationChannelOutput"]["data"]), so change the cast on requestFn to a concrete RequestFn<typeof components["schemas"]["NotificationChannelOutput"]["data"], Error> (or the equivalent proper type alias used in the codebase) and pass that typed requestFn into createAPIClient; update the requestFn declaration/type to match RequestFnResponse<components["schemas"]["NotificationChannelOutput"]["data"], unknown> so no any cast is needed.
22-41:⚠️ Potential issue | 🟡 MinorUse
queryByTestIdin expectations.These assertions use
getByTestId, which conflicts with the repo test guideline for deploy-web specs.♻️ Suggested change
- expect(screen.getByTestId("notification-channel-create-success-notification")).toBeInTheDocument(); + expect(screen.queryByTestId("notification-channel-create-success-notification")).toBeInTheDocument(); @@ - expect(screen.getByTestId("notification-channel-create-error-notification")).toBeInTheDocument(); + expect(screen.queryByTestId("notification-channel-create-error-notification")).toBeInTheDocument();As per coding guidelines,
{apps/deploy-web,apps/provider-console}/**/*.spec.tsx: UsequeryBymethods instead ofgetBymethods in test expectations.Also applies to: 51-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelCreateContainer/NotificationChannelFormContainer.spec.tsx` around lines 22 - 41, The test uses screen.getByTestId(...) in its assertions which violates the deploy-web spec guideline; replace getByTestId with queryByTestId in the assertions (e.g., change expect(screen.getByTestId("notification-channel-create-success-notification")).toBeInTheDocument() to use expect(screen.queryByTestId("notification-channel-create-success-notification")).toBeInTheDocument()), and make the same replacement for the other occurrences noted (lines ~51-70) in NotificationChannelFormContainer.spec.tsx so expectations use queryBy* instead of getBy* while keeping the rest of the assertion logic (requestFn checks and toBeInTheDocument) unchanged.
22-41: 🛠️ Refactor suggestion | 🟠 MajorFix setup function signature and remove
anytype annotation.The
setupfunction at line 73 must accept a single parameter with inline type definition per coding guidelines. Additionally, line 95 usesRequestFn<any, Error>which violates the no-any guideline—define the proper TypeScript type instead ofany.
- Line 73: Add parameter with inline type to
setupfunction- Line 95: Replace
anywith the proper type for the RequestFn generic🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelCreateContainer/NotificationChannelFormContainer.spec.tsx` around lines 22 - 41, Update the test `setup` function to accept a single parameter with an inline type instead of no params and removable `any`; change the signature to take an object like `{ requestFn }` with an inline type annotation using the concrete request payload type (matching the body shape the test posts) and replace `RequestFn<any, Error>` with `RequestFn<CreateNotificationChannelRequest, Error>` (or the actual request DTO name used in the codebase) so `requestFn` is strongly typed; update all call sites in the spec to pass that object. Ensure you reference the `setup` function and the `requestFn` variable and use the concrete request type for the `RequestFn` generic instead of `any`.apps/deploy-web/src/components/alerts/NotificationChannelsGuard/NotificationChannelsGuard.spec.tsx (2)
29-37:⚠️ Potential issue | 🟡 MinorMake
setup’s parameter type inline.The guideline requires an inline param type rather than
Partial<Props>.♻️ Proposed fix
- function setup(providedProps: Partial<Props> = {}) { + function setup( + providedProps: { + isFetched?: Props["isFetched"]; + data?: Props["data"]; + components?: Props["components"]; + } = {} + ) {As per coding guidelines, “**/*.spec.{ts,tsx}: The
setupfunction must … accept a single parameter with inline type definition”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelsGuard/NotificationChannelsGuard.spec.tsx` around lines 29 - 37, The setup function currently types its parameter as Partial<Props>; change its signature to use an inline object type instead (e.g., setup(providedProps: { isFetched?: boolean; data?: typeof props.data; components?: { AccountEmailChannelCreator?: React.ComponentType } } = {})) so it no longer references Partial<Props>; update the default props merge inside setup to remain the same and ensure the inline type options match the shape used in the function (isFetched, data, components / AccountEmailChannelCreator) and keep optional markers for all properties.
11-20:⚠️ Potential issue | 🟡 MinorSwitch expectations to
queryBy*in deploy-web specs.The coding guidelines require
queryBy*for expectations in {apps/deploy-web,apps/provider-console}/**/*.spec.tsx files.♻️ Proposed fix
- expect(screen.getByTestId("loading-blocker")).toBeInTheDocument(); + expect(screen.queryByTestId("loading-blocker")).toBeInTheDocument(); ... - expect(screen.getByText("To start using alerting you need to add at least one notification channel")).toBeInTheDocument(); - expect(screen.getByRole("link", { name: /add notification channel/i })).toBeInTheDocument(); + expect(screen.queryByText("To start using alerting you need to add at least one notification channel")).toBeInTheDocument(); + expect(screen.queryByRole("link", { name: /add notification channel/i })).toBeInTheDocument(); ... - expect(screen.getByTestId(childTestId)).toBeInTheDocument(); + expect(screen.queryByTestId(childTestId)).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelsGuard/NotificationChannelsGuard.spec.tsx` around lines 11 - 20, The tests in NotificationChannelsGuard.spec.tsx use getBy* queries but guidelines require queryBy* in deploy-web specs; update the three assertions that call getByTestId("loading-blocker"), getByText("To start using alerting you need to add at least one notification channel"), and getByRole("link", { name: /add notification channel/i }) to use queryByTestId, queryByText, and queryByRole respectively (keep the same expect(...).toBeInTheDocument() checks) and leave the setup(...) calls unchanged so the test data flow via setup() continues to work.apps/deploy-web/src/queries/useGrantsQuery.spec.tsx (1)
174-182:⚠️ Potential issue | 🟡 MinorRemove
as anycasts and replace with typed mock properties.Both mocks at lines 174-182 and 201-209 use
as anywhich violates the no-anyguideline. Replace with property assignments after mock creation:Typed mock alternative
- const chainApiHttpClient = mock<FallbackableHttpClient>({ - isFallbackEnabled: false, - get: vi.fn().mockResolvedValue({ - data: { - allowances: mockData, - pagination: { next_key: null, total: mockData.length } - } - }) - } as any); + const chainApiHttpClient = mock<FallbackableHttpClient>(); + chainApiHttpClient.isFallbackEnabled = false; + chainApiHttpClient.get.mockResolvedValue({ + data: { + allowances: mockData, + pagination: { next_key: null, total: mockData.length } + } + }); @@ - const chainApiHttpClient = mock<FallbackableHttpClient>({ - isFallbackEnabled: false, - get: vi.fn().mockResolvedValue({ - data: { - allowances: [], - pagination: { next_key: null, total: 0 } - } - }) - } as any); + const chainApiHttpClient = mock<FallbackableHttpClient>(); + chainApiHttpClient.isFallbackEnabled = false; + chainApiHttpClient.get.mockResolvedValue({ + data: { + allowances: [], + pagination: { next_key: null, total: 0 } + } + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useGrantsQuery.spec.tsx` around lines 174 - 182, Remove the runtime "as any" casts on the test mocks (the chainApiHttpClient instance and the other mock around lines 201-209) and instead create typed mocks via mock<FallbackableHttpClient>() and then assign properties on the returned object with proper types, e.g. set isFallbackEnabled = false and set get = vi.fn().mockResolvedValue(...) (and analogous assignments for the second mock), so the mock object remains strongly typed without using "as any".apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts (1)
315-362:⚠️ Potential issue | 🟡 MinorKeep
setupas the last helper inside the root describe.
createRequestsits belowsetup, sosetupis not at the bottom of the root describe block. MovecreateRequestabovesetup(or outside the describe) to comply with the test structure rule.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, 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."🔧 Proposed reorder
- function setup(input: { - route: string; - schema?: z.ZodSchema<any>; - if?: (context: any) => boolean | Promise<boolean> | GetServerSidePropsResult<any> | Promise<GetServerSidePropsResult<any>>; - handler?: (context: any) => Promise<any> | any; - context?: Partial<Omit<AppTypedContext, "services"> & { services?: Partial<AppTypedContext["services"]> }>; - }) { - const context: AppTypedContext = { - req: createRequest(), - res: mock<GetServerSidePropsContext["res"]>(), - query: {}, - params: {}, - resolvedUrl: "/test", - locale: "en", - locales: ["en"], - defaultLocale: "en", - session: null, - ...input.context, - services: { - ...services, - userTracker: mock<typeof services.userTracker>(), - getSession: vi.fn(async () => null), - ...input.context?.services - } - }; - - return defineServerSideProps({ - route: input.route, - schema: input.schema, - if: input.if, - handler: input.handler - })(context); - } - - function createRequest({ headers, ...input }: Partial<GetServerSidePropsContext["req"]> = {}) { - return mock<GetServerSidePropsContext["req"]>({ - url: "/test", - headers: { - host: "localhost", - "content-type": "application/json", - "x-forwarded-host": "localhost", - "x-forwarded-for": "127.0.0.1", - "x-forwarded-proto": "http", - ...headers - }, - ...input - }); - } + function createRequest({ headers, ...input }: Partial<GetServerSidePropsContext["req"]> = {}) { + return mock<GetServerSidePropsContext["req"]>({ + url: "/test", + headers: { + host: "localhost", + "content-type": "application/json", + "x-forwarded-host": "localhost", + "x-forwarded-for": "127.0.0.1", + "x-forwarded-proto": "http", + ...headers + }, + ...input + }); + } + + function setup(input: { + route: string; + schema?: z.ZodSchema<any>; + if?: (context: any) => boolean | Promise<boolean> | GetServerSidePropsResult<any> | Promise<GetServerSidePropsResult<any>>; + handler?: (context: any) => Promise<any> | any; + context?: Partial<Omit<AppTypedContext, "services"> & { services?: Partial<AppTypedContext["services"]> }>; + }) { + const context: AppTypedContext = { + req: createRequest(), + res: mock<GetServerSidePropsContext["res"]>(), + query: {}, + params: {}, + resolvedUrl: "/test", + locale: "en", + locales: ["en"], + defaultLocale: "en", + session: null, + ...input.context, + services: { + ...services, + userTracker: mock<typeof services.userTracker>(), + getSession: vi.fn(async () => null), + ...input.context?.services + } + }; + + return defineServerSideProps({ + route: input.route, + schema: input.schema, + if: input.if, + handler: input.handler + })(context); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts` around lines 315 - 362, The setup helper must be the last helper inside the root describe, so move the createRequest function (currently defined below setup) to above setup or outside the describe block so setup sits last; ensure the setup function signature and body (creating AppTypedContext with services, using createRequest, and returning defineServerSideProps({...})(context)) remain unchanged and continue to reference AppTypedContext, createRequest, services, and defineServerSideProps.
🧹 Nitpick comments (13)
apps/deploy-web/src/utils/keyValue/keyValue.spec.ts (1)
5-5: UsekvArrayToObject.nameinstead of hardcoded string.As per coding guidelines, use
<Subject>.namein the root describe suite description to enable automated refactoring tools to find all references.Suggested fix
-describe("kvArrayToObject", () => { +describe(kvArrayToObject.name, () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/utils/keyValue/keyValue.spec.ts` at line 5, The top-level test suite should use the function's .name property instead of a hardcoded string; update the describe call to use kvArrayToObject.name (and ensure the kvArrayToObject symbol is imported or referenced in the test file) so the suite header is derived from the actual function name and supports automated refactors.apps/deploy-web/src/services/auth/auth/interceptors.spec.ts (1)
9-9: Test description should use present simple without "should".Per coding guidelines, test descriptions should use present simple, 3rd person singular without prepending "should".
Suggested fix
- it("should set baseURL to proxy", () => { + it("sets baseURL to proxy", () => {As per coding guidelines:
Use present simple, 3rd person singular for test descriptions without prepending 'should'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/services/auth/auth/interceptors.spec.ts` at line 9, Rename the test description on the it(...) call in interceptors.spec.ts from "should set baseURL to proxy" to a present-simple, 3rd-person-singular form (e.g., "sets baseURL to proxy"); update the string passed to the it function so the test reads "it('sets baseURL to proxy', ...)".apps/deploy-web/src/queries/useSaveSettings.spec.tsx (1)
82-82: Remove "should" prefix from test description.As per coding guidelines, test descriptions should use present simple, 3rd person singular without prepending "should".
- it("should fetch deposit params successfully", async () => { + it("fetches deposit params successfully", async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useSaveSettings.spec.tsx` at line 82, Update the test title in the it block inside useSaveSettings.spec.tsx: replace the current description "should fetch deposit params successfully" with a present simple, 3rd person singular form like "fetches deposit params successfully" so the test name follows the project's guideline (locate the it(...) for "should fetch deposit params successfully" and change its string).apps/deploy-web/src/queries/useTemplateQuery.spec.tsx (3)
285-289: Avoid mixingfindBy*queries insidevi.waitFor.
screen.findByTextalready has built-in waiting/retry logic. Wrapping it insidevi.waitForwith anasynccallback mixes two different waiting mechanisms, which is redundant and can lead to unexpected timing behavior or flaky tests.Consider separating the assertions: use
vi.waitForfor the synchronous state checks, then awaitfindByTextoutside.Suggested refactor
- await vi.waitFor(async () => { - expect(consoleApiHttpClient.post).toHaveBeenCalledWith("/v1/user/addFavoriteTemplate/template-1"); - expect(result.current.isSuccess).toBe(true); - expect(await screen.findByText(/Favorite added!/i)).toBeInTheDocument(); - }); + await vi.waitFor(() => { + expect(consoleApiHttpClient.post).toHaveBeenCalledWith("/v1/user/addFavoriteTemplate/template-1"); + expect(result.current.isSuccess).toBe(true); + }); + expect(await screen.findByText(/Favorite added!/i)).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useTemplateQuery.spec.tsx` around lines 285 - 289, The test mixes async query retry mechanisms by calling screen.findByText inside vi.waitFor; update the test in useTemplateQuery.spec.tsx so vi.waitFor only wraps the synchronous state assertions (e.g., expect(consoleApiHttpClient.post).toHaveBeenCalledWith and expect(result.current.isSuccess).toBe(true)), then after vi.waitFor completes await screen.findByText(/Favorite added!/i) and assert it is in the document — keep the two steps separate and remove the async screen.findByText call from inside the vi.waitFor callback.
91-91: Use.namefor describe suite descriptions consistently.Several describe blocks use hardcoded strings instead of
<Subject>.name. Other describe blocks in this same file correctly use.name(e.g., lines 59, 133, 173, 231). For consistency and to enable automated refactoring tools to find all references, update these:
- Line 91:
"useUserFavoriteTemplates"→useUserFavoriteTemplates.name- Line 273:
"useAddFavoriteTemplate"→useAddFavoriteTemplate.name- Line 316:
"useRemoveFavoriteTemplate"→useRemoveFavoriteTemplate.name- Line 359:
"useTemplates"→useTemplates.nameSuggested changes
- describe("useUserFavoriteTemplates", () => { + describe(useUserFavoriteTemplates.name, () => {- describe("useAddFavoriteTemplate", () => { + describe(useAddFavoriteTemplate.name, () => {- describe("useRemoveFavoriteTemplate", () => { + describe(useRemoveFavoriteTemplate.name, () => {- describe("useTemplates", () => { + describe(useTemplates.name, () => {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."Also applies to: 273-273, 316-316, 359-359
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useTemplateQuery.spec.tsx` at line 91, The describe suite titles use hardcoded strings instead of the subject .name, which breaks consistency and refactorability; update the four describe calls so they use the function name property: replace "useUserFavoriteTemplates" with useUserFavoriteTemplates.name, "useAddFavoriteTemplate" with useAddFavoriteTemplate.name, "useRemoveFavoriteTemplate" with useRemoveFavoriteTemplate.name, and "useTemplates" with useTemplates.name so the test suites reference the actual function symbols.
328-332: Same issue: avoid mixingfindBy*insidevi.waitFor.Apply the same refactor as the previous comment to separate the waiting mechanisms.
Suggested refactor
- await vi.waitFor(async () => { - expect(consoleApiHttpClient.delete).toHaveBeenCalledWith("/v1/user/removeFavoriteTemplate/template-1"); - expect(result.current.isSuccess).toBe(true); - expect(await screen.findByText(/Favorite removed/i)).toBeInTheDocument(); - }); + await vi.waitFor(() => { + expect(consoleApiHttpClient.delete).toHaveBeenCalledWith("/v1/user/removeFavoriteTemplate/template-1"); + expect(result.current.isSuccess).toBe(true); + }); + expect(await screen.findByText(/Favorite removed/i)).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useTemplateQuery.spec.tsx` around lines 328 - 332, The test mixes async querying with vi.waitFor (using screen.findByText inside vi.waitFor); refactor by separating the two waits: keep the vi.waitFor to assert synchronous state changes and mock calls (retain checks for consoleApiHttpClient.delete and result.current.isSuccess inside vi.waitFor), then outside/after that wait, await screen.findByText(/Favorite removed/i) to assert DOM text; target the test in useTemplateQuery.spec.tsx and the assertions referencing vi.waitFor, consoleApiHttpClient.delete, result.current.isSuccess, and screen.findByText.apps/deploy-web/src/components/new-deployment/NewDeploymentContainer/NewDeploymentContainer.spec.tsx (2)
27-29: Consider using@testing-library/react'swaitForinstead ofvi.waitFor.While
vi.waitForworks here, the testing-library'swaitForis specifically designed for React Testing Library assertions and integrates better with its DOM queries. This applies to allvi.waitForusages throughout the file.+import { render, screen, waitFor } from "@testing-library/react"; -import { render, screen } from "@testing-library/react";Then use:
- await vi.waitFor(() => { + await waitFor(() => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/new-deployment/NewDeploymentContainer/NewDeploymentContainer.spec.tsx` around lines 27 - 29, Replace usages of vi.waitFor in NewDeploymentContainer.spec.tsx with the testing-library waitFor helper: import waitFor from '@testing-library/react' (or import { waitFor } from '@testing-library/react') at the top of the test file and change calls like vi.waitFor(() => { expect(screen.getByTestId("template-list")).toBeInTheDocument(); }) to waitFor(() => { expect(screen.getByTestId("template-list")).toBeInTheDocument(); }); do the same replacement for any other vi.waitFor occurrences in this spec to ensure proper RTL integration.
19-21: Consider moving testing imports with other external imports.These testing library imports could be grouped with other external dependencies (after line 6) for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/new-deployment/NewDeploymentContainer/NewDeploymentContainer.spec.tsx` around lines 19 - 21, The test file imports from testing-library (render, screen) at the top; move those imports to the external imports group alongside other external dependencies to match project import ordering. Reorder the import lines so that "import { render, screen } from '@testing-library/react';" sits with the other external imports (near where other third-party imports like ComponentMock and TestContainerProvider are), keeping the symbols render and screen unchanged.apps/deploy-web/src/hooks/useSdlEnv/useSdlEnv.spec.tsx (2)
12-12: UseuseSdlEnv.nameinstead of a hardcoded string in the describe block.As per coding guidelines, use
<Subject>.namein the root describe suite description to enable automated refactoring tools to find all references.Suggested fix
-describe("useSdlEnv", () => { +describe(useSdlEnv.name, () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/useSdlEnv/useSdlEnv.spec.tsx` at line 12, Replace the hardcoded describe string "useSdlEnv" with the actual function name reference useSdlEnv.name in the root test suite; locate the describe("useSdlEnv", ...) block in the useSdlEnv.spec.tsx file and change the first argument to useSdlEnv.name so automated refactors can find the subject (ensure the useSdlEnv symbol is imported in the spec so useSdlEnv.name resolves).
3-3: Consider using Testing Library'swaitForinstead ofvi.waitForfor React hook tests.While
vi.waitForis a valid Vitest API (contrary to the previous review comment), "The warning itself was pretty easy to resolve: instead of calling vi.waitFor() coming from Vitest, I now use waitFor imported from@testing-library/react." This is because RTL's waitFor handles React's act() semantics correctly, while vi.waitFor does not.For React component/hook testing with Testing Library, RTL's
waitForis generally preferred to avoid potentialact()warnings.Suggested fix
Line 9:
-import { act, renderHook } from "@testing-library/react"; +import { act, renderHook, waitFor } from "@testing-library/react";Line 124:
- form: await vi.waitFor(() => methods) + form: await waitFor(() => methods)Also applies to: 124-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/useSdlEnv/useSdlEnv.spec.tsx` at line 3, The tests currently use vi.waitFor (Vitest) which doesn't wrap React's act semantics; update the test to import waitFor from "@testing-library/react" and replace all uses of vi.waitFor with the imported waitFor (e.g., change any vi.waitFor(() => ...) calls to waitFor(() => ...)); keep the existing vi imports for mocking (vi) but remove vi.waitFor usage so RTL's waitFor handles async React updates correctly in useSdlEnv.spec.tsx and any assertions that depend on it.apps/deploy-web/src/components/alerts/NotificationChannelsListContainer/NotificationChannelsListContainer.spec.tsx (1)
16-16: UseNotificationChannelsListContainer.nameinstead of hardcoded string.As per coding guidelines, use
<Subject>.namein the root describe suite description to enable automated refactoring tools to find all references.♻️ Suggested fix
-describe("NotificationChannelsListContainer", () => { +describe(NotificationChannelsListContainer.name, () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelsListContainer/NotificationChannelsListContainer.spec.tsx` at line 16, Replace the hardcoded suite title string in the root describe call with the component's .name property: change the describe("NotificationChannelsListContainer", ...) to use NotificationChannelsListContainer.name so refactors can locate the suite; locate the describe invocation that references the NotificationChannelsListContainer component and update its first argument accordingly.apps/deploy-web/src/components/alerts/NotificationChannelForm/NotificationChannelForm.spec.tsx (1)
10-10: UseNotificationChannelForm.namein the root describe.The describe suite uses a hardcoded string. Per coding guidelines, use
<Subject>.nameto enable automated refactoring tools to find all references.As per coding guidelines, `**/*.spec.{ts,tsx}`: Use `.name` in the root describe suite description instead of hardcoded class/service name strings.♻️ Suggested fix
-describe("NotificationChannelForm", () => { +describe(NotificationChannelForm.name, () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/NotificationChannelForm/NotificationChannelForm.spec.tsx` at line 10, Replace the hardcoded root describe string with the component's runtime name: change the describe("NotificationChannelForm", ...) to describe(NotificationChannelForm.name, ...) in the NotificationChannelForm.spec.tsx test so automated refactors can find the suite; locate the test file and update the root describe that references NotificationChannelForm to use NotificationChannelForm.name instead.apps/deploy-web/src/hooks/useUser.spec.tsx (1)
12-12: Consider usinguseIsRegisteredUser.namefor the describe block.As per coding guidelines, test suites should use
<Subject>.nameinstead of hardcoded strings to enable automated refactoring tools to find all references.Suggested change
-describe("useIsRegisteredUser", () => { +describe(useIsRegisteredUser.name, () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/useUser.spec.tsx` at line 12, Replace the hardcoded test suite title with the function's name property: locate the describe call that currently uses the string "useIsRegisteredUser" and change it to useIsRegisteredUser.name so the suite references the actual symbol; ensure you import or reference the useIsRegisteredUser function in the spec file if not already so useIsRegisteredUser.name resolves correctly.
apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsx
Show resolved
Hide resolved
...eploy-web/src/components/alerts/NotificationChannelsGuard/NotificationChannelsGuard.spec.tsx
Show resolved
Hide resolved
...eploy-web/src/components/alerts/NotificationChannelsGuard/NotificationChannelsGuard.spec.tsx
Show resolved
Hide resolved
...c/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.spec.tsx
Show resolved
Hide resolved
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts
Show resolved
Hide resolved
ed27d98 to
cb2f4ad
Compare
7cd9946 to
3543ea1
Compare
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 (7)
apps/deploy-web/src/components/new-deployment/NewDeploymentContainer/NewDeploymentContainer.spec.tsx (1)
27-31:⚠️ Potential issue | 🟡 MinorUse
queryByTestIdin expectations for deploy-web specs.These expectations use
getByTestId, which violates the deploy-web test guideline. Please switch toqueryByTestIdfor consistency.As per coding guidelines: `{apps/deploy-web,apps/provider-console}/**/*.spec.tsx : Use queryBy methods instead of getBy methods in test expectations`.Example fix (apply similarly across this file)
- expect(screen.getByTestId("template-list")).toBeInTheDocument(); + expect(screen.queryByTestId("template-list")).toBeInTheDocument();Also applies to: 38-42, 57-63, 284-288
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/new-deployment/NewDeploymentContainer/NewDeploymentContainer.spec.tsx` around lines 27 - 31, Replace assertions that use screen.getByTestId(...) with screen.queryByTestId(...) in this spec: change the waitFor expectation around "template-list" and the subsequent checks for "manifest-edit" and "create-lease" to use queryByTestId; likewise update the other occurrences called out (the blocks around lines 38-42, 57-63, and 284-288) so all test expectations use queryByTestId to comply with the deploy-web guideline.apps/deploy-web/src/lib/websocket/WebsocketSession.spec.ts (2)
349-360:⚠️ Potential issue | 🟡 Minor
setupshould construct and return the subject under test.
setupcurrently returns only mocks, but the guideline expects it to create and return theWebsocketSessioninstance as well. Consider returning{ session, websocket, websocketFactory }and updating call sites to use the returned session.As per coding guidelines "Use
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/lib/websocket/WebsocketSession.spec.ts` around lines 349 - 360, The setup helper currently only builds mocks (createWebsocketMock, websocketFactory) but must also construct the WebsocketSession under test and return it; instantiate a WebsocketSession (using the websocketFactory/mock as its dependency) inside setup, then return an object like { session, websocket, websocketFactory } so tests can use session directly and update all call sites to pull session from setup instead of relying on beforeEach or separate construction. Ensure the created session uses the same websocket/mock readyState override you apply to the websocket prior to passing it to the WebsocketSession.
60-69:⚠️ Potential issue | 🟡 MinorAdd explicit types to transformer mocks to avoid implicit
any.Lines 62 and 124 pass untyped parameters to
vi.fn, which defaults toanyand violates the repo's strict typing rule. TheWebsocketSessionclass is instantiated without explicit generic type parameters, so TypeScript cannot infer the callback parameter types.Add explicit types for the mock arguments:
Suggested fix
+type TestMessage = { type: string; data?: string; value?: number; command?: string; response?: string }; -const transformSentMessage = vi.fn(msg => `custom:${JSON.stringify(msg)}`); +const transformSentMessage = vi.fn((msg: TestMessage) => `custom:${JSON.stringify(msg)}`); -const transformReceivedMessage = vi.fn(msg => ({ transformed: msg })); +const transformReceivedMessage = vi.fn((msg: string) => ({ transformed: msg }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/lib/websocket/WebsocketSession.spec.ts` around lines 60 - 69, The mock transformer passed to vi.fn is untyped causing implicit any; update the test to provide explicit type annotations for the mock callback and/or the WebsocketSession generic so TypeScript can infer the message type: give the transformSentMessage mock a typed signature (e.g., vi.fn<(msg: YourMessageType) => string>(...)) and instantiate new WebsocketSession with the appropriate generic parameter (e.g., new WebsocketSession<YourMessageType>({ websocketFactory, transformSentMessage })); ensure the same change is applied to the other transformer mock used elsewhere in the spec so no implicit any remains for vi.fn callbacks.apps/deploy-web/src/components/alerts/AlertsListView/AlertsListView.spec.tsx (1)
90-105:⚠️ Potential issue | 🟡 MinorUse an inline type for
setup’s parameter.
The setup helper is in a.spec.tsxfile and should accept a single inline-typed parameter rather thanPartial<Props>.Suggested update
- function setup(props: Partial<Props> = {}) { + function setup(input: { + pagination?: Props["pagination"]; + data?: Props["data"]; + isLoading?: Props["isLoading"]; + onToggle?: Props["onToggle"]; + loadingIds?: Props["loadingIds"]; + onPaginationChange?: Props["onPaginationChange"]; + isError?: Props["isError"]; + dependencies?: Props["dependencies"]; + } = {}) { const defaultProps: Props = { pagination: { page: 1, limit: 10, total: 10, totalPages: 1 }, data: Array.from({ length: 10 }, buildAlert), isLoading: false, onToggle: vi.fn(), loadingIds: new Set(), onPaginationChange: vi.fn(), isError: false, - ...props + ...input };As per coding guidelines,
**/*.spec.{ts,tsx}: Use setup function instead of beforeEach in test files. The setup function must be at the bottom of the root describe block, 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.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/AlertsListView/AlertsListView.spec.tsx` around lines 90 - 105, The setup helper currently types its parameter as Partial<Props>; change it to accept a single inline-typed parameter instead (i.e., replace "props: Partial<Props> = {}" with an inline type literal matching the optional props used: pagination?, data?, isLoading?, onToggle?, loadingIds?, onPaginationChange?, isError?), merge those defaults with the inline-typed props inside setup, and keep the rest of the implementation (using buildAlert, defaultProps, etc.) unchanged so the spec file follows the `.spec.tsx` guideline of using an inline parameter type for setup.apps/deploy-web/src/utils/walletUtils.spec.ts (1)
1103-1110:⚠️ Potential issue | 🟡 MinorAdd a single inline-typed parameter to
setup.
The test guideline requires a single parameter with an inline type definition.Suggested update
- function setup() { + function setup(_input: {} = {}) { const storage = new Map<string, string>();As per coding guidelines,
**/*.spec.{ts,tsx}: Use setup function instead of beforeEach in test files. The setup function must be at the bottom of the root describe block, 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.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/utils/walletUtils.spec.ts` around lines 1103 - 1110, The setup function must accept a single inline-typed parameter; update the existing setup definition (function setup()) to take one inline-typed parameter (for example, options: { /* props used by tests */ }) and use that parameter inside the function when configuring storage and mockLocalStorage (referencing setup, storage, mockLocalStorage, mock<Storage>, getItem, setItem) without adding an explicit return type; ensure the created object-under-test is returned and no shared state is used outside the function.apps/deploy-web/src/services/feature-flag/feature-flag.service.spec.ts (1)
69-99:⚠️ Potential issue | 🟡 MinorAdjust
setupplacement and signature to match test helper rules.
setup()should be the last helper in the rootdescribeand should not declare an explicit return type. MovecreateCtxabovesetupand drop the return type annotation.♻️ Minimal signature fix
- function setup(options?: { enableAll?: boolean; isEnabled?: Mock }): { - service: FeatureFlagService; - unleash: typeof unleashModule; - flagsClient: MockProxy<UnleashClient>; - } { + function setup(options?: { enableAll?: boolean; isEnabled?: Mock }) {As per coding guidelines:
**/*.spec.{ts,tsx}: 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.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/services/feature-flag/feature-flag.service.spec.ts` around lines 69 - 99, Move the createCtx helper above setup and make setup the last helper in the root describe; change setup to accept a single inline-typed parameter (e.g. options?: { enableAll?: boolean; isEnabled?: Mock }) and remove the explicit return type annotation so it returns the created objects implicitly, ensure setup constructs and returns the test objects (service, unleash, flagsClient) without sharing external state, and keep extractSessionId spy creation inside setup so setup remains the solitary factory for the test scenario.apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsx (1)
27-33:⚠️ Potential issue | 🟡 MinorUse
queryByTestIdin expectations per deploy-web test rules.Expectation checks inside
vi.waitForshould usequeryBy*APIs rather thangetBy*.🔧 Example
- expect(screen.getByTestId("alert-remove-success-notification")).toBeInTheDocument(); + expect(screen.queryByTestId("alert-remove-success-notification")).toBeInTheDocument();As per coding guidelines:
{apps/deploy-web,apps/provider-console}/**/*.spec.tsx: UsequeryBymethods instead ofgetBymethods in test expectations.#!/bin/bash # List expectation-only getBy* usages in this spec. rg -n "getByTestId" apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsxAlso applies to: 41-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsx` around lines 27 - 33, The test uses screen.getByTestId inside vi.waitFor which violates the deploy-web rule to use queryBy* in expectations; update the assertions in AlertsListContainer.spec.tsx (the vi.waitFor block(s) that reference requestFn and the success notification) to use screen.queryByTestId(...) and assert presence via toBeInTheDocument() (or appropriate truthiness) instead of getByTestId, and do the same for the other waitFor block around lines referencing the alert-remove-success-notification to ensure all expectation-only getByTestId usages are replaced.
🧹 Nitpick comments (13)
apps/deploy-web/src/lib/auth0/setSession/setSession.spec.ts (1)
13-13: Use present simple 3rd person singular for test description.The description "can set session" uses modal verb "can". Per coding guidelines, it should use present simple 3rd person singular.
Suggested fix
- it("can set session", async () => { + it("sets session", async () => {As per coding guidelines: "Use present simple, 3rd person singular for test descriptions without prepending 'should'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/lib/auth0/setSession/setSession.spec.ts` at line 13, Update the test description string in the it(...) call from "can set session" to a present simple 3rd person singular form (e.g., "sets session") so the test name follows the project's guideline; locate the it("can set session", async () => { ... }) in setSession.spec.ts and replace the description text accordingly.apps/deploy-web/src/queries/useGrantsQuery.spec.tsx (3)
11-11: Remove unused empty import.This empty import statement serves no purpose and should be removed.
🧹 Proposed fix
-import {} from "@testing-library/react";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useGrantsQuery.spec.tsx` at line 11, Remove the unused empty import statement in useGrantsQuery.spec.tsx: delete the line "import {} from \"@testing-library/react\";" so the test file no longer contains a no-op import; verify there are no other references to symbols from "@testing-library/react" that would require importing specific helpers instead.
174-182: Avoidas anytype cast.This violates the coding guideline: "Never use type
anyor cast to typeany." Consider usingmockDeepwith a partial type or properly typing the mock to avoid the cast.♻️ Proposed fix
- const chainApiHttpClient = mock<FallbackableHttpClient>({ + const chainApiHttpClient = mock<Partial<FallbackableHttpClient>>({ isFallbackEnabled: false, get: vi.fn().mockResolvedValue({ data: { allowances: mockData, pagination: { next_key: null, total: mockData.length } } }) - } as any); + });As per coding guidelines: "Never use type
anyor cast to typeany. Always define the proper TypeScript types."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useGrantsQuery.spec.tsx` around lines 174 - 182, The test currently casts the mock to any (chainApiHttpClient = mock<FallbackableHttpClient>({...} as any)), which violates the "no any" rule; replace this with a properly typed mock by using mockDeep<FallbackableHttpClient>(...) or mock<Partial<FallbackableHttpClient>>(...) so the object matches the FallbackableHttpClient interface without any casts, keeping properties like isFallbackEnabled and get (mockResolvedValue returning { data: { allowances: mockData, pagination: { next_key: null, total: mockData.length } } }) typed correctly; ensure you import mockDeep from the mocking lib (or use Partial type) and remove the as any cast.
200-209: Avoidas anytype cast.Same issue as above - the
as anycast violates coding guidelines. Apply the same fix pattern here.♻️ Proposed fix
const chainApiHttpClient = mock<FallbackableHttpClient>({ isFallbackEnabled: false, get: vi.fn().mockResolvedValue({ data: { allowances: [], pagination: { next_key: null, total: 0 } } }) - } as any); + });As per coding guidelines: "Never use type
anyor cast to typeany. Always define the proper TypeScript types."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useGrantsQuery.spec.tsx` around lines 200 - 209, The test currently uses an illegal "as any" cast on the mocked client; replace it with a properly typed mock by giving chainApiHttpClient the FallbackableHttpClient type (or a Partial<FallbackableHttpClient> cast) instead of any. For example, remove "as any" and declare the mock with an explicit type: const chainApiHttpClient: FallbackableHttpClient = { isFallbackEnabled: false, get: vi.fn().mockResolvedValue(...) } or if you only implement some members use "as Partial<FallbackableHttpClient>" when calling mock<...> so the mock<FallbackableHttpClient> call and the chainApiHttpClient variable are correctly typed and no "any" cast remains.apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts (1)
99-117: Migration looks correct; consider movingsetupinside the describe block.The
vi.fn()replacements are correct for the Vitest migration.As a minor pre-existing issue: the
setupfunction is defined outside the rootdescribeblock. As per coding guidelines, it should be at the bottom of the rootdescribeblock. This could be addressed in a follow-up if desired.,
♻️ Optional: Move setup inside describe block
describe("pageGuards", () => { describe("isAuthenticated", () => { // ... tests }); // ... other test suites + + function setup(input?: { enabledFeatures?: string[]; session?: Partial<Session> }) { + return mock<AppTypedContext>({ + getCurrentSession: vi.fn().mockImplementation(async () => { + if (!input?.session) return null; + return { + ...input.session, + accessTokenExpiresAt: input.session.accessTokenExpiresAt ? new Date(input.session.accessTokenExpiresAt).getTime() / 1000 : undefined + }; + }), + services: { + featureFlagService: mock<FeatureFlagService>({ + isEnabledForCtx: vi.fn(async featureName => !!input?.enabledFeatures?.includes(featureName)) + }), + logger: mock<LoggerService>(), + urlService: UrlService + }, + resolvedUrl: faker.internet.url() + }); + } }); - -function setup(input?: { enabledFeatures?: string[]; session?: Partial<Session> }) { - // ... implementation -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts` around lines 99 - 117, Move the standalone setup function into the root describe block in pageGuards.spec.ts (place it at the bottom of that describe) so it follows the test suite as per guidelines; locate the setup function (the one returning mock<AppTypedContext> with getCurrentSession and services.featureFlagService mocks) and cut/paste it inside the root describe scope, ensuring any references to types like AppTypedContext, FeatureFlagService, LoggerService, UrlService, vi and faker remain in scope or adjust imports if needed.apps/deploy-web/src/components/sdl/LogCollectorControl/LogCollectorControl.spec.tsx (2)
74-80: Optional: Remove unnecessaryasynckeyword.Same as above – the callback performs no awaits.
✨ Suggested simplification
- await vi.waitFor( - async () => { + await vi.waitFor( + () => { const selector = form.getValues("services.1.env")?.find(env => env.key === "POD_LABEL_SELECTOR"); expect(selector?.value).toBe("akash.network/manifest-service=new-title"); }, { timeout: 1000 } );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/sdl/LogCollectorControl/LogCollectorControl.spec.tsx` around lines 74 - 80, The vi.waitFor callback is declared async but contains no awaits; remove the unnecessary async keyword to make the callback a plain synchronous function in the test in LogCollectorControl.spec.tsx (the lambda passed to vi.waitFor that reads form.getValues("services.1.env") and asserts selector?.value). Update the arrow function signature to omit async so the test is slightly simpler and avoids an unnecessary microtask wrapper.
60-62: Optional: Remove unnecessaryasynckeyword.The callback doesn't await anything, so
asynccan be removed for clarity.✨ Suggested simplification
- await vi.waitFor(async () => { + await vi.waitFor(() => { expect(form.getValues("services.1.title")).toBe(`new-title-log-collector`); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/sdl/LogCollectorControl/LogCollectorControl.spec.tsx` around lines 60 - 62, The vi.waitFor callback is declared async but does not await anything; remove the unnecessary async keyword from the arrow function passed to vi.waitFor (i.e., change vi.waitFor(async () => { expect(form.getValues("services.1.title")).toBe(`new-title-log-collector`); }); to vi.waitFor(() => { expect(form.getValues("services.1.title")).toBe(`new-title-log-collector`); }); so the test uses a synchronous callback while keeping the await vi.waitFor call intact.apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentAlerts.spec.tsx (1)
13-13: UseDeploymentAlertsView.namein root describe for better refactoring support.The root describe suite should use the component's
.nameproperty instead of a hardcoded string to enable automated refactoring tools to find all references.♻️ Proposed fix
-describe("DeploymentAlerts", () => { +describe(DeploymentAlertsView.name, () => {As per coding guidelines: "Use
<Subject>.namein the root describe suite description instead of hardcoded class/service name strings".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentAlerts.spec.tsx` at line 13, The test's root describe uses a hardcoded string "DeploymentAlerts"; update it to use the component's name property by replacing the literal with DeploymentAlertsView.name so the suite reads describe(DeploymentAlertsView.name, () => { ... }); locate the root describe in DeploymentAlerts.spec.tsx and import or reference the DeploymentAlertsView symbol if not already present, ensuring the test still runs and linter/imports remain valid.apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx (1)
13-13: Consider usingqueryByRolefor consistency with coding guidelines.The coding guidelines specify using
queryBymethods instead ofgetBymethods in test expectations for deploy-web spec files. However, note thatgetByprovides better debugging information when elements are not found.♻️ Optional fix
- expect(screen.getByRole("status")).toBeInTheDocument(); + expect(screen.queryByRole("status")).toBeInTheDocument();As per coding guidelines: "Use
queryBymethods instead ofgetBymethods in test expectations" for{apps/deploy-web,apps/provider-console}/**/*.spec.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx` at line 13, Replace the use of screen.getByRole("status") with screen.queryByRole("status") in the CumulativeSpendingLineChart.spec.tsx assertion; update the expectation to continue using toBeInTheDocument() (queryByRole returns null if missing so the existing toBeInTheDocument() assertion remains valid) and run tests to confirm behavior unchanged — target the line with screen.getByRole("status") in the test file and swap to queryByRole as per the deploy-web spec guideline.apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx (1)
12-12: Consider usingqueryByRolefor consistency with coding guidelines.The coding guidelines specify using
queryBymethods instead ofgetBymethods in test expectations for deploy-web spec files.♻️ Optional fix
- expect(screen.getByRole("status")).toBeInTheDocument(); + expect(screen.queryByRole("status")).toBeInTheDocument();As per coding guidelines: "Use
queryBymethods instead ofgetBymethods in test expectations" for{apps/deploy-web,apps/provider-console}/**/*.spec.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx` at line 12, Replace the use of screen.getByRole("status") with screen.queryByRole("status") in the DailyUsageBarChart.spec.tsx assertion to follow the deploy-web testing guideline; locate the expectation using getByRole and change it to queryByRole (keeping the .toBeInTheDocument() assertion), ensuring the test still imports/uses screen from `@testing-library/react`.apps/deploy-web/src/services/stripe/stripe.service.spec.ts (1)
8-8: Consider usingStripeService.namefor the root describe.The coding guidelines recommend using
<Subject>.namein the root describe suite description instead of hardcoded strings to enable automated refactoring tools to find all references.Suggested change
-describe("StripeService", () => { +describe(StripeService.name, () => {As per coding guidelines: "Use
<Subject>.namein the root describe suite description instead of hardcoded class/service name strings."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/services/stripe/stripe.service.spec.ts` at line 8, Replace the hardcoded root describe string with the service's name property: change the root test suite declaration from describe("StripeService", ...) to describe(StripeService.name, ...) so the suite uses StripeService.name for automated refactors; update any imports or references necessary to ensure StripeService is in scope for the test file.apps/deploy-web/src/queries/useMarketData.spec.tsx (1)
11-11: Consider usinguseMarketData.namefor the root describe.The coding guidelines recommend using
<Subject>.namein the root describe suite description instead of hardcoded strings.Suggested change
-describe("useMarketData", () => { +describe(useMarketData.name, () => {As per coding guidelines: "Use
<Subject>.namein the root describe suite description instead of hardcoded class/service name strings."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useMarketData.spec.tsx` at line 11, The root test suite description currently uses a hardcoded string "useMarketData"; change it to use the function's name property by replacing that string with useMarketData.name so the root describe uses useMarketData.name (locate the describe call in this spec file that references useMarketData and update it accordingly).apps/deploy-web/src/hooks/useTrialDeploymentTimeRemaining.spec.ts (1)
17-17: Fix the root describe name to match the function.The describe uses
"useTrialTimeRemaining"but the actual function isuseTrialDeploymentTimeRemaining. UseuseTrialDeploymentTimeRemaining.nameto ensure consistency and enable automated refactoring.Suggested change
-describe("useTrialTimeRemaining", () => { +describe(useTrialDeploymentTimeRemaining.name, () => {As per coding guidelines: "Use
<Subject>.namein the root describe suite description instead of hardcoded class/service name strings."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/useTrialDeploymentTimeRemaining.spec.ts` at line 17, The root describe string is hardcoded as "useTrialTimeRemaining" but should reference the actual hook to stay consistent and support automated refactors; update the test suite declaration in useTrialDeploymentTimeRemaining.spec.ts so the top-level describe uses useTrialDeploymentTimeRemaining.name (referencing the exported hook function name) instead of the hardcoded string, ensuring the describe matches the function under test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/deploy-web/src/hooks/useProviderCredentials/useProviderCredentials.spec.ts`:
- Around line 173-176: Update the setup parameter typing so generateToken is a
properly generic Mock instead of an untyped Mock<any, any>; specifically, type
generateToken using the Mock generic with the actual parameter and return types
of UseProviderJwtResult['generateToken'] (e.g.,
Mock<Parameters<UseProviderJwtResult['generateToken']>,
ReturnType<UseProviderJwtResult['generateToken']>>), and import
UseProviderJwtResult if necessary so the compiler can resolve the signature.
In `@apps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.ts`:
- Around line 781-782: The test currently uses untyped JSON.parse(...) for
websocket.send payloads which returns any; define local TypeScript interfaces
for the different message shapes (e.g., UrlMessage with url:string, AuthMessage
with auth:{token:string} or similar, DataMessage with
data:Record<string,unknown>) and replace each JSON.parse call in
provider-proxy.service.spec.ts with a typed parse (e.g., const sentMessage =
JSON.parse(payload) as UrlMessage) so each assertion uses a properly typed
variable; apply the same pattern to all websocket.send parse sites (the ten
occurrences) and ensure the local types are declared near the top of the test
file for reuse.
In `@apps/deploy-web/src/utils/domUtils.spec.ts`:
- Around line 33-58: Move the current setup helper into the bottom of the root
describe block and refactor it to accept a single inline-typed parameter (e.g.,
options: { /* inline type */ }) instead of no arguments; inside the function
recreate the mocks (createObjectURLMock, revokeObjectURLMock, setAttributeMock,
appendChildMock, removeChildMock), spyOn document.createElement to return
linkMock for "a", spyOn document.body.appendChild/removeChild, build and return
the object under test (no explicit return type) so there is no shared state
between tests and the helper follows the spec-guideline for setup placement and
signature.
---
Outside diff comments:
In
`@apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsx`:
- Around line 27-33: The test uses screen.getByTestId inside vi.waitFor which
violates the deploy-web rule to use queryBy* in expectations; update the
assertions in AlertsListContainer.spec.tsx (the vi.waitFor block(s) that
reference requestFn and the success notification) to use
screen.queryByTestId(...) and assert presence via toBeInTheDocument() (or
appropriate truthiness) instead of getByTestId, and do the same for the other
waitFor block around lines referencing the alert-remove-success-notification to
ensure all expectation-only getByTestId usages are replaced.
In
`@apps/deploy-web/src/components/alerts/AlertsListView/AlertsListView.spec.tsx`:
- Around line 90-105: The setup helper currently types its parameter as
Partial<Props>; change it to accept a single inline-typed parameter instead
(i.e., replace "props: Partial<Props> = {}" with an inline type literal matching
the optional props used: pagination?, data?, isLoading?, onToggle?, loadingIds?,
onPaginationChange?, isError?), merge those defaults with the inline-typed props
inside setup, and keep the rest of the implementation (using buildAlert,
defaultProps, etc.) unchanged so the spec file follows the `.spec.tsx` guideline
of using an inline parameter type for setup.
In
`@apps/deploy-web/src/components/new-deployment/NewDeploymentContainer/NewDeploymentContainer.spec.tsx`:
- Around line 27-31: Replace assertions that use screen.getByTestId(...) with
screen.queryByTestId(...) in this spec: change the waitFor expectation around
"template-list" and the subsequent checks for "manifest-edit" and "create-lease"
to use queryByTestId; likewise update the other occurrences called out (the
blocks around lines 38-42, 57-63, and 284-288) so all test expectations use
queryByTestId to comply with the deploy-web guideline.
In `@apps/deploy-web/src/lib/websocket/WebsocketSession.spec.ts`:
- Around line 349-360: The setup helper currently only builds mocks
(createWebsocketMock, websocketFactory) but must also construct the
WebsocketSession under test and return it; instantiate a WebsocketSession (using
the websocketFactory/mock as its dependency) inside setup, then return an object
like { session, websocket, websocketFactory } so tests can use session directly
and update all call sites to pull session from setup instead of relying on
beforeEach or separate construction. Ensure the created session uses the same
websocket/mock readyState override you apply to the websocket prior to passing
it to the WebsocketSession.
- Around line 60-69: The mock transformer passed to vi.fn is untyped causing
implicit any; update the test to provide explicit type annotations for the mock
callback and/or the WebsocketSession generic so TypeScript can infer the message
type: give the transformSentMessage mock a typed signature (e.g., vi.fn<(msg:
YourMessageType) => string>(...)) and instantiate new WebsocketSession with the
appropriate generic parameter (e.g., new WebsocketSession<YourMessageType>({
websocketFactory, transformSentMessage })); ensure the same change is applied to
the other transformer mock used elsewhere in the spec so no implicit any remains
for vi.fn callbacks.
In `@apps/deploy-web/src/services/feature-flag/feature-flag.service.spec.ts`:
- Around line 69-99: Move the createCtx helper above setup and make setup the
last helper in the root describe; change setup to accept a single inline-typed
parameter (e.g. options?: { enableAll?: boolean; isEnabled?: Mock }) and remove
the explicit return type annotation so it returns the created objects
implicitly, ensure setup constructs and returns the test objects (service,
unleash, flagsClient) without sharing external state, and keep extractSessionId
spy creation inside setup so setup remains the solitary factory for the test
scenario.
In `@apps/deploy-web/src/utils/walletUtils.spec.ts`:
- Around line 1103-1110: The setup function must accept a single inline-typed
parameter; update the existing setup definition (function setup()) to take one
inline-typed parameter (for example, options: { /* props used by tests */ }) and
use that parameter inside the function when configuring storage and
mockLocalStorage (referencing setup, storage, mockLocalStorage, mock<Storage>,
getItem, setItem) without adding an explicit return type; ensure the created
object-under-test is returned and no shared state is used outside the function.
---
Duplicate comments:
In
`@apps/deploy-web/src/components/alerts/AlertsListContainer/AlertsListContainer.spec.tsx`:
- Around line 79-90: Remove the `as any` cast by typing the mock `requestFn`
with the proper request function signature and passing it directly to
`createAPIClient`; e.g. import the `RequestFn` (and `RequestFnResponse` if
needed) type, change the mock declaration to `const requestFn =
vi.fn<RequestFn>(() => Promise.resolve({ data: mockData }) as
Promise<RequestFnResponse<typeof mockData, unknown>>)`, and then pass
`requestFn` (no cast) into `createAPIClient({ requestFn, baseUrl: "" })` so
types line up for `requestFn` and `createAPIClient`.
In
`@apps/deploy-web/src/components/alerts/NotificationChannelsGuard/NotificationChannelsGuard.spec.tsx`:
- Around line 2-7: Replace uses of vi.waitFor in the NotificationChannelsGuard
test with React Testing Library's waitFor (and prefer findBy* queries where
appropriate) to avoid act() gaps: import waitFor from "@testing-library/react"
at top of NotificationChannelsGuard.spec.tsx, change any vi.waitFor(...) calls
that poll for DOM changes to waitFor(() => { expect(...).toBe... }) or use await
screen.findByText/findByRole when awaiting elements, and keep test assertions
referring to the NotificationChannelsGuardView render results unchanged.
In `@apps/deploy-web/src/components/auth/SignUpForm/SignUpForm.spec.tsx`:
- Around line 61-64: The test's mock for onSubmit uses Jest-style generics
(vi.fn<void, [SignUpFormValues]>()) which Vitest doesn't accept; update the mock
in setup to use a single generic function type like a function signature (e.g.,
(values: SignUpFormValues) => void) so onSubmit: input?.onSubmit ??
vi.fn<(values: SignUpFormValues) => void>(); — change the vi.fn generic
accordingly in the setup helper referencing SignUpForm, setup, onSubmit, and
SignUpFormValues.
In
`@apps/deploy-web/src/components/billing-usage/PaymentPopup/PaymentPopup.spec.tsx`:
- Line 872: The mock declaration mockZodResolver = vi.fn((schema: any) =>
schema) uses type any; change the parameter type to a stricter type (e.g.,
unknown or the actual Zod schema type) so typings are preserved — update the
mockZodResolver signature to vi.fn((schema: unknown) => schema) or the
appropriate Zod schema type and adjust any downstream assertions to match the
new type for PaymentPopup.spec.tsx.
- Around line 509-517: The test mock for mockUse3DSecure currently types the
config parameter as any; update it to use the real 3DS options type (or a small
local interface) instead of any so typings are strict. Replace the signature of
mockUse3DSecure((config: any) => ...) with the correct type for config (matching
the production hook/options used by use3DSecure) so threeDSecureSuccessCallback
= config.onSuccess is correctly typed, and keep the returned { start3DSecure:
vi.fn() } shape unchanged to satisfy consumers that call start3DSecure.
In
`@apps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.spec.tsx`:
- Around line 216-219: The union type for SdlBuilder in the test mock uses
ForwardRefExoticComponent<any>; replace the any with a concrete minimal props
interface: define an interface (e.g. SdlBuilderProps) that includes the props
your test/component expects (even empty or with a couple of required props used
in tests), and change the type to Mock |
ForwardRefExoticComponent<SdlBuilderProps & RefAttributes<unknown>> (or
RefAttributes<HTMLDivElement> if a DOM ref is used). Update the SdlBuilder
declaration to reference SdlBuilderProps and import RefAttributes from React so
the mock has strict types and no use of any.
- Line 312: Replace the unsafe cast "(input?.selectedTemplate as any)" in
ManifestEdit.spec.tsx by introducing a proper TypeScript alias for the expected
template shape and using that instead; e.g., import or reference the actual
template interface used by ManifestEdit (the expected type name used in
production code), declare a test alias like "type PartialTemplate =
Partial<TheExpectedTemplateType>" in the spec, and change the cast to
"(input?.selectedTemplate as PartialTemplate)" or construct a fully-typed test
object of that alias; update any mock/test data to conform to the new type so
selectedTemplate is no longer cast to any.
In
`@apps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.spec.tsx`:
- Around line 120-160: The test helper setup currently types its parameter as {
user?: any; onComplete?: Mock } — replace the any by the real user type used in
the app (e.g., User or Partial<User>) by importing that type and updating the
parameter to something like { user?: Partial<User>; onComplete?: Mock } and
adjust the mockUseCustomUser.mockReturnValue so its user property conforms to
that type; update any local mock objects (mockUser shape) to satisfy the actual
type instead of using any to keep type-safety in the setup function and
downstream mocks (setup, mockUseCustomUser, and the mock user literal).
In `@apps/deploy-web/src/components/sdl/SDLEditor/SDLEditor.spec.tsx`:
- Around line 102-117: The test "schedules validation when content changes"
enables fake timers with vi.useFakeTimers() but never restores real timers;
update the test in SDLEditor.spec.tsx (the it block named "schedules validation
when content changes") to restore timers after the assertions by calling
vi.useRealTimers() (or vi.restoreAllMocks/vi.resetAllMocks as appropriate) —
ensure the restoration happens even if the test fails (e.g., place
vi.useRealTimers() in a finally block or after await assertions) so subsequent
tests are not affected.
In `@apps/deploy-web/src/hooks/useThrottledEffect/useThrottledEffect.spec.tsx`:
- Around line 7-15: Replace the beforeEach/afterEach timer setup with a local
setup helper: remove vi.useFakeTimers() and
vi.runOnlyPendingTimers()/vi.useRealTimers() calls from beforeEach/afterEach and
create a setup function (e.g., function setup() { vi.useFakeTimers(); return ()
=> { vi.runOnlyPendingTimers(); vi.useRealTimers(); }; }) at the bottom of the
top-level describe for useThrottledEffect.name; call setup() at the start of
each test and invoke the returned cleanup function in that test's teardown to
preserve the same timer initialization and cleanup behavior associated with
vi.useFakeTimers and vi.runOnlyPendingTimers/vi.useRealTimers.
In
`@apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts`:
- Around line 315-362: Move the helper createRequest function out of the root
describe block to file scope so that setup remains the final declaration inside
the root describe; update any references to createRequest (used by setup) to
still work at file scope and ensure createRequest's signature and mocks remain
unchanged, leaving the setup function (defineServerSideProps test helper) as the
last declaration in the root describe.
In `@apps/deploy-web/src/queries/useVerifyEmailQuery.spec.tsx`:
- Around line 18-20: Replace the top-level beforeEach that calls
vi.clearAllMocks() with a setup helper placed at the bottom of the root
describe: remove beforeEach, create a setup(params) function (single
inline-typed parameter, no explicit return type) that calls vi.clearAllMocks(),
creates and returns the hook instance under test, and update tests to call
setup() to obtain the hook instead of relying on shared state; ensure setup is
the last declaration in the root describe and does not introduce shared mutable
variables.
In
`@apps/deploy-web/src/utils/getValidInternalReturnToUrl/getValidInternalReturnToUrl.spec.ts`:
- Line 5: Replace the hardcoded root describe string with the function's runtime
name: change describe("getValidInternalReturnToUrl", ...) to
describe(getValidInternalReturnToUrl.name, ...); ensure the test file imports or
references the getValidInternalReturnToUrl symbol so .name is available (update
imports if needed) and run tests to confirm no breakage.
In `@apps/deploy-web/vitest.config.ts`:
- Around line 15-23: The test config sets test.outputFile.junit but never
enables the JUnit reporter, so Vitest won’t emit junit.xml; update the root test
config created via defineConfig to include reporters: ["junit"] (or add "junit"
to existing reporters) alongside the existing test.outputFile setting so the
JUnit reporter is active and will consolidate multi-project results.
---
Nitpick comments:
In
`@apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx`:
- Line 13: Replace the use of screen.getByRole("status") with
screen.queryByRole("status") in the CumulativeSpendingLineChart.spec.tsx
assertion; update the expectation to continue using toBeInTheDocument()
(queryByRole returns null if missing so the existing toBeInTheDocument()
assertion remains valid) and run tests to confirm behavior unchanged — target
the line with screen.getByRole("status") in the test file and swap to
queryByRole as per the deploy-web spec guideline.
In
`@apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx`:
- Line 12: Replace the use of screen.getByRole("status") with
screen.queryByRole("status") in the DailyUsageBarChart.spec.tsx assertion to
follow the deploy-web testing guideline; locate the expectation using getByRole
and change it to queryByRole (keeping the .toBeInTheDocument() assertion),
ensuring the test still imports/uses screen from `@testing-library/react`.
In
`@apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentAlerts.spec.tsx`:
- Line 13: The test's root describe uses a hardcoded string "DeploymentAlerts";
update it to use the component's name property by replacing the literal with
DeploymentAlertsView.name so the suite reads describe(DeploymentAlertsView.name,
() => { ... }); locate the root describe in DeploymentAlerts.spec.tsx and import
or reference the DeploymentAlertsView symbol if not already present, ensuring
the test still runs and linter/imports remain valid.
In
`@apps/deploy-web/src/components/sdl/LogCollectorControl/LogCollectorControl.spec.tsx`:
- Around line 74-80: The vi.waitFor callback is declared async but contains no
awaits; remove the unnecessary async keyword to make the callback a plain
synchronous function in the test in LogCollectorControl.spec.tsx (the lambda
passed to vi.waitFor that reads form.getValues("services.1.env") and asserts
selector?.value). Update the arrow function signature to omit async so the test
is slightly simpler and avoids an unnecessary microtask wrapper.
- Around line 60-62: The vi.waitFor callback is declared async but does not
await anything; remove the unnecessary async keyword from the arrow function
passed to vi.waitFor (i.e., change vi.waitFor(async () => {
expect(form.getValues("services.1.title")).toBe(`new-title-log-collector`); });
to vi.waitFor(() => {
expect(form.getValues("services.1.title")).toBe(`new-title-log-collector`); });
so the test uses a synchronous callback while keeping the await vi.waitFor call
intact.
In `@apps/deploy-web/src/hooks/useTrialDeploymentTimeRemaining.spec.ts`:
- Line 17: The root describe string is hardcoded as "useTrialTimeRemaining" but
should reference the actual hook to stay consistent and support automated
refactors; update the test suite declaration in
useTrialDeploymentTimeRemaining.spec.ts so the top-level describe uses
useTrialDeploymentTimeRemaining.name (referencing the exported hook function
name) instead of the hardcoded string, ensuring the describe matches the
function under test.
In `@apps/deploy-web/src/lib/auth0/setSession/setSession.spec.ts`:
- Line 13: Update the test description string in the it(...) call from "can set
session" to a present simple 3rd person singular form (e.g., "sets session") so
the test name follows the project's guideline; locate the it("can set session",
async () => { ... }) in setSession.spec.ts and replace the description text
accordingly.
In `@apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts`:
- Around line 99-117: Move the standalone setup function into the root describe
block in pageGuards.spec.ts (place it at the bottom of that describe) so it
follows the test suite as per guidelines; locate the setup function (the one
returning mock<AppTypedContext> with getCurrentSession and
services.featureFlagService mocks) and cut/paste it inside the root describe
scope, ensuring any references to types like AppTypedContext,
FeatureFlagService, LoggerService, UrlService, vi and faker remain in scope or
adjust imports if needed.
In `@apps/deploy-web/src/queries/useGrantsQuery.spec.tsx`:
- Line 11: Remove the unused empty import statement in useGrantsQuery.spec.tsx:
delete the line "import {} from \"@testing-library/react\";" so the test file no
longer contains a no-op import; verify there are no other references to symbols
from "@testing-library/react" that would require importing specific helpers
instead.
- Around line 174-182: The test currently casts the mock to any
(chainApiHttpClient = mock<FallbackableHttpClient>({...} as any)), which
violates the "no any" rule; replace this with a properly typed mock by using
mockDeep<FallbackableHttpClient>(...) or
mock<Partial<FallbackableHttpClient>>(...) so the object matches the
FallbackableHttpClient interface without any casts, keeping properties like
isFallbackEnabled and get (mockResolvedValue returning { data: { allowances:
mockData, pagination: { next_key: null, total: mockData.length } } }) typed
correctly; ensure you import mockDeep from the mocking lib (or use Partial type)
and remove the as any cast.
- Around line 200-209: The test currently uses an illegal "as any" cast on the
mocked client; replace it with a properly typed mock by giving
chainApiHttpClient the FallbackableHttpClient type (or a
Partial<FallbackableHttpClient> cast) instead of any. For example, remove "as
any" and declare the mock with an explicit type: const chainApiHttpClient:
FallbackableHttpClient = { isFallbackEnabled: false, get:
vi.fn().mockResolvedValue(...) } or if you only implement some members use "as
Partial<FallbackableHttpClient>" when calling mock<...> so the
mock<FallbackableHttpClient> call and the chainApiHttpClient variable are
correctly typed and no "any" cast remains.
In `@apps/deploy-web/src/queries/useMarketData.spec.tsx`:
- Line 11: The root test suite description currently uses a hardcoded string
"useMarketData"; change it to use the function's name property by replacing that
string with useMarketData.name so the root describe uses useMarketData.name
(locate the describe call in this spec file that references useMarketData and
update it accordingly).
In `@apps/deploy-web/src/services/stripe/stripe.service.spec.ts`:
- Line 8: Replace the hardcoded root describe string with the service's name
property: change the root test suite declaration from describe("StripeService",
...) to describe(StripeService.name, ...) so the suite uses StripeService.name
for automated refactors; update any imports or references necessary to ensure
StripeService is in scope for the test file.
Why
To speed up feedback from unit tests. Tests run in ~30 sec, previously was ~60s
What
Summary by CodeRabbit
Chores
Tests
Style