test: fix connect plugin tests #1441
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixed Issues
- Incomplete nginx configuration in test mocks: The service processes 5 URL types (defaultUrl, lanIp, lanIp6, lanName, lanMdns) plus FQDN entries. Tests were providing partial configurations, causing errors when the service tried to process missing fields.
- IPv6 URL parsing issue: Used hostname ipv6.unraid.local instead of raw IPv6 address 2001:db8::1 which fails URL parsing (IPv6 addresses need square brackets in URLs).
- Missing getOrThrow mocking: Added proper mocking for configService.getOrThrow('connect.config.wanport') calls when processing FQDN URLs.
- Incorrect WAN URL assumptions: The service doesn't process wanIp directly - WAN URLs only come from FQDN entries. Updated test to use FQDN entries with interface type 'WAN'.
- Wrong property assertions: Fixed IPv6 URL assertion to check ipv6 property instead of ipv4.
Key Changes
- Provided complete nginx configurations in all test mocks
- Made error assertions more flexible - checking for specific error messages rather than exact counts
- Used realistic test data that matches what the service expects
- Focused on behavior over implementation - tests verify correct URLs are generated without encoding implementation details
WalkthroughThe changes refactor legacy configuration migration for both API and Connect plugins, modularizing parsing, conversion, and persistence logic. The Changes
Sequence Diagram(s)sequenceDiagram
participant LegacyConfigFile
participant ConnectConfigPersister
participant ConfigService
LegacyConfigFile->>ConnectConfigPersister: readLegacyConfig(filePath)
ConnectConfigPersister->>ConnectConfigPersister: parseLegacyConfig(iniContent)
ConnectConfigPersister->>ConnectConfigPersister: convertLegacyConfig(parsedConfig)
ConnectConfigPersister->>ConfigService: set(newConfig)
sequenceDiagram
participant OldConfig
participant ApiConfigPersistence
participant ConfigService
OldConfig->>ApiConfigPersistence: convertLegacyConfig(config)
ApiConfigPersistence->>ConfigService: set('api', migratedConfig)
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)`api/**/*`: Use pnpm ONLY for package management. Always run scripts from api/pa...
📄 Source: CodeRabbit Inference Engine (CLAUDE.md) List of files the instruction was applied to:
🧠 Learnings (2)📓 Common learningsapi/src/__test__/config/api-config.test.ts (10)⏰ Context from checks skipped due to timeout of 90000ms (6)
🔇 Additional comments (6)
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/main.yml (1)
121-151: Excellent concurrent test execution implementation.The parallel execution of API, Connect plugin, and Shared package tests with proper logging and exit code handling significantly improves CI performance while maintaining visibility into individual test results.
However, fix the trailing spaces:
- set -e - + set -e + - echo "🚀 Starting API coverage tests..." + echo "🚀 Starting API coverage tests..." - pnpm run coverage > api-test.log 2>&1 & + pnpm run coverage > api-test.log 2>&1 & - API_PID=$! - + API_PID=$! + - echo "🚀 Starting Connect plugin tests..." + echo "🚀 Starting Connect plugin tests..." - (cd ../packages/unraid-api-plugin-connect && pnpm test) > connect-test.log 2>&1 & + (cd ../packages/unraid-api-plugin-connect && pnpm test) > connect-test.log 2>&1 & - CONNECT_PID=$! - + CONNECT_PID=$! + - echo "🚀 Starting Shared package tests..." + echo "🚀 Starting Shared package tests..." - (cd ../unraid-shared && pnpm test) > shared-test.log 2>&1 & + (cd ../unraid-shared && pnpm test) > shared-test.log 2>&1 & - SHARED_PID=$! - + SHARED_PID=$! + - # Wait for all processes and capture exit codes + # Wait for all processes and capture exit codes - wait $API_PID && echo "✅ API tests completed" || { echo "❌ API tests failed"; API_EXIT=1; } + wait $API_PID && echo "✅ API tests completed" || { echo "❌ API tests failed"; API_EXIT=1; } - wait $CONNECT_PID && echo "✅ Connect tests completed" || { echo "❌ Connect tests failed"; CONNECT_EXIT=1; } + wait $CONNECT_PID && echo "✅ Connect tests completed" || { echo "❌ Connect tests failed"; CONNECT_EXIT=1; } - wait $SHARED_PID && echo "✅ Shared tests completed" || { echo "❌ Shared tests failed"; SHARED_EXIT=1; } - + wait $SHARED_PID && echo "✅ Shared tests completed" || { echo "❌ Shared tests failed"; SHARED_EXIT=1; } + - # Display all outputs + # Display all outputs - echo "📋 API Test Results:" && cat api-test.log + echo "📋 API Test Results:" && cat api-test.log - echo "📋 Connect Plugin Test Results:" && cat connect-test.log + echo "📋 Connect Plugin Test Results:" && cat connect-test.log - echo "📋 Shared Package Test Results:" && cat shared-test.log - + echo "📋 Shared Package Test Results:" && cat shared-test.log + - # Exit with error if any test failed + # Exit with error if any test failedpackages/unraid-api-plugin-connect/src/test/config.persistence.test.ts (1)
208-228: Use anonymized test data instead of potentially real credentials.The test includes what appears to be a real email address (
pujitm2009@gmail.com) and potentially real API keys. Consider using anonymized test data to avoid exposing personal information.- apikey: 'unraid_sfHboeSNzTzx24816QBssqi0A3nIT0f4Xg4c9Ht49WQfQKLMojU81Sb3f', - localApiKey: '101d204832d24fc7e5d387f6fce47067ba230f8aa0ac3bcc6c12a415aa27dbd9', - email: 'pujitm2009@gmail.com', - username: 'pujitm2009@gmail.com', + apikey: 'unraid_test_api_key_example_12345', + localApiKey: 'test_local_api_key_example_67890', + email: 'testuser@example.com', + username: 'testuser@example.com',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.github/workflows/main.yml(2 hunks)api/src/unraid-api/config/api-config.module.ts(4 hunks)api/src/unraid-api/config/api-config.test.ts(1 hunks)packages/unraid-api-plugin-connect/package.json(2 hunks)packages/unraid-api-plugin-connect/src/model/connect-config.model.ts(0 hunks)packages/unraid-api-plugin-connect/src/service/config.persistence.ts(3 hunks)packages/unraid-api-plugin-connect/src/service/connect-config.service.ts(1 hunks)packages/unraid-api-plugin-connect/src/service/url-resolver.service.ts(1 hunks)packages/unraid-api-plugin-connect/src/test/cloud.service.test.ts(1 hunks)packages/unraid-api-plugin-connect/src/test/config.persistence.test.ts(1 hunks)packages/unraid-api-plugin-connect/src/test/url-resolver.service.test.ts(10 hunks)
💤 Files with no reviewable changes (1)
- packages/unraid-api-plugin-connect/src/model/connect-config.model.ts
🧰 Additional context used
📓 Path-based instructions (3)
`api/**/*`: Use pnpm ONLY for package management. Always run scripts from api/pa...
api/**/*: Use pnpm ONLY for package management.
Always run scripts from api/package.json unless requested.
Test suite is VITEST, do not use jest.
Run tests with: pnpm --filter ./api test.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
api/src/unraid-api/config/api-config.test.tsapi/src/unraid-api/config/api-config.module.ts
`api/src/unraid-api/**/*`: Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code.
api/src/unraid-api/**/*: Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
api/src/unraid-api/config/api-config.test.tsapi/src/unraid-api/config/api-config.module.ts
`api/src/unraid-api/**/*`: Prefer adding new files to the nest repo located at api/src/unraid-api/ instead of the legacy code.
api/src/unraid-api/**/*: Prefer adding new files to the nest repo located at api/src/unraid-api/ instead of the legacy code.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
List of files the instruction was applied to:
api/src/unraid-api/config/api-config.test.tsapi/src/unraid-api/config/api-config.module.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: elibosley
PR: unraid/api#1425
File: api/dev/configs/connect.json:5-13
Timestamp: 2025-06-20T20:50:14.050Z
Learning: Connect configuration files like api/dev/configs/connect.json use their own naming conventions (e.g., apikey, accesstoken, idtoken, refreshtoken) as implementation details, and these should not be changed to follow general camelCase patterns.
Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-generator/src/create-plugin.ts:91-112
Timestamp: 2025-04-21T18:27:36.482Z
Learning: For utility functions like file operations in api plugin generators, pujitm prefers minimal error handling rather than verbose try/catch blocks for each operation, as errors will propagate properly to higher level error handlers.
packages/unraid-api-plugin-connect/package.json (10)
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:13.145Z
Learning: The test suite for the project is VITEST; do not use jest for testing.
Learnt from: pujitm
PR: unraid/api#1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.
Learnt from: mdatelle
PR: unraid/api#1122
File: web/package.json:16-16
Timestamp: 2025-02-06T17:25:45.397Z
Learning: The build script in web/package.json should include type-check command (`npm run type-check`) even when NODE_ENV is production, as it provides value for local builds by catching type errors before pushing to CI/CD.
Learnt from: elibosley
PR: unraid/api#1120
File: plugin/package.json:1-8
Timestamp: 2025-02-05T14:43:25.062Z
Learning: The repository uses Renovate for automated dependency updates, making strict version pinning in package.json less critical as updates are handled automatically through PRs.
Learnt from: mdatelle
PR: unraid/api#1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Learnt from: elibosley
PR: unraid/api#1120
File: plugin/package.json:0-0
Timestamp: 2025-02-05T14:43:48.568Z
Learning: In Node.js projects, npm scripts should be organized with clear namespacing (e.g., build:*, docker:*, env:*) and include proper environment validation and error handling. Each script should follow the single responsibility principle.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:38:09.623Z
Learning: When testing Vue components in the Nuxt.js app (web/), use vitest outside of the Nuxt environment, and use mount from Vue Test Utils for component testing.
Learnt from: elibosley
PR: unraid/api#1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:38:09.623Z
Learning: In the Unraid API monorepo, TypeScript imports should use .js extensions for ESM compatibility.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:32.437Z
Learning: Do not rely on Nuxt's auto-imports in test files; explicitly import Vue reactivity utilities (`computed`, `ref`, `watchEffect`) in store files to avoid missing dependencies.
packages/unraid-api-plugin-connect/src/service/url-resolver.service.ts (3)
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:62-70
Timestamp: 2024-11-05T14:49:07.308Z
Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/types/fastify.ts:25-30
Timestamp: 2025-03-10T18:43:13.016Z
Learning: In the Unraid API, cookies should be marked as optional (with `?`) in the FastifyRequest type definition since the cookies may not always be present, which is consistent with the authentication flow throughout the codebase.
packages/unraid-api-plugin-connect/src/service/connect-config.service.ts (4)
Learnt from: pujitm
PR: unraid/api#1211
File: web/codegen.ts:14-14
Timestamp: 2025-03-12T13:35:43.900Z
Learning: The JSON scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibility with JsonForms integration. This change facilitates the implementation of the Connect settings web component.
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:44:46.432Z
Learning: When modifying `apiKey.roles` in `removeRoleFromApiKey` and `addRoleToApiKey` within `api/src/unraid-api/auth/auth.service.ts`, concurrency issues are not a concern because the keys are stored in the file system.
Learnt from: elibosley
PR: unraid/api#1063
File: web/components/SsoButton.ce.vue:5-8
Timestamp: 2025-01-27T14:31:42.305Z
Learning: In the Unraid API web components, SSO-related props are intentionally provided in both camelCase (`ssoEnabled`) and lowercase (`ssoenabled`) variants to support interchangeable usage across different contexts (e.g., HTML attributes vs Vue props).
Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the @UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
packages/unraid-api-plugin-connect/src/test/cloud.service.test.ts (8)
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:62-70
Timestamp: 2024-11-05T14:49:07.308Z
Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:44:46.432Z
Learning: When modifying `apiKey.roles` in `removeRoleFromApiKey` and `addRoleToApiKey` within `api/src/unraid-api/auth/auth.service.ts`, concurrency issues are not a concern because the keys are stored in the file system.
Learnt from: elibosley
PR: unraid/api#1063
File: web/_data/serverState.ts:137-147
Timestamp: 2025-01-27T14:57:46.617Z
Learning: The values in `web/_data/serverState.ts` are used for testing purposes and should remain as hardcoded mock data to facilitate testing different scenarios.
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:176-188
Timestamp: 2024-11-15T16:22:03.485Z
Learning: Atomic writes are not required for the `saveApiKey` method in `ApiKeyService` (`api/src/unraid-api/auth/api-key.service.ts`) unless specifically needed.
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/types/fastify.ts:25-30
Timestamp: 2025-03-10T18:43:13.016Z
Learning: In the Unraid API, cookies should be marked as optional (with `?`) in the FastifyRequest type definition since the cookies may not always be present, which is consistent with the authentication flow throughout the codebase.
Learnt from: pujitm
PR: unraid/api#1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.
Learnt from: pujitm
PR: unraid/api#1047
File: api/src/unraid-api/graph/sandbox-plugin.ts:57-57
Timestamp: 2025-01-15T21:34:00.006Z
Learning: In the GraphQL sandbox (api/src/unraid-api/graph/sandbox-plugin.ts), CSRF token validation should fail silently with a fallback value to maintain sandbox accessibility, as it's a development tool where strict security measures aren't required.
.github/workflows/main.yml (2)
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:13.145Z
Learning: Use pnpm as the package manager for all operations in the codebase.
Learnt from: elibosley
PR: unraid/api#1120
File: plugin/package.json:0-0
Timestamp: 2025-02-05T14:43:48.568Z
Learning: In Node.js projects, npm scripts should be organized with clear namespacing (e.g., build:*, docker:*, env:*) and include proper environment validation and error handling. Each script should follow the single responsibility principle.
packages/unraid-api-plugin-connect/src/test/config.persistence.test.ts (6)
Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.persistence.ts:143-149
Timestamp: 2025-04-22T14:41:19.603Z
Learning: The `demo` property in the config.persistence.ts file's migrateLegacyConfig method is used for demonstration purposes, adding a current timestamp to keep demo environments looking fresh.
Learnt from: elibosley
PR: unraid/api#1063
File: web/_data/serverState.ts:137-147
Timestamp: 2025-01-27T14:57:46.617Z
Learning: The values in `web/_data/serverState.ts` are used for testing purposes and should remain as hardcoded mock data to facilitate testing different scenarios.
Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the @UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:13.145Z
Learning: The test suite for the project is VITEST; do not use jest for testing.
Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom ConfigService implementation that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
api/src/unraid-api/config/api-config.test.ts (9)
Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.persistence.ts:143-149
Timestamp: 2025-04-22T14:41:19.603Z
Learning: The `demo` property in the config.persistence.ts file's migrateLegacyConfig method is used for demonstration purposes, adding a current timestamp to keep demo environments looking fresh.
Learnt from: elibosley
PR: unraid/api#1063
File: web/_data/serverState.ts:137-147
Timestamp: 2025-01-27T14:57:46.617Z
Learning: The values in `web/_data/serverState.ts` are used for testing purposes and should remain as hardcoded mock data to facilitate testing different scenarios.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:13.145Z
Learning: The test suite for the project is VITEST; do not use jest for testing.
Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:38:09.623Z
Learning: Stub complex child components that aren't the focus of the test, and mock external dependencies and services in Vue component tests.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:38:09.623Z
Learning: When testing Vue components in the Nuxt.js app (web/), use vitest outside of the Nuxt environment, and use mount from Vue Test Utils for component testing.
Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom ConfigService implementation that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:32.437Z
Learning: All mock declarations should be placed at the top level of the test file, and module mocks should use factory functions to avoid hoisting issues with vitest.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:32.437Z
Learning: When mocking modules with vitest, always use a factory function to avoid hoisting issues and ensure mocks are correctly scoped.
packages/unraid-api-plugin-connect/src/test/url-resolver.service.test.ts (13)
Learnt from: elibosley
PR: unraid/api#1063
File: web/_data/serverState.ts:137-147
Timestamp: 2025-01-27T14:57:46.617Z
Learning: The values in `web/_data/serverState.ts` are used for testing purposes and should remain as hardcoded mock data to facilitate testing different scenarios.
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:32.437Z
Learning: Always reset mocks between tests using `vi.clearAllMocks()` to ensure test isolation and prevent state leakage.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:32.437Z
Learning: When mocking modules with vitest, always use a factory function to avoid hoisting issues and ensure mocks are correctly scoped.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:38:09.623Z
Learning: Stub complex child components that aren't the focus of the test, and mock external dependencies and services in Vue component tests.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:32.437Z
Learning: All mock declarations should be placed at the top level of the test file, and module mocks should use factory functions to avoid hoisting issues with vitest.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:38:09.623Z
Learning: Place all mock declarations at the top level of test files, use factory functions for module mocks to avoid hoisting issues, and clear mocks between tests to ensure isolation.
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:87-89
Timestamp: 2024-11-06T20:59:06.096Z
Learning: Error handling in mutations like `addRoleForUser` in `auth.resolver.ts` is performed at the service level, so additional error handling in the resolver is unnecessary.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:32.437Z
Learning: Do not mock the store under test in the test file; instead, let it initialize naturally to ensure realistic behavior.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:32.437Z
Learning: For mocking Pinia stores in Vue component tests, use `createTestingPinia()` to provide a mock store instance, allowing direct manipulation of state and automatic stubbing of actions.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:38:09.623Z
Learning: Let Pinia stores initialize with their natural default state and do not mock the store being tested.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:38:09.623Z
Learning: Use createTestingPinia() for mocking stores in Vue component tests, and prefer semantic queries (e.g., find('button')) over data-test IDs.
Learnt from: elibosley
PR: unraid/api#1381
File: plugin/builder/cli/common-environment.ts:18-22
Timestamp: 2025-05-08T19:28:37.034Z
Learning: In plugin/builder/cli/common-environment.ts, the getDefaultBaseUrl() function intentionally returns an invalid URL when running in CI environments to force explicit specification of the base URL parameter rather than relying on defaults.
api/src/unraid-api/config/api-config.module.ts (14)
Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.persistence.ts:143-149
Timestamp: 2025-04-22T14:41:19.603Z
Learning: The `demo` property in the config.persistence.ts file's migrateLegacyConfig method is used for demonstration purposes, adding a current timestamp to keep demo environments looking fresh.
Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:44:46.432Z
Learning: When modifying `apiKey.roles` in `removeRoleFromApiKey` and `addRoleToApiKey` within `api/src/unraid-api/auth/auth.service.ts`, concurrency issues are not a concern because the keys are stored in the file system.
Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the @UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:38:09.623Z
Learning: In the Unraid API monorepo, TypeScript imports should use .js extensions for ESM compatibility.
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:41:22.303Z
Learning: In `api/src/unraid-api/auth/auth.service.ts`, the `addRoleToApiKey` function operates on API keys stored as JSON files in a directory, not a database, so concurrency is not a concern for modifying `apiKey.roles`.
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:176-188
Timestamp: 2024-11-15T16:22:03.485Z
Learning: Atomic writes are not required for the `saveApiKey` method in `ApiKeyService` (`api/src/unraid-api/auth/api-key.service.ts`) unless specifically needed.
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:42-105
Timestamp: 2024-11-04T22:00:41.946Z
Learning: In the 'AuthService' within `api/src/unraid-api/auth/auth.service.ts`, rate limiting is not required because the API is not public facing.
Learnt from: mdatelle
PR: unraid/api#1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Learnt from: pujitm
PR: unraid/api#1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
Learnt from: elibosley
PR: unraid/api#1181
File: web/store/theme.ts:0-0
Timestamp: 2025-02-24T14:51:21.328Z
Learning: In the Unraid API project's theme system, exact TypeScript type definitions are preferred over index signatures for theme variables to ensure better type safety.
Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom ConfigService implementation that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
packages/unraid-api-plugin-connect/src/service/config.persistence.ts (9)
Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the @UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.persistence.ts:143-149
Timestamp: 2025-04-22T14:41:19.603Z
Learning: The `demo` property in the config.persistence.ts file's migrateLegacyConfig method is used for demonstration purposes, adding a current timestamp to keep demo environments looking fresh.
Learnt from: mdatelle
PR: unraid/api#1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Learnt from: elibosley
PR: unraid/api#1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh:11-16
Timestamp: 2025-05-08T19:31:52.417Z
Learning: The `dynamix.my.servers` namespace is still valid and should not be changed to `dynamix.unraid.net` in file paths, as both namespaces coexist in the codebase.
Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
Learnt from: pujitm
PR: unraid/api#1211
File: web/codegen.ts:14-14
Timestamp: 2025-03-12T13:35:43.900Z
Learning: The JSON scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibility with JsonForms integration. This change facilitates the implementation of the Connect settings web component.
Learnt from: elibosley
PR: unraid/api#1425
File: api/dev/configs/connect.json:5-13
Timestamp: 2025-06-20T20:50:14.050Z
Learning: Connect configuration files like api/dev/configs/connect.json use their own naming conventions (e.g., apikey, accesstoken, idtoken, refreshtoken) as implementation details, and these should not be changed to follow general camelCase patterns.
Learnt from: elibosley
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.persistence.ts:0-0
Timestamp: 2025-04-21T18:44:39.643Z
Learning: Use Promise-based file writing instead of callback-based approaches for better error handling and to allow proper propagation of success/failure states to callers.
Learnt from: elibosley
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.persistence.ts:0-0
Timestamp: 2025-04-21T18:44:39.643Z
Learning: When working with file operations in Node.js, prefer using the native Promise-based APIs from the fs/promises module instead of callback-based APIs from fs or manually wrapping callbacks in Promises.
🧬 Code Graph Analysis (5)
packages/unraid-api-plugin-connect/src/service/url-resolver.service.ts (1)
api/src/core/types/states/nginx.ts (1)
FqdnEntry(1-6)
packages/unraid-api-plugin-connect/src/service/connect-config.service.ts (1)
packages/unraid-api-plugin-connect/src/model/connect-config.model.ts (1)
emptyMyServersConfig(204-218)
packages/unraid-api-plugin-connect/src/test/cloud.service.test.ts (1)
api/src/environment.ts (1)
API_VERSION(68-68)
packages/unraid-api-plugin-connect/src/test/config.persistence.test.ts (1)
packages/unraid-api-plugin-connect/src/model/connect-config.model.ts (1)
ConfigType(199-202)
api/src/unraid-api/config/api-config.module.ts (1)
api/src/utils.ts (1)
csvStringToArray(262-272)
🪛 YAMLlint (1.37.1)
.github/workflows/main.yml
[error] 124-124: trailing spaces
(trailing-spaces)
[error] 129-129: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 137-137: trailing spaces
(trailing-spaces)
[error] 142-142: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 147-147: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (20)
packages/unraid-api-plugin-connect/package.json (2)
11-11: Excellent test infrastructure enablement!The change from a placeholder error to actual test execution properly enables the Connect plugin test suite.
63-63: Good vitest version update.The minor version bump from 3.1.4 to 3.2.4 should provide bug fixes and improvements.
packages/unraid-api-plugin-connect/src/service/url-resolver.service.ts (1)
331-331: Good defensive programming with optional chaining.The optional chaining prevents runtime errors if
fqdnUrlsis undefined or null, which is a solid defensive programming practice.packages/unraid-api-plugin-connect/src/test/cloud.service.test.ts (1)
35-40: Improved error handling pattern.The change from expecting rejected promises to resolved promises with error status objects is a better error handling pattern. This aligns with the service returning structured error responses instead of throwing exceptions.
packages/unraid-api-plugin-connect/src/service/connect-config.service.ts (1)
45-45: Proper identity reset alignment with model changes.Including
ssoSubIdsin the identity reset is correct since it was removed from the MyServersConfig model. This ensures complete user identity cleanup during logout..github/workflows/main.yml (2)
75-76: Good addition of early linting.Moving the lint step immediately after dependency installation catches formatting and style issues earlier in the pipeline.
117-119: Bun runtime setup added.The Bun setup step provides additional runtime capabilities for the test environment.
api/src/unraid-api/config/api-config.test.ts (1)
1-137: Excellent comprehensive test coverage!This test suite thoroughly covers all aspects of the
convertLegacyConfigmethod including string-to-boolean conversions, CSV parsing with filtering, and edge cases. The tests are well-structured with clear descriptions and proper vitest usage.packages/unraid-api-plugin-connect/src/test/config.persistence.test.ts (1)
26-332: Well-structured test suite with good coverage.The test suite effectively covers both parsing and conversion scenarios, includes validation testing, and provides valuable integration testing. The separation of concerns between parsing INI content and converting legacy config is well-tested.
api/src/unraid-api/config/api-config.module.ts (4)
59-59: Export enables proper testing.Making the
ApiConfigPersistenceclass exported is appropriate as it enables the comprehensive test suite to directly test the class methods.
101-115: Well-structured config conversion method.The
convertLegacyConfigmethod properly handles the conversion of legacy config sections (local.sandbox, api.extraOrigins, remote.ssoSubIds) with appropriate type transformations and filtering for HTTP/HTTPS origins.
117-123: Clean refactoring using the new conversion method.The refactoring of
migrateFromMyServersConfigto use the newconvertLegacyConfigmethod improves modularity and testability while maintaining the same functionality.
88-98: Verify the rationale for switching from debounceTime to bufferTime.The change from
debounceTimetobufferTime(25)changes the behavior from waiting for a pause in events to collecting events over a fixed time window. This could result in more frequent persistence calls if changes occur continuously.#!/bin/bash # Description: Check if this bufferTime pattern is used consistently across the codebase # Expected: Find similar patterns or confirm this is intentional change rg "bufferTime\(" --type ts -A 2 -B 2 rg "debounceTime\(" --type ts -A 2 -B 2packages/unraid-api-plugin-connect/src/test/url-resolver.service.test.ts (4)
22-23: Good addition of getOrThrow mock.Adding the
getOrThrowmock ensures complete ConfigService interface coverage and prevents potential test failures if the service uses this method.
114-116: Improved error assertion flexibility.Using
.some()with specific error message checks provides more robust test assertions that won't break if additional errors are present, which is better than exact array length matching.
274-294: Good adaptation to FQDN URL-based remote access.The test correctly adapts to use FQDN URLs for WAN access instead of direct IP addresses, which aligns with the service's enhanced URL resolution capabilities.
209-209: ```bash
#!/usr/bin/env bashExtract the getServerIps function and list/count all urls.push calls
sed -n '/getServerIps()/,/getRemoteAccessUrl/p' packages/unraid-api-plugin-connect/src/service/url-resolver.service.ts
| grep -n 'urls.push'
echo
echo "Total urls.push calls: $(sed -n '/getServerIps()/,/getRemoteAccessUrl/p' packages/unraid-api-plugin-connect/src/service/url-resolver.service.ts
| grep -c 'urls.push')"This will confirm if `getServerIps()` indeed pushes 7 URLs as the test expects. </details> <details> <summary>packages/unraid-api-plugin-connect/src/service/config.persistence.ts (3)</summary> `139-144`: **Excellent modularization of legacy config migration.** The refactoring into separate read, parse, and convert steps improves testability and maintainability. Each step has a clear, single responsibility which makes the code easier to understand and test. --- `153-164`: **Clear and explicit type conversions.** The explicit boolean and number conversions make the migration logic much clearer compared to relying on implicit conversions. The string-to-boolean logic for `wanaccess` and `upnpEnabled` and string-to-number conversion for `wanport` are correctly implemented. --- `173-189`: **Good method renaming and visibility changes.** Renaming `getLegacyConfig` to `readLegacyConfig` better reflects that it reads raw file content. Making `parseLegacyConfig` public enables comprehensive testing of the INI parsing logic, which is appropriate given the new test suite. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/main.yml (1)
75-77: Lint currently scopes only to/api; consider linting the whole monorepo
pnpm run lintexecutes from theapiworking directory, so packages outsideapi/(e.g.packages/*,web/,unraid-ui/) are skipped.
If the intent is to gate the entire workspace on lint, switch to a recursive workspace-wide lint:- - name: Lint - run: pnpm run lint + - name: Lint + # ‑r runs the script in every workspace package that exposes “lint” + run: pnpm -r --parallel run lint
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/main.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: elibosley
PR: unraid/api#1425
File: api/dev/configs/connect.json:5-13
Timestamp: 2025-06-20T20:50:14.050Z
Learning: Connect configuration files like api/dev/configs/connect.json use their own naming conventions (e.g., apikey, accesstoken, idtoken, refreshtoken) as implementation details, and these should not be changed to follow general camelCase patterns.
Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-generator/src/create-plugin.ts:91-112
Timestamp: 2025-04-21T18:27:36.482Z
Learning: For utility functions like file operations in api plugin generators, pujitm prefers minimal error handling rather than verbose try/catch blocks for each operation, as errors will propagate properly to higher level error handlers.
.github/workflows/main.yml (4)
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:13.145Z
Learning: Use pnpm as the package manager for all operations in the codebase.
Learnt from: mdatelle
PR: unraid/api#1122
File: web/package.json:16-16
Timestamp: 2025-02-06T17:25:45.397Z
Learning: The build script in web/package.json should include type-check command (`npm run type-check`) even when NODE_ENV is production, as it provides value for local builds by catching type errors before pushing to CI/CD.
Learnt from: elibosley
PR: unraid/api#1120
File: plugin/package.json:0-0
Timestamp: 2025-02-05T14:43:48.568Z
Learning: In Node.js projects, npm scripts should be organized with clear namespacing (e.g., build:*, docker:*, env:*) and include proper environment validation and error handling. Each script should follow the single responsibility principle.
Learnt from: pujitm
PR: unraid/api#1252
File: plugin/docker-compose.yml:15-15
Timestamp: 2025-03-26T15:18:27.452Z
Learning: The files in the `api/deploy` directory (including `packed-pnpm-store.txz`) are gitignored build artifacts that are generated when the API is built. Building the API is a prerequisite before using the plugin builder container, as it needs access to these artifacts.
🪛 YAMLlint (1.37.1)
.github/workflows/main.yml
[error] 124-124: trailing spaces
(trailing-spaces)
[error] 129-129: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 137-137: trailing spaces
(trailing-spaces)
[error] 142-142: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 147-147: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test API
- GitHub Check: Build Web App
- GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
.github/workflows/main.yml (1)
117-120: Double-check the need for Bun in this job
setup-bunis installed but the subsequent script only invokespnpm.
Unless one of the test scripts internally callsbun test(or another Bun-specific CLI), you can save 5-10 s of CI time by removing this step.
Please verify whether Bun is actually required.
Comment on lines
+121
to
+151
| - name: Run Tests Concurrently | ||
| run: | | ||
| set -e | ||
| # Run all tests in parallel with labeled output | ||
| echo "🚀 Starting API coverage tests..." | ||
| pnpm run coverage > api-test.log 2>&1 & | ||
| API_PID=$! | ||
| echo "🚀 Starting Connect plugin tests..." | ||
| (cd ../packages/unraid-api-plugin-connect && pnpm test) > connect-test.log 2>&1 & | ||
| CONNECT_PID=$! | ||
| echo "🚀 Starting Shared package tests..." | ||
| (cd ../packages/unraid-shared && pnpm test) > shared-test.log 2>&1 & | ||
| SHARED_PID=$! | ||
| # Wait for all processes and capture exit codes | ||
| wait $API_PID && echo "✅ API tests completed" || { echo "❌ API tests failed"; API_EXIT=1; } | ||
| wait $CONNECT_PID && echo "✅ Connect tests completed" || { echo "❌ Connect tests failed"; CONNECT_EXIT=1; } | ||
| wait $SHARED_PID && echo "✅ Shared tests completed" || { echo "❌ Shared tests failed"; SHARED_EXIT=1; } | ||
| # Display all outputs | ||
| echo "📋 API Test Results:" && cat api-test.log | ||
| echo "📋 Connect Plugin Test Results:" && cat connect-test.log | ||
| echo "📋 Shared Package Test Results:" && cat shared-test.log | ||
| # Exit with error if any test failed | ||
| if [[ ${API_EXIT:-0} -eq 1 || ${CONNECT_EXIT:-0} -eq 1 || ${SHARED_EXIT:-0} -eq 1 ]]; then | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-fail safety & trailing-space lint
-
Robust shell flags
Useset -euo pipefailinstead of plainset -eto catch:
• unset variables ($API_EXITet al. before defaulting)
• errors in pipelines. -
Quoting & paths
Thecd ../packages/...hops rely onapibeing the CWD. For clarity & resilience, prefer${{ github.workspace }}/packages/.... -
YAML-lint errors
Static analysis flags trailing spaces on lines 124, 129, 133, 137, 142, 145, 147. Remove them to keep the workflow YAML-lint-clean.
Example patch (trimmed for brevity):
- set -e
+ set -euo pipefail
...
- (cd ../packages/unraid-api-plugin-connect && pnpm test) > connect-test.log 2>&1 &
+ (cd "${{ github.workspace }}/packages/unraid-api-plugin-connect" && pnpm test) > connect-test.log 2>&1 &…and delete the offending trailing spaces.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Run Tests Concurrently | |
| run: | | |
| set -e | |
| # Run all tests in parallel with labeled output | |
| echo "🚀 Starting API coverage tests..." | |
| pnpm run coverage > api-test.log 2>&1 & | |
| API_PID=$! | |
| echo "🚀 Starting Connect plugin tests..." | |
| (cd ../packages/unraid-api-plugin-connect && pnpm test) > connect-test.log 2>&1 & | |
| CONNECT_PID=$! | |
| echo "🚀 Starting Shared package tests..." | |
| (cd ../packages/unraid-shared && pnpm test) > shared-test.log 2>&1 & | |
| SHARED_PID=$! | |
| # Wait for all processes and capture exit codes | |
| wait $API_PID && echo "✅ API tests completed" || { echo "❌ API tests failed"; API_EXIT=1; } | |
| wait $CONNECT_PID && echo "✅ Connect tests completed" || { echo "❌ Connect tests failed"; CONNECT_EXIT=1; } | |
| wait $SHARED_PID && echo "✅ Shared tests completed" || { echo "❌ Shared tests failed"; SHARED_EXIT=1; } | |
| # Display all outputs | |
| echo "📋 API Test Results:" && cat api-test.log | |
| echo "📋 Connect Plugin Test Results:" && cat connect-test.log | |
| echo "📋 Shared Package Test Results:" && cat shared-test.log | |
| # Exit with error if any test failed | |
| if [[ ${API_EXIT:-0} -eq 1 || ${CONNECT_EXIT:-0} -eq 1 || ${SHARED_EXIT:-0} -eq 1 ]]; then | |
| exit 1 | |
| fi | |
| - name: Run Tests Concurrently | |
| run: | | |
| - set -e | |
| + set -euo pipefail | |
| # Run all tests in parallel with labeled output | |
| echo "🚀 Starting API coverage tests..." | |
| pnpm run coverage > api-test.log 2>&1 & | |
| API_PID=$! | |
| echo "🚀 Starting Connect plugin tests..." | |
| - (cd ../packages/unraid-api-plugin-connect && pnpm test) > connect-test.log 2>&1 & | |
| + (cd "${{ github.workspace }}/packages/unraid-api-plugin-connect" && pnpm test) > connect-test.log 2>&1 & | |
| CONNECT_PID=$! | |
| echo "🚀 Starting Shared package tests..." | |
| (cd ../packages/unraid-shared && pnpm test) > shared-test.log 2>&1 & | |
| SHARED_PID=$! | |
| # Wait for all processes and capture exit codes | |
| wait $API_PID && echo "✅ API tests completed" || { echo "❌ API tests failed"; API_EXIT=1; } | |
| wait $CONNECT_PID && echo "✅ Connect tests completed" || { echo "❌ Connect tests failed"; CONNECT_EXIT=1; } | |
| wait $SHARED_PID && echo "✅ Shared tests completed" || { echo "❌ Shared tests failed"; SHARED_EXIT=1; } | |
| # Display all outputs | |
| echo "📋 API Test Results:" && cat api-test.log | |
| echo "📋 Connect Plugin Test Results:" && cat connect-test.log | |
| echo "📋 Shared Package Test Results:" && cat shared-test.log | |
| # Exit with error if any test failed | |
| if [[ ${API_EXIT:-0} -eq 1 || ${CONNECT_EXIT:-0} -eq 1 || ${SHARED_EXIT:-0} -eq 1 ]]; then | |
| exit 1 | |
| fi |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 124-124: trailing spaces
(trailing-spaces)
[error] 129-129: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 137-137: trailing spaces
(trailing-spaces)
[error] 142-142: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 147-147: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In .github/workflows/main.yml lines 121 to 151, improve shell safety by
replacing 'set -e' with 'set -euo pipefail' to catch unset variables and
pipeline errors. Update all relative 'cd ../packages/...' commands to use the
absolute path '${{ github.workspace }}/packages/...' for clarity and robustness.
Remove all trailing spaces on lines 124, 129, 133, 137, 142, 145, and 147 to fix
YAML lint errors and ensure clean formatting.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up of #1440
Summary by CodeRabbit
Bug Fixes
Refactor
Tests
Chores