chore: fix setup-app-deps gha and api test types#1536
Conversation
WalkthroughThis update introduces explicit TypeScript type assertions for JSON-parsed HTTP responses across numerous functional test files and a service in the API app. Additionally, the GitHub Actions workflow for setting up app dependencies is modified to install Node.js dependencies only for a specified app workspace, rather than the entire repository. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant npm
participant App Workspace
GitHub Actions->>npm: npm ci -w apps/${{ inputs.app }}
npm->>App Workspace: Install dependencies for specified app
App Workspace-->>npm: Dependencies installed
npm-->>GitHub Actions: Installation complete
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (25)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (22)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1536 +/- ##
========================================
Coverage 40.57% 40.57%
========================================
Files 872 872
Lines 21217 21217
Branches 3876 3876
========================================
Hits 8608 8608
+ Misses 12030 11892 -138
- Partials 579 717 +138
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
stalniy
force-pushed
the
chore/fix-setup-app-deps
branch
from
a8881b9 to
0860def
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (6)
apps/api/test/functional/bids.spec.ts (1)
31-59: ReplacebeforeEachwithsetupfunction per coding guidelines.According to the coding guidelines, test files should use a
setupfunction instead ofbeforeEachfor test setup. The setup function should be at the bottom of the root describe block, create and return the object under test, accept a single parameter with inline type definition, avoid shared state, and should not specify a return type.Consider refactoring to use a setup function pattern like this:
- beforeEach(() => { - knownUsers = {}; - knownApiKeys = {}; - knownWallets = {}; - // ... existing setup logic - }); + const setup = (options: { dseq?: string } = {}) => { + const knownUsers: Record<string, UserOutput> = {}; + const knownApiKeys: Record<string, ApiKeyOutput> = {}; + const knownWallets: Record<string, UserWalletOutput[]> = {}; + + // ... setup logic here + + return { knownUsers, knownApiKeys, knownWallets }; + };apps/api/test/functional/api-key.spec.ts (1)
58-62: ReplacebeforeEachwithsetupfunction per coding guidelines.According to the coding guidelines, test files should use a
setupfunction instead ofbeforeEachfor test setup. The setup function should be at the bottom of the root describe block, create and return the object under test, accept a single parameter with inline type definition, avoid shared state, and should not specify a return type.Consider refactoring to use a setup function pattern like this:
- beforeEach(async () => { - config = stub<CoreConfigService>({ get: jest.fn() }); - config.get.mockReturnValue("test"); - apiKeyGenerator = new ApiKeyGeneratorService(config); - }); Move this to a setup function at the bottom of the describe block: + const setup = (options: { returnValue?: string } = {}) => { + const config = stub<CoreConfigService>({ get: jest.fn() }); + config.get.mockReturnValue(options.returnValue || "test"); + const apiKeyGenerator = new ApiKeyGeneratorService(config); + + return { config, apiKeyGenerator }; + };apps/api/test/functional/anonymous-user.spec.ts (1)
8-16: ReplacebeforeEachwithsetupfunction to follow coding guidelines.According to the coding guidelines, test files should use a
setupfunction instead ofbeforeEach. The setup function should be at the bottom of the root describe block, create and return the object under test, accept a single parameter with inline type definition, and avoid shared state.Consider refactoring to this pattern:
- beforeEach(async () => { - const userResponse = await app.request("/v1/anonymous-users", { - method: "POST", - headers: new Headers({ "Content-Type": "application/json" }) - }); - const body = (await userResponse.json()) as any; - user = body.data; - token = body.token; - });And add a setup function at the bottom:
+ async function setup() { + const userResponse = await app.request("/v1/anonymous-users", { + method: "POST", + headers: new Headers({ "Content-Type": "application/json" }) + }); + const body = (await userResponse.json()) as any; + return { user: body.data, token: body.token }; + }Then update tests to call
setup()as needed.apps/api/test/functional/deployment-setting.spec.ts (1)
18-24: Violation: Use setup function instead of beforeEach per coding guidelines.The coding guidelines specify that
**/*.spec.{ts,tsx}files should use asetupfunction instead ofbeforeEachfor test setup. Thesetupfunction must be at the bottom of the rootdescribeblock, create and return the object under test, accept a single parameter with inline type definition, avoid shared state, and should not specify a return type.Consider refactoring to use a setup function:
- beforeEach(() => { - jest.spyOn(leaseRepository, "findOneByDseqAndOwner").mockResolvedValue(DrainingDeploymentSeeder.create()); - }); - - afterEach(async () => { - jest.restoreAllMocks(); - }); + function setup() { + jest.spyOn(leaseRepository, "findOneByDseqAndOwner").mockResolvedValue(DrainingDeploymentSeeder.create()); + + return { + // return test objects + }; + }apps/api/test/functional/lease-flow.spec.ts (1)
84-95: Violation: Use setup function instead of beforeEach per coding guidelines.This file violates the coding guidelines that require
**/*.spec.{ts,tsx}files to use asetupfunction instead ofbeforeEachfor test setup.apps/api/test/functional/deployments.spec.ts (1)
45-94: Violation: Use setup function instead of beforeEach per coding guidelines.This file violates the coding guidelines requiring
**/*.spec.{ts,tsx}files to use asetupfunction instead ofbeforeEachfor test setup. Consider refactoring the mock setup logic into a setup function at the bottom of the root describe block.
♻️ Duplicate comments (1)
apps/api/test/functional/market-data.spec.ts (1)
79-81: Duplicate of previous comment – sameanycasting issue.
🧹 Nitpick comments (4)
apps/api/test/functional/start-trial.spec.ts (1)
29-32: Replaceanywith a typed interface to preserve type-safetyCasting the JSON payload to
anyerodes the very benefit of having TypeScript in tests.
A lightweight local interface keeps the code self-documenting while retaining compile-time checks:- } = (await userResponse.json()) as any; + } = (await userResponse.json()) as { + data: { id: string }; + token: string; + };No further changes are required elsewhere because the destructured variables already match the interface shape.
apps/api/test/functional/market-data.spec.ts (1)
46-48: Prefer a typed response overanyThe response schema is fully known and stable in this test.
Introduce a minimal interface instead of falling back toany:- const data = (await response.json()) as any; + const data = (await response.json()) as { + price: number; + volume: number; + marketCap: number; + marketCapRank: number; + priceChange24h: number; + priceChangePercentage24: number; + };This shields future refactors from silent breakage.
apps/api/test/functional/sign-and-broadcast-tx.spec.ts (1)
47-48: Inline type instead ofanyfor better IDE support- const { token } = (await differentUserResponse.json()) as any; + const { token } = (await differentUserResponse.json()) as { token: string };A one-liner type keeps the intent explicit without polluting the file with extra declarations.
apps/api/test/functional/provider-attributes-schema.spec.ts (1)
6-9: Replaceanywith a structural typeEven a broad structural type is better than
anyhere:- const data = (await response.json()) as any; + const data = (await response.json()) as Record< + string, + { key: string; type: string; required: boolean; description: string; values?: unknown[] } + >;This ensures the subsequent property assertions stay aligned with the expected schema.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
.github/actions/setup-app-deps/action.yml(1 hunks)apps/api/test/functional/anonymous-user.spec.ts(2 hunks)apps/api/test/functional/api-key.spec.ts(4 hunks)apps/api/test/functional/auditors.spec.ts(1 hunks)apps/api/test/functional/balances.spec.ts(2 hunks)apps/api/test/functional/bids.spec.ts(3 hunks)apps/api/test/functional/blocks.spec.ts(2 hunks)apps/api/test/functional/dashboard-data.spec.ts(1 hunks)apps/api/test/functional/deployment-setting.spec.ts(3 hunks)apps/api/test/functional/deployments.spec.ts(10 hunks)apps/api/test/functional/graph-data.spec.ts(1 hunks)apps/api/test/functional/lease-flow.spec.ts(10 hunks)apps/api/test/functional/leases-duration.spec.ts(1 hunks)apps/api/test/functional/market-data.spec.ts(2 hunks)apps/api/test/functional/provider-attributes-schema.spec.ts(1 hunks)apps/api/test/functional/provider-dashboard.spec.ts(2 hunks)apps/api/test/functional/provider-deployments.spec.ts(3 hunks)apps/api/test/functional/provider-graph-data.spec.ts(2 hunks)apps/api/test/functional/provider-regions.spec.ts(1 hunks)apps/api/test/functional/providers.spec.ts(4 hunks)apps/api/test/functional/sign-and-broadcast-tx.spec.ts(1 hunks)apps/api/test/functional/start-trial.spec.ts(1 hunks)apps/api/test/functional/transactions.spec.ts(2 hunks)apps/api/test/functional/usage.spec.ts(3 hunks)apps/api/test/services/wallet-testing.service.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.spec.{ts,tsx}`: Use a `setup` function instead of `beforeEach` for test setup. The `setup` function must be at the bottom of the root `describe` block, create and return the ...
**/*.spec.{ts,tsx}: Use asetupfunction instead ofbeforeEachfor test setup. Thesetupfunction must be at the bottom of the rootdescribeblock, create and return the object under test, accept a single parameter with inline type definition, avoid shared state, and should not specify a return type.
apps/api/test/functional/start-trial.spec.tsapps/api/test/functional/sign-and-broadcast-tx.spec.tsapps/api/test/functional/provider-regions.spec.tsapps/api/test/functional/provider-attributes-schema.spec.tsapps/api/test/functional/market-data.spec.tsapps/api/test/functional/provider-dashboard.spec.tsapps/api/test/functional/provider-deployments.spec.tsapps/api/test/functional/blocks.spec.tsapps/api/test/functional/dashboard-data.spec.tsapps/api/test/functional/auditors.spec.tsapps/api/test/functional/balances.spec.tsapps/api/test/functional/bids.spec.tsapps/api/test/functional/api-key.spec.tsapps/api/test/functional/transactions.spec.tsapps/api/test/functional/providers.spec.tsapps/api/test/functional/lease-flow.spec.tsapps/api/test/functional/graph-data.spec.tsapps/api/test/functional/leases-duration.spec.tsapps/api/test/functional/provider-graph-data.spec.tsapps/api/test/functional/deployment-setting.spec.tsapps/api/test/functional/anonymous-user.spec.tsapps/api/test/functional/usage.spec.tsapps/api/test/functional/deployments.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: validate-api
- GitHub Check: test-api-build
🔇 Additional comments (25)
apps/api/test/functional/transactions.spec.ts (1)
42-42: LGTM! Type assertions improve TypeScript safety.The explicit type assertions
as anyfor JSON responses appropriately handle the API response typing while maintaining test flexibility.Also applies to: 64-64
apps/api/test/functional/leases-duration.spec.ts (1)
138-138: LGTM! Type assertion enhances type safety in helper function.The explicit
as anytype assertion on the JSON response in theexpectLeasesDurationhelper function appropriately handles API response typing.apps/api/test/functional/blocks.spec.ts (1)
42-42: LGTM! Consistent type assertions improve code clarity.The explicit
as anytype assertions for JSON responses align with the broader test suite typing improvements and enhance TypeScript safety.Also applies to: 64-64
apps/api/test/functional/graph-data.spec.ts (1)
189-189: LGTM! Type assertion improves code explicitness.The
as anytype assertion on the JSON response appropriately handles the dynamic nature of different graph data types in the test loop.apps/api/test/functional/auditors.spec.ts (1)
9-9: LGTM! Excellent use of specific type assertion.The explicit type assertion using
AuditorListResponseprovides better type safety than genericanywhile maintaining consistency with the broader test suite typing improvements.apps/api/test/functional/bids.spec.ts (1)
128-128: LGTM! Type assertions improve type safety.The explicit type assertions
(await response.json()) as anyenhance type clarity and safety in the test code while maintaining the same functionality.Also applies to: 147-147, 167-167
apps/api/test/functional/dashboard-data.spec.ts (1)
208-208: LGTM! Type assertion improves type safety.The explicit type assertion
(await response.json()) as anyenhances type clarity in the test code.apps/api/test/functional/provider-dashboard.spec.ts (1)
40-40: LGTM! Type assertions improve type safety.The explicit type assertions
(await response.json()) as anyenhance type clarity and safety in both the success and error response handling.Also applies to: 54-54
apps/api/test/functional/api-key.spec.ts (1)
149-149: LGTM! Type assertions improve type safety.The explicit type assertions
(await response.json()) as anyenhance type clarity and safety across all API key test operations.Also applies to: 196-196, 243-243, 356-356
apps/api/test/functional/provider-deployments.spec.ts (1)
140-140: LGTM! Type assertions improve type safety.The explicit type assertions
(await response.json()) as anyenhance type clarity and safety across all provider deployment test scenarios.Also applies to: 150-150, 160-160
apps/api/test/functional/providers.spec.ts (1)
106-106: Consistent type assertion pattern applied correctly.The explicit
as anytype assertions improve type safety and align with the repository-wide standardization of JSON response handling in tests.Also applies to: 115-115, 124-124, 135-135
apps/api/test/services/wallet-testing.service.ts (1)
15-15: Type assertions improve consistency across service methods.The
as anyassertions align with the repository-wide pattern for JSON response handling and provide appropriate flexibility for varied API response structures.Also applies to: 25-25, 34-34
apps/api/test/functional/balances.spec.ts (2)
47-47: Type assertions follow established pattern.The
as anyassertions are consistent with the repository-wide standardization effort for JSON response handling.Also applies to: 76-76
106-224: Setup function follows coding guidelines correctly.The
setupfunction is properly positioned at the bottom of the describe block, accepts a parameter with inline type definition, creates and returns test objects, and avoids shared state.apps/api/test/functional/anonymous-user.spec.ts (1)
13-13: Type assertions follow established pattern.The
as anyassertions are consistent with the repository-wide standardization effort.Also applies to: 55-55
apps/api/test/functional/provider-graph-data.spec.ts (1)
209-209: Type assertions maintain consistency across test suite.The
as anyassertions follow the established pattern for JSON response handling and improve type safety without affecting test logic.Also applies to: 284-284
apps/api/test/functional/deployment-setting.spec.ts (1)
181-181: LGTM! Type assertions improve type safety.The explicit type assertions for JSON responses enhance type safety and clearly document the expected response structure.
Also applies to: 234-234, 305-305
apps/api/test/functional/lease-flow.spec.ts (1)
107-107: LGTM! Type assertions enhance code clarity and safety.The explicit type assertions for JSON responses improve type safety. Good use of specific types like
BidResponse[]where available, andanyas fallback for complex response structures.Also applies to: 130-130, 141-141, 155-155, 166-166, 202-202, 222-222, 242-242, 251-251, 263-263
apps/api/test/functional/usage.spec.ts (3)
4-4: LGTM! Added proper type import.Adding the
UsageHistoryStatsimport enables proper type assertions throughout the file.
280-280: LGTM! Improved type safety with specific type assertions.Using specific imported types (
UsageHistoryResponse,UsageHistoryStats) instead of generic casting improves type safety and code clarity.Also applies to: 417-417
412-433: Partial compliance: setup function used correctly here.This describe block correctly implements the setup function pattern as required by coding guidelines. The setup function is defined within the describe block and returns the necessary test utilities.
.github/actions/setup-app-deps/action.yml (1)
30-30: LGTM! Performance optimization for workspace-specific dependency installation.Scoping
npm cito the specific app workspace (-w apps/${{ inputs.app }}) is a good optimization that installs only the necessary dependencies for the target app, improving build performance and reducing unnecessary installations.apps/api/test/functional/deployments.spec.ts (3)
218-218: LGTM! Type assertions improve response handling.The explicit type assertions for JSON responses enhance type safety and document expected response structures. Good use of specific types like
{ data: unknown }and{ owner: string; dseq: string }.Also applies to: 274-274, 413-413, 685-685, 850-850
313-313: Good type safety with specific array typing.The type assertion
{ data: { deployments: unknown[] } }properly types the deployments array structure, enabling safe access to array properties in the test assertions.Also applies to: 315-321
457-457: Consistent error response typing.Using
{ message: string }type assertion for error responses provides consistent typing for error message validation.Also applies to: 476-476, 765-765
stalniy
force-pushed
the
chore/fix-setup-app-deps
branch
from
0860def to
a557a0f
Compare
2 participants
Summary by CodeRabbit