Repository navigation
test(frontend): add embed builder and guild automation tests - #391
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds comprehensive test suites for multiple bot and frontend modules. Seven new test files totaling ~2,240 lines cover presence rotation, client initialization, player/queue management, embed builder, and guild automation with mocked dependencies and extensive edge-case validation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
|
Size Change: 0 B Total Size: 318 kB ℹ️ View Unchanged
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
packages/frontend/src/pages/EmbedBuilder.test.tsx (2)
67-68: Prefer accessible queries over CSS class selectors for loading states.Querying
.animate-pulsecouples the test to Tailwind implementation details. Consider addingaria-busyor a test ID to loading skeletons for more resilient tests.♻️ Alternative approach
- const skeletons = document.querySelectorAll('.animate-pulse') - expect(skeletons.length).toBeGreaterThan(0) + // Option 1: If component uses aria-busy + expect(screen.getByRole('region', { busy: true })).toBeInTheDocument() + + // Option 2: Add data-testid="loading-skeleton" to component + expect(screen.getAllByTestId('loading-skeleton').length).toBeGreaterThan(0)As per coding guidelines: "Test behavior, not implementation details".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/pages/EmbedBuilder.test.tsx` around lines 67 - 68, Replace the brittle CSS-class check for loading skeletons (document.querySelectorAll('.animate-pulse')) with an accessible or test-specific query: update the component to add an accessibility attribute like aria-busy="true" on the loading container or a data-testid (e.g., data-testid="loading-skeleton"), then change the test in EmbedBuilder.test.tsx to query by role/attribute or getByTestId/getAllByTestId and assert that at least one loading element is present; update references to the CSS selector in the test accordingly so it no longer depends on Tailwind implementation details.
23-31: Avoidanytype for the overrides parameter.The
overrides: anyparameter violates the project's strict type safety guidelines. Define a partial type for the guild store state.♻️ Proposed fix using a typed override
+type GuildStoreState = { + selectedGuild: { id: string; name: string } | null +} + -function mockGuildStore(overrides: any = {}) { +function mockGuildStore(overrides: Partial<GuildStoreState> = {}) { vi.mocked(useGuildStore).mockImplementation((selector?: any) => { const state = { selectedGuild: { id: '123', name: 'Test Guild' }, ...overrides, } return typeof selector === 'function' ? selector(state) : state }) }As per coding guidelines: "Don't ever use
any- type safety always".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/pages/EmbedBuilder.test.tsx` around lines 23 - 31, Replace the unsafe any on mockGuildStore's overrides by defining and using a typed partial of the guild store state (e.g. declare an interface or import GuildStoreState and use overrides: Partial<GuildStoreState>), update the mock implementation to merge overrides into the default state (selectedGuild: { id: '123', name: 'Test Guild' }) as you already do, and adjust any test imports/fixtures accordingly so useGuildStore and the selectedGuild shape are type-checked.packages/frontend/src/pages/GuildAutomation.test.tsx (1)
37-45: Avoidanytype for the overrides parameter.Same issue as in EmbedBuilder.test.tsx. Define a typed partial for the store state.
♻️ Proposed fix using a typed override
+type GuildStoreState = { + selectedGuild: { id: string; name: string } | null +} + -function mockGuildStore(overrides: any = {}) { +function mockGuildStore(overrides: Partial<GuildStoreState> = {}) { vi.mocked(useGuildStore).mockImplementation((selector?: any) => { const state = { selectedGuild: { id: '456', name: 'Test Automation Guild' }, ...overrides, } return typeof selector === 'function' ? selector(state) : state }) }As per coding guidelines: "Don't ever use
any- type safety always".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/pages/GuildAutomation.test.tsx` around lines 37 - 45, The mockGuildStore function currently types its overrides parameter as any; change it to a typed Partial that matches the mocked store shape (e.g. Partial<{ selectedGuild: { id: string; name: string } }>) and update the function signature from mockGuildStore(overrides: any = {}) to use that type; keep the rest of the implementation the same, ensuring the overrides variable is typed and merged into state before returning (references: mockGuildStore and useGuildStore).packages/bot/src/handlers/queueHandler.spec.ts (1)
90-90: Assert autoplay with the enum constant, not numeric literal3.Using
3couples the test to currentdiscord-playerenum internals. PreferQueueRepeatMode.AUTOPLAYto keep compatibility across library updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/handlers/queueHandler.spec.ts` at line 90, Replace the numeric literal 3 in the expectation with the enum constant to avoid coupling to discord-player internals: update the assertion using QueueRepeatMode.AUTOPLAY (importing QueueRepeatMode if not already) so the test calls expect(mockQueue.setRepeatMode).toHaveBeenCalledWith(QueueRepeatMode.AUTOPLAY) instead of 3.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bot/src/handlers/clientHandler/presence.spec.ts`:
- Around line 1-437: The spec file is too large (>250 lines); split tests into
multiple focused spec files so each is under the limit. Create separate test
files for: 1) math/helpers (nextPresenceIndex, PRESENCE_ROTATION_INTERVAL_MS),
2) guild/player helpers (getTotalMemberCount, getActiveMusicSessions), 3)
activity builder (buildPresenceActivities), and 4) presence control logic
(setPresenceActivity, startPresenceRotation) — move relevant describe blocks
into those files, preserve the shared mocks (mockGetBotPresenceStatus) or
re-mock it per file, and update imports to reference the same symbols
(nextPresenceIndex, getTotalMemberCount, getActiveMusicSessions,
buildPresenceActivities, setPresenceActivity, startPresenceRotation) so tests
run unchanged but each file stays under 250 lines.
In `@packages/bot/src/handlers/clientHandler/service.spec.ts`:
- Line 52: The import list in the test file includes an unused symbol; remove
the unused infoLog import from the import statement that currently reads "import
{ debugLog, infoLog, errorLog } from '@lucky/shared/utils'" so only referenced
utilities (e.g., debugLog and errorLog) remain; update the import to drop
infoLog to eliminate the dead import.
- Around line 1-271: The test file exceeds the 250-line limit; split the spec
into two smaller files by behavior: extract the describe('createClient', ...)
block (tests referencing createClient, Client, errorLog, debugLog, Collection)
into a new spec (e.g., service.createClient.spec.ts) and keep
describe('startClient', ...) (tests referencing startClient,
startPresenceRotation, once/login, errorLog) in the original or a new
service.startClient.spec.ts; update imports/mocks at top of each new file so
each contains only the mocks and imports needed for its tests (retain mocks for
'@lucky/shared/utils', '@lucky/shared/config', './presence',
'../../services/MusicPresenceService', and the discord.js mock where required),
run tests to ensure each file is under 250 lines and CI passes.
- Around line 101-103: Replace all `mockClient as any` casts in service.spec.ts
with `unknown` and assert types via explicit type guards or jest.Mocked
utilities; locate uses of `mockClient` (at the listed lines) and change the cast
pattern to `(mockClient as unknown)` then narrow it using a runtime/type-guard
check or wrap with `jest.Mocked<typeof Client>`/`jest.Mocked<Client>` where
appropriate, following the existing example at the `;(Client as unknown as
jest.Mock).mockImplementationOnce` line to preserve type safety without using
`any`.
In `@packages/bot/src/handlers/player/playerFactory.spec.ts`:
- Around line 5-126: Tests are re-implementing internals instead of exercising
the real exports from playerFactory.ts; replace the inline/local constants and
helpers (e.g., the local isYouTubeUrl, validateParams, extractorOptions,
ytDlpArgs, spawnOptions, handleError, isProcessError, maxListeners) with imports
from packages/bot/src/handlers/player/playerFactory.ts and assert the actual
runtime behavior (for example import the exported ytDlpArgs or a function that
builds the yt-dlp args and assert it appends the URL, import the exported
createPlayer or param validator and test real validation, import the extractor
options/highWaterMark constant and test its value, and import the process error
type guard and test real behavior). Locate usages by symbol names in the file
(isYouTubeUrl, validateParams, extractorOptions, ytDlpArgs, spawnOptions,
handleError, isProcessError, maxListeners, createPlayer) and replace the
duplicated definitions with direct imports and tests against those exports so
the suite verifies public behavior rather than local redefinitions.
- Line 26: The test uses an unnecessary type cast; remove the "as any" on the
object passed to validateParams in playerFactory.spec.ts. Update the assertion
to call validateParams({ client: null }) directly (the object already matches
CreatePlayerParams where client: unknown) so the test uses real typing rather
than bypassing it; keep the expect(validateParams(...)).toBe(true) assertion and
do not change the validateParams function itself.
In `@packages/bot/src/handlers/playerHandler.spec.ts`:
- Around line 5-53: Replace the local, dummy type checks with real imports and
behavior assertions: import the actual player handler exports (e.g.,
createPlayer, recentlyPlayedTracks, lastPlayedTracks) and the track
handlers/types from the production module (reference trackHandlers.ts shape:
url, author, thumbnail, timestamp) and update the tests to validate the real
exported structures and values (assert that recentlyPlayedTracks is a Map with
expected entries, lastPlayedTracks is a Map, and TrackHistoryEntry matches the
production shape including url/author/thumbnail/timestamp) rather than asserting
locally-declared placeholder types.
In `@packages/bot/src/handlers/queueHandler.spec.ts`:
- Around line 102-114: The tests in queueHandler.spec.ts use try/catch blocks
that allow silent success (tests pass if no error is thrown); replace those with
deterministic assertions using Jest's async reject matcher—call createQueue({
client, interaction }) inside await expect(...).rejects and assert the error
shape (e.g., expect.objectContaining({ name: 'ValidationError', details:
expect.objectContaining({ userId: 'user-1', channelId: 'channel-1' }) })) so
failures are caught; apply the same change to the other similar test blocks
referenced (around the ranges that include lines 116-128, 130-142, and 225-235)
and reference the createQueue invocation and error.details checks when updating
each test.
In `@packages/frontend/src/pages/GuildAutomation.test.tsx`:
- Around line 563-576: The test currently finds the refresh button using a
fragile DOM traversal (document.querySelectorAll('button') + svg) and then
guards with an if, which can let the test pass silently; replace that with an
accessible query (e.g., use getByRole('button', { name: /refresh/i }) or
getByLabelText('Refresh') from your testing library) to explicitly locate the
button and remove the conditional so the test fails if not found, then call
user.click on that retrieved element and keep the existing waitFor assertions
(api.automation.getStatus / api.automation.getManifest); if the component lacks
an accessible name, add aria-label="Refresh" to the refresh button in the
component to make it testable and accessible.
---
Nitpick comments:
In `@packages/bot/src/handlers/queueHandler.spec.ts`:
- Line 90: Replace the numeric literal 3 in the expectation with the enum
constant to avoid coupling to discord-player internals: update the assertion
using QueueRepeatMode.AUTOPLAY (importing QueueRepeatMode if not already) so the
test calls
expect(mockQueue.setRepeatMode).toHaveBeenCalledWith(QueueRepeatMode.AUTOPLAY)
instead of 3.
In `@packages/frontend/src/pages/EmbedBuilder.test.tsx`:
- Around line 67-68: Replace the brittle CSS-class check for loading skeletons
(document.querySelectorAll('.animate-pulse')) with an accessible or
test-specific query: update the component to add an accessibility attribute like
aria-busy="true" on the loading container or a data-testid (e.g.,
data-testid="loading-skeleton"), then change the test in EmbedBuilder.test.tsx
to query by role/attribute or getByTestId/getAllByTestId and assert that at
least one loading element is present; update references to the CSS selector in
the test accordingly so it no longer depends on Tailwind implementation details.
- Around line 23-31: Replace the unsafe any on mockGuildStore's overrides by
defining and using a typed partial of the guild store state (e.g. declare an
interface or import GuildStoreState and use overrides:
Partial<GuildStoreState>), update the mock implementation to merge overrides
into the default state (selectedGuild: { id: '123', name: 'Test Guild' }) as you
already do, and adjust any test imports/fixtures accordingly so useGuildStore
and the selectedGuild shape are type-checked.
In `@packages/frontend/src/pages/GuildAutomation.test.tsx`:
- Around line 37-45: The mockGuildStore function currently types its overrides
parameter as any; change it to a typed Partial that matches the mocked store
shape (e.g. Partial<{ selectedGuild: { id: string; name: string } }>) and update
the function signature from mockGuildStore(overrides: any = {}) to use that
type; keep the rest of the implementation the same, ensuring the overrides
variable is typed and merged into state before returning (references:
mockGuildStore and useGuildStore).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e791c2a-79c7-422d-b3f2-c01cb85da21f
📒 Files selected for processing (7)
packages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/clientHandler/service.spec.tspackages/bot/src/handlers/player/playerFactory.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/frontend/src/pages/GuildAutomation.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Quality Gates
- GitHub Check: SonarCloud Scan
- GitHub Check: compressed-size
🧰 Additional context used
📓 Path-based instructions (22)
**/*.{js,jsx,ts,tsx,vue,html}
📄 CodeRabbit inference engine (.cursor/rules/accessibility-openness.mdc)
Provide accessible UI components using semantic HTML and ARIA attributes where necessary
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dependency-injection.mdc)
**/*.{ts,tsx,js,jsx}: Prefer constructor injection for classes that require dependencies
Avoid global mutable singletons unless necessary
Use explicit interfaces for external dependencies to make testing easier
**/*.{ts,tsx,js,jsx}: Include required references in PRs/code for non-trivial logic: TypeScript (official docs), MDN (JavaScript reference), and official docs for any runtime/framework/libraries used (e.g., Node.js, React) as applicable.
Before assuming behavior of an API, include the doc link and a ≤25-word quote when the change relies on it.
**/*.{ts,tsx,js,jsx}: Prefer named exports for clear usage and easier refactors in TypeScript/JavaScript
Keep import order consistent: external first, then internal modules
Remove dead code and unused imports
**/*.{ts,tsx,js,jsx}: Use PascalCase naming convention for React/UI components
Use camelCase naming convention for variables and functions
Use UPPER_SNAKE_CASE naming convention for constants
Maintain consistent import grouping and ordering within the project, keeping third-party imports separate from local imports
For external data sources (HTTP, database), always validate and sanitize input using type guards or schema validators
**/*.{ts,tsx,js,jsx}: Use Prettier with no semicolons, single quotes, 4-space indent, 80 character width
Files must not exceed 250 lines and this is enforcedImplement TypeScript typecheck and linter in CI quality checks
**/*.{ts,tsx,js,jsx}: Use TypeScript for enhanced type safety
Implement error handling and error logging
Avoid commenting code unless extremely necessary - code should explain itself with descriptive names
Leave NO todos, placeholders or missing pieces in the code
Variables and functions must use camelCase
Constants must use UPPER_SNAKE_CASE
Use arrow functions for methods and computed properties
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Maintain consistent import grouping/order: external imports first, then...
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{js,jsx,ts,tsx}: Never throw strings. ThrowError(or typed subclasses) with descriptive messages
Include causal error ascausewhen available for better debugging
Define clear, stable error codes (e.g.,ERR_AUTH_EXPIRED,ERR_NETWORK_TIMEOUT)
Provide optional metadata (e.g.,details,retryable,status,correlationId) in error objects
Use domain error classes per area (e.g.,AuthenticationError,ValidationError,NetworkError)
Log errors with structure (message, code, stack, cause, correlationId, user context where appropriate)
MarkretryablevsnonRetryableerrors where helpful for operations
Set timeouts and handle aborts/cancellations; avoid dangling requests in API/network code
Implement backoff for transient failures; avoid infinite retries
Map HTTP status → domain errors; 4xx vs 5xx behave differently (e.g., retry for 5xx/network)
**/*.{js,jsx,ts,tsx}: Use functional components with hooks in React/React Native. Avoid class components.
Keep components focused on a single responsibility; extract complex logic into custom hooks.
Keep state local when possible. Use Context/Zustand/Redux only when necessary for state management.
If props or state traverse more than 3 levels, consider using context or a feature-scoped store instead of prop drilling.
Use performance optimization techniques:React.memo,useMemo,useCallback,Suspense(web), and virtualization for long lists; avoid unnecessary re-renders.
Web accessibility: use semantic HTML, labels, focus management, keyboard navigation, andaria-*attributes as needed.
React Native accessibility: use accessibility props (accessible,accessibilityLabel), proper roles and labels.
Identify and extract repetitive UI components proactively tocomponents/with clear props and minimal coupling.
Web styles: prefer co-located styles or design system tokens; avoid global style leakage.
React Native styles: preferStyleSheet.create, design tokens, and theme providers; avoid in...
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Test behavior, not implementation. Prefer Testing Library utilities for testing React/React Native components.
For React Native tests: mock native modules and test component interactions and accessibility labels.
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
Introduce interfaces at module boundaries to enable testing and substitutions
**/*.{ts,tsx}: Avoid usinganytype in TypeScript. If unavoidable, useunknownwith type guards and justify with a code comment
Preferinterfacefor defining public object shapes in TypeScript, usetypefor unions and utility types
Use TypeScript utility types such asPartial,Pick,Omit,Readonly, andRecordwhen appropriate
UseI{Name}naming convention for interfaces in TypeScript
UseT{Name}naming convention for type aliases and utility types in TypeScript
**/*.{ts,tsx}: Functions must be less than 50 lines with cyclomatic complexity less than 10
Do not useanytypes - ESLint enforces this at error level
**/*.{ts,tsx}: Prefer types over interfaces for most cases
Don't ever useany- type safety always
Avoid enums; use const objects instead
For complex types, create a separate file to declare them and import them
Avoid usinganytype; if unavoidable, useunknownwith type guards and justify with code comment
Preferinterfacefor public API shapes; usetypefor unions and utility types
Use TypeScript utility types (Partial, Pick, Omit, Readonly, Record)
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Test behavior, not implementation details
Prefer unit tests for core logic; add integration tests at meaningful boundaries
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/testing-quality.mdc)
**/*.{test,spec}.{js,ts,jsx,tsx}: Use Jest + a React testing library for unit and component tests as applicable
Test behavior, not implementation details
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/documentation.mdc)
**/*.{js,ts,tsx,jsx}: Minimize comments in code; explain the 'why' when non-obvious, let code express the 'what' through clear naming
Document trade-offs briefly when deviating from ideal patterns
**/*.{js,ts,tsx,jsx}: Store secrets, ports, and hosts in environment variables (.env,.env.example) and never hardcode them
Avoid redundant or decorative AI comments; code should be self-explanatory and only commented when logic is non-obvious; prefer refactoring over lengthy commentsNever hardcode secrets, IPs, or ports; use
.envanddocs/for required configuration variables
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
packages/bot/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)
packages/bot/**/*.{ts,tsx}: UseuseMainPlayer()fromdiscord-playerto access the player instance; do not instantiate player directly
Do not duplicate queue or player state outside Discord Player; use shared services from@lucky/sharedfor persistent data like track history and session information
UseerrorLoganddebugLogfrom@lucky/shared/utilsfor logging throughout the bot package
Use embed and reply utilities from@lucky/sharedfor consistent message formatting and error sanitization across the bot
Use services from@lucky/shared(DatabaseService, Redis client) for database and cache access; do not instantiate Prisma or Redis directly in the bot package
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/bot/src/handlers/clientHandler/service.spec.ts
packages/bot/src/handlers/player/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)
Track handling, errors, and lifecycle must be managed through dedicated handlers in
packages/bot/src/handlers/player/(trackHandlers, errorHandlers, lifecycleHandlers)
Files:
packages/bot/src/handlers/player/playerFactory.spec.ts
packages/bot/**
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
The
botpackage depends onsharedand contains Discord bot commands and player handlers using Discord.js and Discord Player
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/bot/src/handlers/clientHandler/service.spec.ts
**/*.{js,mjs,ts,mts}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Use Node.js version ≥22 with ESM (ECMAScript modules) only; no CommonJS
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/bot/src/handlers/clientHandler/service.spec.ts
**/*.{spec,test}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)
**/*.{spec,test}.{ts,tsx,js,jsx}: Use Jest for unit and integration tests
Test behavior, not implementation details
Run unit, integration tests, and coverage report in CI quality checks
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)
Unit tests must use naming convention
*.spec.ts
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/bot/src/handlers/clientHandler/service.spec.ts
packages/bot/src/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)
Use
@lucky/sharedfor database, Redis, logging, and embed utilities instead of implementing them locally
Files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/bot/src/handlers/clientHandler/service.spec.ts
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{jsx,tsx}: Use toasts/snackbars for transient errors; avoid blocking modals for non-critical issues in React/React Native UI
Debounce/suppress duplicate toasts to prevent spam
Provide retry/refresh actions when meaningful (e.g., network failure) in error UI
Use error boundaries for render-time exceptions; show fallback UI in React
Respect accessibility: toasts should be announced (aria-live on web; accessibility hints on React Native)
Files:
packages/frontend/src/pages/EmbedBuilder.test.tsxpackages/frontend/src/pages/GuildAutomation.test.tsx
packages/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/frontend/src/**/*.{ts,tsx}: Frontend errors are created by Axios interceptor and should be of typeApiErrorwith status and details from backend
Frontend uses path alias@/mapped tosrc/- use this alias for all imports from the src directoryDo not depend on
@lucky/sharedpackage in frontend code; make API calls to backend via configured base URL (env)
Files:
packages/frontend/src/pages/EmbedBuilder.test.tsxpackages/frontend/src/pages/GuildAutomation.test.tsx
packages/frontend/src/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Organize pages in
packages/frontend/src/pages/directory (e.g., Login, Dashboard, Config, Features, ServersPage)
Files:
packages/frontend/src/pages/EmbedBuilder.test.tsxpackages/frontend/src/pages/GuildAutomation.test.tsx
packages/frontend/src/{components,pages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Use React functional components and hooks; keep components small and focused
Files:
packages/frontend/src/pages/EmbedBuilder.test.tsxpackages/frontend/src/pages/GuildAutomation.test.tsx
packages/frontend/src/{components,pages}/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Follow existing styling approach (e.g., Tailwind if present); avoid inline styles for layout and theming
Files:
packages/frontend/src/pages/EmbedBuilder.test.tsxpackages/frontend/src/pages/GuildAutomation.test.tsx
packages/frontend/**
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
The
frontendpackage uses React with Vite and must not depend on the shared package
Files:
packages/frontend/src/pages/EmbedBuilder.test.tsxpackages/frontend/src/pages/GuildAutomation.test.tsx
**/[A-Z]*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Components must use PascalCase naming
Files:
packages/frontend/src/pages/EmbedBuilder.test.tsxpackages/frontend/src/pages/GuildAutomation.test.tsx
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-project.mdc:0-0
Timestamp: 2026-03-09T20:21:08.612Z
Learning: Applies to {packages/*/tests/**/*.test.{js,ts},tests/**/*.test.{js,ts}} : Add or adjust unit and integration tests when changing behavior; follow existing patterns in `packages/*/tests` and root `tests/` directories
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-frontend.mdc:0-0
Timestamp: 2026-03-09T20:21:58.991Z
Learning: Write unit and integration tests in `packages/frontend/tests`; use Playwright for E2E tests when changing user flows
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to tests/**/*.test.{ts,tsx,js,jsx} : Add integration tests where appropriate
📚 Learning: 2026-03-09T20:21:08.612Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-project.mdc:0-0
Timestamp: 2026-03-09T20:21:08.612Z
Learning: Applies to {packages/*/tests/**/*.test.{js,ts},tests/**/*.test.{js,ts}} : Add or adjust unit and integration tests when changing behavior; follow existing patterns in `packages/*/tests` and root `tests/` directories
Applied to files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/src/handlers/player/**/*.{ts,tsx} : Track handling, errors, and lifecycle must be managed through dedicated handlers in `packages/bot/src/handlers/player/` (trackHandlers, errorHandlers, lifecycleHandlers)
Applied to files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/bot/src/handlers/clientHandler/service.spec.ts
📚 Learning: 2026-03-09T20:21:38.098Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-backend.mdc:0-0
Timestamp: 2026-03-09T20:21:38.098Z
Learning: Applies to packages/backend/tests/**/*.ts : Follow existing patterns for unit and integration tests in `packages/backend/tests/`
Applied to files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to **/*.{spec,test}.{ts,tsx,js,jsx} : Test behavior, not implementation details
Applied to files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsx
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to **/*.{spec,test}.{ts,tsx,js,jsx} : Use Jest for unit and integration tests
Applied to files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/frontend/src/pages/GuildAutomation.test.tsx
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/tests/**/*.{ts,tsx,js} : Write tests in `packages/frontend/tests/` using existing test patterns (e.g., Playwright for e2e if configured)
Applied to files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/playerHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsx
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/tests/**/*.{ts,tsx} : Organize tests in `packages/backend/tests/` with unit tests under `unit/` and integration tests under `integration/`, following existing patterns with fixtures and setup
Applied to files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Use `useMainPlayer()` from `discord-player` to access the player instance; do not instantiate player directly
Applied to files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/bot/src/handlers/playerHandler.spec.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to tests/**/*.test.{ts,tsx,js,jsx} : Add integration tests where appropriate
Applied to files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/frontend/src/pages/GuildAutomation.test.tsxpackages/bot/src/handlers/clientHandler/service.spec.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to tests/**/*.test.ts : Integration tests must use naming convention `*.test.ts` and be located inside a `/tests` folder at the project's root
Applied to files:
packages/bot/src/handlers/player/playerFactory.spec.tspackages/frontend/src/pages/EmbedBuilder.test.tsxpackages/frontend/src/pages/GuildAutomation.test.tsx
📚 Learning: 2026-03-09T20:21:58.991Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-frontend.mdc:0-0
Timestamp: 2026-03-09T20:21:58.991Z
Learning: Write unit and integration tests in `packages/frontend/tests`; use Playwright for E2E tests when changing user flows
Applied to files:
packages/frontend/src/pages/EmbedBuilder.test.tsxpackages/frontend/src/pages/GuildAutomation.test.tsx
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to tests/e2e/**/*.{ts,tsx,js,jsx} : E2E tests must be placed under `tests/e2e/`
Applied to files:
packages/frontend/src/pages/EmbedBuilder.test.tsx
📚 Learning: 2026-03-15T21:57:49.951Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-15T21:57:49.951Z
Learning: When working on unit tests, Jest ESM mocks, or fixing disabled tests, use the `testing-lucky` skill
Applied to files:
packages/bot/src/handlers/clientHandler/presence.spec.tspackages/bot/src/handlers/clientHandler/service.spec.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Do not duplicate queue or player state outside Discord Player; use shared services from `lucky/shared` for persistent data like track history and session information
Applied to files:
packages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.tspackages/bot/src/handlers/clientHandler/service.spec.ts
📚 Learning: 2026-03-09T20:21:08.612Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-project.mdc:0-0
Timestamp: 2026-03-09T20:21:08.612Z
Learning: Applies to packages/bot/** : The `bot` package depends on `shared` and contains Discord bot commands and player handlers using Discord.js and Discord Player
Applied to files:
packages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/clientHandler/service.spec.ts
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/music/**/*.ts : Use existing voice/queue/guild validators before manipulating player or queue state
Applied to files:
packages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.ts
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/music/commands/**/*.ts : Use `.cursor/skills/music-queue-player/SKILL.md` for play, queue, skip, volume commands and player lifecycle management
Applied to files:
packages/bot/src/handlers/playerHandler.spec.tspackages/bot/src/handlers/queueHandler.spec.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/src/functions/*/commands/**/*.{ts,tsx} : Use existing validators from `packages/bot/src/utils/command/` for voice channel, queue, and guild validations in commands
Applied to files:
packages/bot/src/handlers/queueHandler.spec.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to **/*.{spec,test}.{ts,tsx,js,jsx} : Run unit, integration tests, and coverage report in CI quality checks
Applied to files:
packages/frontend/src/pages/GuildAutomation.test.tsx
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/src/{services,middleware}/**/*.{ts,tsx} : Implement Discord OAuth for authentication in backend services
Applied to files:
packages/bot/src/handlers/clientHandler/service.spec.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Use services from `lucky/shared` (DatabaseService, Redis client) for database and cache access; do not instantiate Prisma or Redis directly in the bot package
Applied to files:
packages/bot/src/handlers/clientHandler/service.spec.ts
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/{general,music,download}/commands/**/*.ts : Apply `.cursor/rules/lucky-discord-bot.mdc` rules for Discord bot commands and player implementation
Applied to files:
packages/bot/src/handlers/clientHandler/service.spec.ts
🪛 GitHub Check: SonarCloud Code Analysis
packages/bot/src/handlers/player/playerFactory.spec.ts
[warning] 22-22: Prefer using an optional chain expression instead, as it's more concise and easier to read.
packages/bot/src/handlers/clientHandler/presence.spec.ts
[warning] 42-45: This assertion is unnecessary since it does not change the type of the expression.
packages/bot/src/handlers/playerHandler.spec.ts
[warning] 7-7: Remove this redundant type alias and replace its occurrences with "unknown".
[warning] 6-6: Remove this redundant type alias and replace its occurrences with "unknown".
packages/bot/src/handlers/queueHandler.spec.ts
[warning] 51-56: This assertion is unnecessary since it does not change the type of the expression.
[warning] 36-47: This assertion is unnecessary since it does not change the type of the expression.
[warning] 14-32: This assertion is unnecessary since it does not change the type of the expression.
packages/bot/src/handlers/clientHandler/service.spec.ts
[warning] 52-52: Remove this unused import of 'infoLog'.
🔇 Additional comments (5)
packages/frontend/src/pages/EmbedBuilder.test.tsx (1)
1-456: Well-structured test suite with comprehensive coverage.The test suite effectively covers the EmbedBuilder component's key behaviors: loading states, CRUD operations, form validation, field management, preview functionality, and error handling. The tests use proper Testing Library patterns with accessible queries and realistic user interactions.
packages/frontend/src/pages/GuildAutomation.test.tsx (2)
411-447: Good coverage for JSON validation error handling.The test properly verifies that invalid JSON in the manifest editor shows an appropriate validation error message. This edge case coverage is valuable for preventing silent failures.
1-657: Comprehensive test suite for GuildAutomation workflows.The test suite provides thorough coverage of the GuildAutomation page including: status display, plan/apply/reconcile operations, manifest editor expand/save/validation flows, run history rendering, and error handling. The toast mocking approach correctly verifies user feedback. API mock signatures align with the actual implementation.
packages/bot/src/handlers/clientHandler/presence.spec.ts (1)
27-46: ESLint does not enforceno-anyin test files; this pattern is exempted from linting.The ESLint config explicitly ignores
**/*.spec.tsfiles, so the assertion that "ESLint enforces this at error level" is incorrect. While improving mock typing is a code quality goal, it is not subject to the linting rules applied to production code.> Likely an incorrect or invalid review comment.packages/bot/src/handlers/queueHandler.spec.ts (1)
32-32: Reduceanycasts in test fixtures where practical; ESLint does not enforce this for spec files.While coding guidelines discourage
anytypes, ESLint explicitly ignores**/*.spec.tsfiles and only enforces@typescript-eslint/no-explicit-anyonsrc/**/*.tssource code. Test files commonly useanyfor mocking and error assertions where strict typing is less critical. Consider replacing with proper types where feasible (e.g., usingPartial<T>orunknownwith type guards), but recognize this is a guideline preference rather than an enforcement error.Affected lines: 32, 47, 56, 86, 108, 125, 139, 165, 179, 214, 231
Lines 32, 47, 56: Mock object casts in factory functions Lines 108, 125, 139, 214, 231: Error type assertions in catch blocks Lines 165, 179: Mock data casts> Likely an incorrect or invalid review comment.
| import { describe, it, expect, beforeEach, jest } from '@jest/globals' | ||
| import { ActivityType } from 'discord.js' | ||
| import type { CustomClient } from '../../types' | ||
|
|
||
| const mockGetBotPresenceStatus = jest.fn().mockReturnValue('online') | ||
|
|
||
| jest.mock('../../utils/presenceStatus', () => ({ | ||
| getBotPresenceStatus: mockGetBotPresenceStatus, | ||
| })) | ||
|
|
||
| import { | ||
| PRESENCE_ROTATION_INTERVAL_MS, | ||
| nextPresenceIndex, | ||
| getTotalMemberCount, | ||
| getActiveMusicSessions, | ||
| buildPresenceActivities, | ||
| setPresenceActivity, | ||
| startPresenceRotation, | ||
| } from './presence' | ||
|
|
||
| function createMockGuild(memberCount: number) { | ||
| return { | ||
| memberCount, | ||
| } | ||
| } | ||
|
|
||
| function createMockClient(overrides?: Partial<CustomClient>): CustomClient { | ||
| const defaultClient = { | ||
| guilds: { | ||
| cache: { | ||
| size: 0, | ||
| values: () => [], | ||
| }, | ||
| }, | ||
| commands: { | ||
| size: 0, | ||
| }, | ||
| user: null, | ||
| player: null, | ||
| } | ||
|
|
||
| return { | ||
| ...defaultClient, | ||
| ...overrides, | ||
| } as any | ||
| } | ||
|
|
||
| describe('presence', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks() | ||
| }) | ||
|
|
||
| describe('PRESENCE_ROTATION_INTERVAL_MS', () => { | ||
| it('should be 45 seconds', () => { | ||
| expect(PRESENCE_ROTATION_INTERVAL_MS).toBe(45_000) | ||
| }) | ||
| }) | ||
|
|
||
| describe('nextPresenceIndex', () => { | ||
| it('should increment index', () => { | ||
| expect(nextPresenceIndex(0, 5)).toBe(1) | ||
| expect(nextPresenceIndex(1, 5)).toBe(2) | ||
| }) | ||
|
|
||
| it('should wrap around at end', () => { | ||
| expect(nextPresenceIndex(4, 5)).toBe(0) | ||
| }) | ||
|
|
||
| it('should handle single activity', () => { | ||
| expect(nextPresenceIndex(0, 1)).toBe(0) | ||
| }) | ||
| }) | ||
|
|
||
| describe('getTotalMemberCount', () => { | ||
| it('should return 0 for no guilds', () => { | ||
| const client = createMockClient() | ||
| expect(getTotalMemberCount(client)).toBe(0) | ||
| }) | ||
|
|
||
| it('should sum member counts across guilds', () => { | ||
| const client = createMockClient({ | ||
| guilds: { | ||
| cache: { | ||
| values: jest | ||
| .fn() | ||
| .mockReturnValue([ | ||
| createMockGuild(100), | ||
| createMockGuild(200), | ||
| createMockGuild(50), | ||
| ]), | ||
| }, | ||
| }, | ||
| } as any) | ||
|
|
||
| expect(getTotalMemberCount(client)).toBe(350) | ||
| }) | ||
|
|
||
| it('should handle guilds with undefined memberCount', () => { | ||
| const client = createMockClient({ | ||
| guilds: { | ||
| cache: { | ||
| values: jest | ||
| .fn() | ||
| .mockReturnValue([ | ||
| { memberCount: 100 }, | ||
| { memberCount: undefined }, | ||
| { memberCount: 50 }, | ||
| ]), | ||
| }, | ||
| }, | ||
| } as any) | ||
|
|
||
| expect(getTotalMemberCount(client)).toBe(150) | ||
| }) | ||
| }) | ||
|
|
||
| describe('getActiveMusicSessions', () => { | ||
| it('should return 0 when player nodes are undefined', () => { | ||
| const client = createMockClient({ player: null } as any) | ||
| expect(getActiveMusicSessions(client)).toBe(0) | ||
| }) | ||
|
|
||
| it('should return 0 when cache is undefined', () => { | ||
| const client = createMockClient({ | ||
| player: { nodes: {} }, | ||
| } as any) | ||
| expect(getActiveMusicSessions(client)).toBe(0) | ||
| }) | ||
|
|
||
| it('should count nodes with currentTrack', () => { | ||
| const client = createMockClient({ | ||
| player: { | ||
| nodes: { | ||
| cache: { | ||
| values: jest | ||
| .fn() | ||
| .mockReturnValue([ | ||
| { currentTrack: { title: 'Song 1' } }, | ||
| { currentTrack: null }, | ||
| { currentTrack: { title: 'Song 2' } }, | ||
| ]), | ||
| }, | ||
| }, | ||
| }, | ||
| } as any) | ||
|
|
||
| expect(getActiveMusicSessions(client)).toBe(2) | ||
| }) | ||
|
|
||
| it('should return 0 when no nodes have currentTrack', () => { | ||
| const client = createMockClient({ | ||
| player: { | ||
| nodes: { | ||
| cache: { | ||
| values: jest | ||
| .fn() | ||
| .mockReturnValue([ | ||
| { currentTrack: null }, | ||
| { currentTrack: undefined }, | ||
| ]), | ||
| }, | ||
| }, | ||
| }, | ||
| } as any) | ||
|
|
||
| expect(getActiveMusicSessions(client)).toBe(0) | ||
| }) | ||
| }) | ||
|
|
||
| describe('buildPresenceActivities', () => { | ||
| it('should build 5 activities', () => { | ||
| const activities = buildPresenceActivities({ | ||
| guildCount: 10, | ||
| memberCount: 500, | ||
| commandCount: 50, | ||
| activeMusicSessions: 3, | ||
| }) | ||
|
|
||
| expect(activities).toHaveLength(5) | ||
| }) | ||
|
|
||
| it('should include guild count in activities', () => { | ||
| const activities = buildPresenceActivities({ | ||
| guildCount: 42, | ||
| memberCount: 500, | ||
| commandCount: 50, | ||
| activeMusicSessions: 0, | ||
| }) | ||
|
|
||
| expect(activities[1].name).toBe('42 servers managed') | ||
| }) | ||
|
|
||
| it('should include member count in activities', () => { | ||
| const activities = buildPresenceActivities({ | ||
| guildCount: 10, | ||
| memberCount: 1234, | ||
| commandCount: 50, | ||
| activeMusicSessions: 0, | ||
| }) | ||
|
|
||
| expect(activities[2].name).toBe('1234 members protected') | ||
| }) | ||
|
|
||
| it('should show active music sessions when > 0', () => { | ||
| const activities = buildPresenceActivities({ | ||
| guildCount: 10, | ||
| memberCount: 500, | ||
| commandCount: 50, | ||
| activeMusicSessions: 7, | ||
| }) | ||
|
|
||
| expect(activities[3].name).toBe('7 active music sessions') | ||
| }) | ||
|
|
||
| it('should show moderation message when no music sessions', () => { | ||
| const activities = buildPresenceActivities({ | ||
| guildCount: 10, | ||
| memberCount: 500, | ||
| commandCount: 50, | ||
| activeMusicSessions: 0, | ||
| }) | ||
|
|
||
| expect(activities[3].name).toBe('Fast and safe moderation') | ||
| }) | ||
|
|
||
| it('should include command count', () => { | ||
| const activities = buildPresenceActivities({ | ||
| guildCount: 10, | ||
| memberCount: 500, | ||
| commandCount: 75, | ||
| activeMusicSessions: 0, | ||
| }) | ||
|
|
||
| expect(activities[4].name).toBe('/help • 75 commands') | ||
| }) | ||
|
|
||
| it('should set correct activity types', () => { | ||
| const activities = buildPresenceActivities({ | ||
| guildCount: 10, | ||
| memberCount: 500, | ||
| commandCount: 50, | ||
| activeMusicSessions: 0, | ||
| }) | ||
|
|
||
| expect(activities[0].type).toBe(ActivityType.Listening) | ||
| expect(activities[1].type).toBe(ActivityType.Watching) | ||
| expect(activities[2].type).toBe(ActivityType.Watching) | ||
| expect(activities[3].type).toBe(ActivityType.Competing) | ||
| expect(activities[4].type).toBe(ActivityType.Playing) | ||
| }) | ||
| }) | ||
|
|
||
| describe('setPresenceActivity', () => { | ||
| it('should return index when client.user is null', () => { | ||
| const client = createMockClient({ user: null }) | ||
| const result = setPresenceActivity(client, 2) | ||
| expect(result).toBe(2) | ||
| }) | ||
|
|
||
| it('should set presence and return next index', () => { | ||
| const setPresence = jest.fn() | ||
| const client = createMockClient({ | ||
| user: { setPresence }, | ||
| guilds: { cache: { size: 5, values: () => [] } }, | ||
| commands: { size: 30 }, | ||
| } as any) | ||
|
|
||
| const result = setPresenceActivity(client, 0) | ||
|
|
||
| expect(setPresence).toHaveBeenCalled() | ||
| expect(result).toBe(1) | ||
| }) | ||
|
|
||
| it('should handle negative index', () => { | ||
| const setPresence = jest.fn() | ||
| const client = createMockClient({ | ||
| user: { setPresence }, | ||
| guilds: { cache: { size: 5, values: () => [] } }, | ||
| commands: { size: 30 }, | ||
| } as any) | ||
|
|
||
| const result = setPresenceActivity(client, -1) | ||
|
|
||
| expect(setPresence).toHaveBeenCalled() | ||
| expect(result).toBeGreaterThanOrEqual(0) | ||
| }) | ||
|
|
||
| it('should handle index beyond array length', () => { | ||
| const setPresence = jest.fn() | ||
| const client = createMockClient({ | ||
| user: { setPresence }, | ||
| guilds: { cache: { size: 5, values: () => [] } }, | ||
| commands: { size: 30 }, | ||
| } as any) | ||
|
|
||
| const result = setPresenceActivity(client, 100) | ||
|
|
||
| expect(setPresence).toHaveBeenCalled() | ||
| expect(result).toBeLessThan(5) | ||
| }) | ||
|
|
||
| it('should use getBotPresenceStatus', () => { | ||
| const setPresence = jest.fn() | ||
| const client = createMockClient({ | ||
| user: { setPresence }, | ||
| guilds: { cache: { size: 5, values: () => [] } }, | ||
| commands: { size: 30 }, | ||
| } as any) | ||
|
|
||
| setPresenceActivity(client, 0) | ||
|
|
||
| expect(mockGetBotPresenceStatus).toHaveBeenCalled() | ||
| }) | ||
| }) | ||
|
|
||
| describe('startPresenceRotation', () => { | ||
| beforeEach(() => { | ||
| jest.useFakeTimers() | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| jest.useRealTimers() | ||
| }) | ||
|
|
||
| it('should return controls object', () => { | ||
| const client = createMockClient({ | ||
| user: { setPresence: jest.fn() }, | ||
| guilds: { cache: { size: 5, values: () => [] } }, | ||
| commands: { size: 30 }, | ||
| } as any) | ||
|
|
||
| const controls = startPresenceRotation(client) | ||
|
|
||
| expect(controls).toHaveProperty('stop') | ||
| expect(controls).toHaveProperty('pause') | ||
| expect(controls).toHaveProperty('resume') | ||
| expect(typeof controls.stop).toBe('function') | ||
| expect(typeof controls.pause).toBe('function') | ||
| expect(typeof controls.resume).toBe('function') | ||
|
|
||
| controls.stop() | ||
| }) | ||
|
|
||
| it('should set presence immediately', () => { | ||
| const setPresence = jest.fn() | ||
| const client = createMockClient({ | ||
| user: { setPresence }, | ||
| guilds: { cache: { size: 5, values: () => [] } }, | ||
| commands: { size: 30 }, | ||
| } as any) | ||
|
|
||
| const controls = startPresenceRotation(client) | ||
|
|
||
| expect(setPresence).toHaveBeenCalled() | ||
|
|
||
| controls.stop() | ||
| }) | ||
|
|
||
| it('should rotate presence on interval', () => { | ||
| const setPresence = jest.fn() | ||
| const client = createMockClient({ | ||
| user: { setPresence }, | ||
| guilds: { cache: { size: 5, values: () => [] } }, | ||
| commands: { size: 30 }, | ||
| } as any) | ||
|
|
||
| const controls = startPresenceRotation(client) | ||
|
|
||
| setPresence.mockClear() | ||
|
|
||
| jest.advanceTimersByTime(PRESENCE_ROTATION_INTERVAL_MS) | ||
| expect(setPresence).toHaveBeenCalledTimes(1) | ||
|
|
||
| jest.advanceTimersByTime(PRESENCE_ROTATION_INTERVAL_MS) | ||
| expect(setPresence).toHaveBeenCalledTimes(2) | ||
|
|
||
| controls.stop() | ||
| }) | ||
|
|
||
| it('should pause rotation', () => { | ||
| const setPresence = jest.fn() | ||
| const client = createMockClient({ | ||
| user: { setPresence }, | ||
| guilds: { cache: { size: 5, values: () => [] } }, | ||
| commands: { size: 30 }, | ||
| } as any) | ||
|
|
||
| const controls = startPresenceRotation(client) | ||
| setPresence.mockClear() | ||
|
|
||
| controls.pause() | ||
|
|
||
| jest.advanceTimersByTime(PRESENCE_ROTATION_INTERVAL_MS * 3) | ||
| expect(setPresence).not.toHaveBeenCalled() | ||
|
|
||
| controls.stop() | ||
| }) | ||
|
|
||
| it('should resume rotation after pause', () => { | ||
| const setPresence = jest.fn() | ||
| const client = createMockClient({ | ||
| user: { setPresence }, | ||
| guilds: { cache: { size: 5, values: () => [] } }, | ||
| commands: { size: 30 }, | ||
| } as any) | ||
|
|
||
| const controls = startPresenceRotation(client) | ||
| setPresence.mockClear() | ||
|
|
||
| controls.pause() | ||
| jest.advanceTimersByTime(PRESENCE_ROTATION_INTERVAL_MS) | ||
| expect(setPresence).not.toHaveBeenCalled() | ||
|
|
||
| controls.resume() | ||
| expect(setPresence).toHaveBeenCalledTimes(1) | ||
|
|
||
| controls.stop() | ||
| }) | ||
|
|
||
| it('should stop rotation', () => { | ||
| const setPresence = jest.fn() | ||
| const client = createMockClient({ | ||
| user: { setPresence }, | ||
| guilds: { cache: { size: 5, values: () => [] } }, | ||
| commands: { size: 30 }, | ||
| } as any) | ||
|
|
||
| const controls = startPresenceRotation(client) | ||
| setPresence.mockClear() | ||
|
|
||
| controls.stop() | ||
|
|
||
| jest.advanceTimersByTime(PRESENCE_ROTATION_INTERVAL_MS * 3) | ||
| expect(setPresence).not.toHaveBeenCalled() | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Split this spec file to comply with enforced size limits.
At 437 lines, this exceeds the repository’s enforced 250-line file limit. Please split into focused suites (e.g., index/math helpers, activity builder, rotation controls).
As per coding guidelines, **/*.{ts,tsx,js,jsx}: Files must not exceed 250 lines and this is enforced.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 42-45: This assertion is unnecessary since it does not change the type of the expression.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/handlers/clientHandler/presence.spec.ts` around lines 1 -
437, The spec file is too large (>250 lines); split tests into multiple focused
spec files so each is under the limit. Create separate test files for: 1)
math/helpers (nextPresenceIndex, PRESENCE_ROTATION_INTERVAL_MS), 2) guild/player
helpers (getTotalMemberCount, getActiveMusicSessions), 3) activity builder
(buildPresenceActivities), and 4) presence control logic (setPresenceActivity,
startPresenceRotation) — move relevant describe blocks into those files,
preserve the shared mocks (mockGetBotPresenceStatus) or re-mock it per file, and
update imports to reference the same symbols (nextPresenceIndex,
getTotalMemberCount, getActiveMusicSessions, buildPresenceActivities,
setPresenceActivity, startPresenceRotation) so tests run unchanged but each file
stays under 250 lines.
| import { describe, it, expect, beforeEach, jest } from '@jest/globals' | ||
| import { Client, Collection } from 'discord.js' | ||
| import { createClient, startClient } from './service' | ||
|
|
||
| jest.mock('@lucky/shared/utils', () => ({ | ||
| debugLog: jest.fn(), | ||
| infoLog: jest.fn(), | ||
| errorLog: jest.fn(), | ||
| })) | ||
|
|
||
| jest.mock('@lucky/shared/config', () => ({ | ||
| config: jest.fn().mockReturnValue({ | ||
| TOKEN: 'test-token', | ||
| CLIENT_ID: 'test-client-id', | ||
| }), | ||
| })) | ||
|
|
||
| jest.mock('./presence', () => ({ | ||
| startPresenceRotation: jest.fn().mockReturnValue({ | ||
| stop: jest.fn(), | ||
| pause: jest.fn(), | ||
| resume: jest.fn(), | ||
| }), | ||
| })) | ||
|
|
||
| jest.mock('../../services/MusicPresenceService', () => ({ | ||
| initMusicPresence: jest.fn(), | ||
| })) | ||
|
|
||
| jest.mock('discord.js', () => { | ||
| const originalModule = | ||
| jest.requireActual<typeof import('discord.js')>('discord.js') | ||
| return { | ||
| ...originalModule, | ||
| Client: jest.fn().mockImplementation(() => ({ | ||
| commands: new originalModule.Collection(), | ||
| login: jest.fn().mockResolvedValue('client'), | ||
| once: jest.fn(), | ||
| guilds: { | ||
| cache: { | ||
| values: jest.fn().mockReturnValue([]), | ||
| }, | ||
| }, | ||
| })), | ||
| REST: jest.fn().mockImplementation(() => ({ | ||
| setToken: jest.fn().mockReturnThis(), | ||
| put: jest.fn().mockResolvedValue(undefined), | ||
| })), | ||
| } | ||
| }) | ||
|
|
||
| import { debugLog, infoLog, errorLog } from '@lucky/shared/utils' | ||
| import { config } from '@lucky/shared/config' | ||
|
|
||
| describe('service', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks() | ||
| ;(config as jest.Mock).mockReturnValue({ | ||
| TOKEN: 'test-token', | ||
| CLIENT_ID: 'test-client-id', | ||
| }) | ||
| }) | ||
|
|
||
| describe('createClient', () => { | ||
| it('should create a Discord client successfully', async () => { | ||
| const client = await createClient() | ||
|
|
||
| expect(Client).toHaveBeenCalledWith({ | ||
| intents: expect.arrayContaining([expect.any(Number)]), | ||
| }) | ||
| expect(client.commands).toBeInstanceOf(Collection) | ||
| expect(debugLog).toHaveBeenCalledWith({ | ||
| message: 'Discord client created successfully', | ||
| }) | ||
| }) | ||
|
|
||
| it('should throw when TOKEN is missing', async () => { | ||
| ;(config as jest.Mock).mockReturnValue({ | ||
| TOKEN: '', | ||
| CLIENT_ID: 'test-client-id', | ||
| }) | ||
|
|
||
| await expect(createClient()).rejects.toThrow( | ||
| 'DISCORD_TOKEN or CLIENT_ID not configured', | ||
| ) | ||
| }) | ||
|
|
||
| it('should throw when CLIENT_ID is missing', async () => { | ||
| ;(config as jest.Mock).mockReturnValue({ | ||
| TOKEN: 'test-token', | ||
| CLIENT_ID: '', | ||
| }) | ||
|
|
||
| await expect(createClient()).rejects.toThrow( | ||
| 'DISCORD_TOKEN or CLIENT_ID not configured', | ||
| ) | ||
| }) | ||
|
|
||
| it('should log error when client creation fails', async () => { | ||
| const error = new Error('Creation failed') | ||
| ;(Client as unknown as jest.Mock).mockImplementationOnce(() => { | ||
| throw error | ||
| }) | ||
|
|
||
| await expect(createClient()).rejects.toThrow('Creation failed') | ||
|
|
||
| expect(errorLog).toHaveBeenCalledWith({ | ||
| message: 'Error creating Discord client:', | ||
| error, | ||
| }) | ||
| }) | ||
|
|
||
| it('should set player to null initially', async () => { | ||
| const client = await createClient() | ||
|
|
||
| expect(client.player).toBeNull() | ||
| }) | ||
| }) | ||
|
|
||
| describe('startClient', () => { | ||
| it('should throw when TOKEN is missing', async () => { | ||
| ;(config as jest.Mock).mockReturnValue({ | ||
| TOKEN: '', | ||
| CLIENT_ID: 'test-client-id', | ||
| }) | ||
|
|
||
| const mockClient = { | ||
| login: jest.fn(), | ||
| } | ||
|
|
||
| await expect( | ||
| startClient({ client: mockClient as any }), | ||
| ).rejects.toThrow('DISCORD_TOKEN or CLIENT_ID not configured') | ||
| }) | ||
|
|
||
| it('should throw when CLIENT_ID is missing', async () => { | ||
| ;(config as jest.Mock).mockReturnValue({ | ||
| TOKEN: 'test-token', | ||
| CLIENT_ID: '', | ||
| }) | ||
|
|
||
| const mockClient = { | ||
| login: jest.fn(), | ||
| } | ||
|
|
||
| await expect( | ||
| startClient({ client: mockClient as any }), | ||
| ).rejects.toThrow('DISCORD_TOKEN or CLIENT_ID not configured') | ||
| }) | ||
|
|
||
| it('should call login with token', async () => { | ||
| const mockClient = { | ||
| login: jest.fn().mockResolvedValue('client'), | ||
| once: jest.fn((event, handler) => { | ||
| if (event === 'ready') { | ||
| Promise.resolve().then(() => handler()) | ||
| } | ||
| }), | ||
| user: null, | ||
| commands: { | ||
| map: jest.fn().mockReturnValue([]), | ||
| }, | ||
| guilds: { | ||
| cache: { | ||
| values: jest.fn().mockReturnValue([]), | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| const startPromise = startClient({ client: mockClient as any }) | ||
|
|
||
| await new Promise((resolve) => setImmediate(resolve)) | ||
|
|
||
| expect(mockClient.login).toHaveBeenCalledWith('test-token') | ||
|
|
||
| await startPromise | ||
| }) | ||
|
|
||
| it('should register ready event handler', async () => { | ||
| const mockClient = { | ||
| login: jest.fn().mockResolvedValue('client'), | ||
| once: jest.fn((event, handler) => { | ||
| if (event === 'ready') { | ||
| Promise.resolve().then(() => handler()) | ||
| } | ||
| }), | ||
| user: null, | ||
| commands: { | ||
| map: jest.fn().mockReturnValue([]), | ||
| }, | ||
| guilds: { | ||
| cache: { | ||
| values: jest.fn().mockReturnValue([]), | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| const startPromise = startClient({ client: mockClient as any }) | ||
|
|
||
| await new Promise((resolve) => setImmediate(resolve)) | ||
|
|
||
| expect(mockClient.once).toHaveBeenCalledWith( | ||
| 'ready', | ||
| expect.any(Function), | ||
| ) | ||
|
|
||
| await startPromise | ||
| }) | ||
|
|
||
| it('should skip presence setup when user is null', async () => { | ||
| const { startPresenceRotation } = await import('./presence') | ||
|
|
||
| const mockClient = { | ||
| login: jest.fn().mockResolvedValue('client'), | ||
| once: jest.fn((event, handler) => { | ||
| if (event === 'ready') { | ||
| Promise.resolve().then(() => handler()) | ||
| } | ||
| }), | ||
| user: null, | ||
| commands: { | ||
| map: jest.fn().mockReturnValue([]), | ||
| }, | ||
| guilds: { | ||
| cache: { | ||
| values: jest.fn().mockReturnValue([]), | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| ;(startPresenceRotation as jest.Mock).mockClear() | ||
|
|
||
| const startPromise = startClient({ client: mockClient as any }) | ||
| await new Promise((resolve) => setImmediate(resolve)) | ||
| await startPromise | ||
|
|
||
| expect(startPresenceRotation).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it('should handle errors in ready handler gracefully', async () => { | ||
| const mockClient = { | ||
| login: jest.fn().mockResolvedValue('client'), | ||
| once: jest.fn((event, handler) => { | ||
| if (event === 'ready') { | ||
| Promise.resolve().then(() => handler()) | ||
| } | ||
| }), | ||
| user: null, | ||
| commands: { | ||
| map: jest.fn().mockImplementation(() => { | ||
| throw new Error('Test error') | ||
| }), | ||
| }, | ||
| guilds: { | ||
| cache: { | ||
| values: jest.fn().mockReturnValue([]), | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| const startPromise = startClient({ client: mockClient as any }) | ||
| await new Promise((resolve) => setImmediate(resolve)) | ||
| await startPromise | ||
|
|
||
| expect(errorLog).toHaveBeenCalledWith({ | ||
| message: 'Error in ready handler:', | ||
| error: expect.any(Error), | ||
| }) | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
This spec exceeds the enforced per-file size limit.
At 271 lines, this violates the 250-line maximum. Split by behavior (createClient vs startClient) to keep maintenance and CI checks clean.
As per coding guidelines, **/*.{ts,tsx,js,jsx}: Files must not exceed 250 lines and this is enforced.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 52-52: Remove this unused import of 'infoLog'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/handlers/clientHandler/service.spec.ts` around lines 1 -
271, The test file exceeds the 250-line limit; split the spec into two smaller
files by behavior: extract the describe('createClient', ...) block (tests
referencing createClient, Client, errorLog, debugLog, Collection) into a new
spec (e.g., service.createClient.spec.ts) and keep describe('startClient', ...)
(tests referencing startClient, startPresenceRotation, once/login, errorLog) in
the original or a new service.startClient.spec.ts; update imports/mocks at top
of each new file so each contains only the mocks and imports needed for its
tests (retain mocks for '@lucky/shared/utils', '@lucky/shared/config',
'./presence', '../../services/MusicPresenceService', and the discord.js mock
where required), run tests to ensure each file is under 250 lines and CI passes.
| } | ||
| }) | ||
|
|
||
| import { debugLog, infoLog, errorLog } from '@lucky/shared/utils' |
There was a problem hiding this comment.
Drop the unused infoLog import.
infoLog is imported but never referenced.
As per coding guidelines, **/*.{ts,tsx,js,jsx}: Remove dead code and unused imports.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 52-52: Remove this unused import of 'infoLog'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/handlers/clientHandler/service.spec.ts` at line 52, The
import list in the test file includes an unused symbol; remove the unused
infoLog import from the import statement that currently reads "import {
debugLog, infoLog, errorLog } from '@lucky/shared/utils'" so only referenced
utilities (e.g., debugLog and errorLog) remain; update the import to drop
infoLog to eliminate the dead import.
| ;(Client as unknown as jest.Mock).mockImplementationOnce(() => { | ||
| throw error | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check if the file exists and read the specific lines mentioned
FILE="packages/bot/src/handlers/clientHandler/service.spec.ts"
if [ -f "$FILE" ]; then
echo "=== File size ===" && wc -l "$FILE"
echo -e "\n=== Lines 101-103 ===" && sed -n '101,103p' "$FILE"
echo -e "\n=== Lines 130-135 ===" && sed -n '130,135p' "$FILE"
echo -e "\n=== Lines 145-150 ===" && sed -n '145,150p' "$FILE"
echo -e "\n=== Lines 168-172 ===" && sed -n '168,172p' "$FILE"
echo -e "\n=== Lines 196-200 ===" && sed -n '196,200p' "$FILE"
echo -e "\n=== Lines 231-235 ===" && sed -n '231,235p' "$FILE"
echo -e "\n=== Lines 259-263 ===" && sed -n '259,263p' "$FILE"
else
echo "File not found: $FILE"
fiRepository: LucasSantana-Dev/Lucky
Length of output: 1512
Remove any casts throughout the test file.
The file contains multiple mockClient as any casts that weaken type safety and bypass API contract checks. These violate the strict typing requirement enforced by ESLint.
Affected locations
- Line 132:
mockClient as any - Line 147:
mockClient as any - Line 170:
mockClient as any - Line 198:
mockClient as any - Line 233:
mockClient as any - Line 261:
mockClient as any
Use unknown with type guards instead, as demonstrated correctly in line 101.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/handlers/clientHandler/service.spec.ts` around lines 101 -
103, Replace all `mockClient as any` casts in service.spec.ts with `unknown` and
assert types via explicit type guards or jest.Mocked utilities; locate uses of
`mockClient` (at the listed lines) and change the cast pattern to `(mockClient
as unknown)` then narrow it using a runtime/type-guard check or wrap with
`jest.Mocked<typeof Client>`/`jest.Mocked<Client>` where appropriate, following
the existing example at the `;(Client as unknown as
jest.Mock).mockImplementationOnce` line to preserve type safety without using
`any`.
| it('should define YouTube URL detection logic', () => { | ||
| const isYouTubeUrl = (url: string): boolean => | ||
| url.includes('youtube.com') || url.includes('youtu.be') | ||
|
|
||
| expect(isYouTubeUrl('https://www.youtube.com/watch?v=test')).toBe( | ||
| true, | ||
| ) | ||
| expect(isYouTubeUrl('https://youtu.be/test')).toBe(true) | ||
| expect(isYouTubeUrl('https://soundcloud.com/track')).toBe(false) | ||
| }) | ||
|
|
||
| it('should validate player creation parameters', () => { | ||
| type CreatePlayerParams = { | ||
| client: unknown | ||
| } | ||
|
|
||
| const validateParams = (params: CreatePlayerParams): boolean => { | ||
| return params && params.client !== undefined | ||
| } | ||
|
|
||
| expect(validateParams({ client: {} })).toBe(true) | ||
| expect(validateParams({ client: null } as any)).toBe(true) | ||
| }) | ||
| }) | ||
|
|
||
| describe('YouTube extractor configuration', () => { | ||
| it('should configure extractor options correctly', () => { | ||
| const extractorOptions = { | ||
| streamOptions: { | ||
| useClient: 'IOS' as const, | ||
| highWaterMark: 1 << 25, | ||
| }, | ||
| } | ||
|
|
||
| expect(extractorOptions.streamOptions.useClient).toBe('IOS') | ||
| expect(extractorOptions.streamOptions.highWaterMark).toBe(33554432) | ||
| }) | ||
|
|
||
| it('should define correct water mark calculation', () => { | ||
| const highWaterMark = 1 << 25 | ||
| expect(highWaterMark).toBe(33554432) | ||
| expect(highWaterMark).toBe(Math.pow(2, 25)) | ||
| }) | ||
| }) | ||
|
|
||
| describe('yt-dlp integration', () => { | ||
| it('should define yt-dlp command arguments', () => { | ||
| const ytDlpArgs = [ | ||
| '-f', | ||
| 'bestaudio/best', | ||
| '-o', | ||
| '-', | ||
| '--no-warnings', | ||
| '--quiet', | ||
| ] | ||
|
|
||
| expect(ytDlpArgs).toContain('-f') | ||
| expect(ytDlpArgs).toContain('bestaudio/best') | ||
| expect(ytDlpArgs).toContain('--no-warnings') | ||
| expect(ytDlpArgs).toContain('--quiet') | ||
| expect(ytDlpArgs).toHaveLength(6) | ||
| }) | ||
|
|
||
| it('should configure spawn options for yt-dlp', () => { | ||
| const spawnOptions = { | ||
| stdio: ['ignore', 'pipe', 'pipe'] as const, | ||
| } | ||
|
|
||
| expect(spawnOptions.stdio[0]).toBe('ignore') | ||
| expect(spawnOptions.stdio[1]).toBe('pipe') | ||
| expect(spawnOptions.stdio[2]).toBe('pipe') | ||
| }) | ||
|
|
||
| it('should configure stream types correctly', () => { | ||
| type StreamType = 'ignore' | 'pipe' | ||
| const stdin: StreamType = 'ignore' | ||
| const stdout: StreamType = 'pipe' | ||
| const stderr: StreamType = 'pipe' | ||
|
|
||
| expect(stdin).toBe('ignore') | ||
| expect(stdout).toBe('pipe') | ||
| expect(stderr).toBe('pipe') | ||
| }) | ||
| }) | ||
|
|
||
| describe('error handling', () => { | ||
| it('should define error handling patterns', () => { | ||
| const handleError = (error: Error): void => { | ||
| expect(error).toBeInstanceOf(Error) | ||
| } | ||
|
|
||
| const testError = new Error('Test error') | ||
| handleError(testError) | ||
| }) | ||
|
|
||
| it('should handle process errors', () => { | ||
| type ProcessError = { | ||
| code?: string | ||
| signal?: string | ||
| } | ||
|
|
||
| const isProcessError = (err: unknown): err is ProcessError => { | ||
| return ( | ||
| typeof err === 'object' && | ||
| err !== null && | ||
| ('code' in err || 'signal' in err) | ||
| ) | ||
| } | ||
|
|
||
| expect(isProcessError({ code: 'ENOENT' })).toBe(true) | ||
| expect(isProcessError({ signal: 'SIGTERM' })).toBe(true) | ||
| expect(isProcessError({})).toBe(false) | ||
| }) | ||
| }) | ||
|
|
||
| describe('max listeners configuration', () => { | ||
| it('should define max listeners value', () => { | ||
| const maxListeners = 20 | ||
| expect(maxListeners).toBe(20) | ||
| expect(maxListeners).toBeGreaterThan(0) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Tests re-implement internals instead of testing playerFactory.ts.
This suite validates local constants/helpers, not exported runtime behavior.
Example: Line 52-59 defines yt-dlp args without the URL argument, while packages/bot/src/handlers/player/playerFactory.ts appends the URL. These assertions won’t catch regressions in real code paths.
As per coding guidelines, **/*.{spec,test}.{ts,tsx,js,jsx}: Test behavior, not implementation details.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 22-22: Prefer using an optional chain expression instead, as it's more concise and easier to read.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/handlers/player/playerFactory.spec.ts` around lines 5 - 126,
Tests are re-implementing internals instead of exercising the real exports from
playerFactory.ts; replace the inline/local constants and helpers (e.g., the
local isYouTubeUrl, validateParams, extractorOptions, ytDlpArgs, spawnOptions,
handleError, isProcessError, maxListeners) with imports from
packages/bot/src/handlers/player/playerFactory.ts and assert the actual runtime
behavior (for example import the exported ytDlpArgs or a function that builds
the yt-dlp args and assert it appends the URL, import the exported createPlayer
or param validator and test real validation, import the extractor
options/highWaterMark constant and test its value, and import the process error
type guard and test real behavior). Locate usages by symbol names in the file
(isYouTubeUrl, validateParams, extractorOptions, ytDlpArgs, spawnOptions,
handleError, isProcessError, maxListeners, createPlayer) and replace the
duplicated definitions with direct imports and tests against those exports so
the suite verifies public behavior rather than local redefinitions.
| } | ||
|
|
||
| expect(validateParams({ client: {} })).toBe(true) | ||
| expect(validateParams({ client: null } as any)).toBe(true) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "playerFactory.spec.ts" | head -20Repository: LucasSantana-Dev/Lucky
Length of output: 122
🏁 Script executed:
cat -n ./packages/bot/src/handlers/player/playerFactory.spec.tsRepository: LucasSantana-Dev/Lucky
Length of output: 5270
Remove the unnecessary as any cast.
Line 26 uses as any on { client: null }, which bypasses type safety. The object already satisfies the CreatePlayerParams type (where client: unknown), so the cast is unnecessary. Pass the object directly: expect(validateParams({ client: null })).toBe(true).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/handlers/player/playerFactory.spec.ts` at line 26, The test
uses an unnecessary type cast; remove the "as any" on the object passed to
validateParams in playerFactory.spec.ts. Update the assertion to call
validateParams({ client: null }) directly (the object already matches
CreatePlayerParams where client: unknown) so the test uses real typing rather
than bypassing it; keep the expect(validateParams(...)).toBe(true) assertion and
do not change the validateParams function itself.
| it('should define createPlayer function signature', () => { | ||
| type CustomClient = unknown | ||
| type Player = unknown | ||
| type CreatePlayerParams = { client: CustomClient } | ||
| type CreatePlayerFunction = (params: CreatePlayerParams) => Player | ||
|
|
||
| const mockCreatePlayer: CreatePlayerFunction = (params) => { | ||
| expect(params).toHaveProperty('client') | ||
| return {} as Player | ||
| } | ||
|
|
||
| const result = mockCreatePlayer({ client: {} }) | ||
| expect(result).toBeDefined() | ||
| }) | ||
|
|
||
| it('should define TrackHistoryEntry type', () => { | ||
| type TrackHistoryEntry = { | ||
| trackId: string | ||
| title: string | ||
| guildId: string | ||
| timestamp: number | ||
| } | ||
|
|
||
| const entry: TrackHistoryEntry = { | ||
| trackId: 'track-1', | ||
| title: 'Test Song', | ||
| guildId: 'guild-1', | ||
| timestamp: Date.now(), | ||
| } | ||
|
|
||
| expect(entry.trackId).toBe('track-1') | ||
| expect(entry.title).toBe('Test Song') | ||
| }) | ||
|
|
||
| it('should define lastPlayedTracks map structure', () => { | ||
| type TrackMap = Map<string, unknown> | ||
| const lastPlayedTracks: TrackMap = new Map() | ||
|
|
||
| expect(lastPlayedTracks).toBeInstanceOf(Map) | ||
| expect(lastPlayedTracks.size).toBe(0) | ||
| }) | ||
|
|
||
| it('should define recentlyPlayedTracks array structure', () => { | ||
| type TrackArray = unknown[] | ||
| const recentlyPlayedTracks: TrackArray = [] | ||
|
|
||
| expect(Array.isArray(recentlyPlayedTracks)).toBe(true) | ||
| expect(recentlyPlayedTracks.length).toBe(0) | ||
| }) |
There was a problem hiding this comment.
This suite is not validating real module behavior.
The tests only assert locally-declared types/collections and never import or execute actual player handler exports.
Also, Line 20-33 defines a TrackHistoryEntry shape that conflicts with packages/bot/src/handlers/player/trackHandlers.ts (which uses url/author/thumbnail/timestamp), and Line 47-53 expects an array while recentlyPlayedTracks is a Map. This can pass while production contracts are broken.
As per coding guidelines, **/*.{spec,test}.{ts,tsx,js,jsx}: Test behavior, not implementation details.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 7-7: Remove this redundant type alias and replace its occurrences with "unknown".
[warning] 6-6: Remove this redundant type alias and replace its occurrences with "unknown".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/handlers/playerHandler.spec.ts` around lines 5 - 53, Replace
the local, dummy type checks with real imports and behavior assertions: import
the actual player handler exports (e.g., createPlayer, recentlyPlayedTracks,
lastPlayedTracks) and the track handlers/types from the production module
(reference trackHandlers.ts shape: url, author, thumbnail, timestamp) and update
the tests to validate the real exported structures and values (assert that
recentlyPlayedTracks is a Map with expected entries, lastPlayedTracks is a Map,
and TrackHistoryEntry matches the production shape including
url/author/thumbnail/timestamp) rather than asserting locally-declared
placeholder types.
| it('should include details in ValidationError', async () => { | ||
| const interaction = createMockInteraction({ guild: null }) | ||
| const client = createMockClient() | ||
|
|
||
| try { | ||
| await createQueue({ client, interaction }) | ||
| } catch (error: any) { | ||
| expect(error.name).toBe('ValidationError') | ||
| expect(error.details).toBeDefined() | ||
| expect(error.details.userId).toBe('user-1') | ||
| expect(error.details.channelId).toBe('channel-1') | ||
| } | ||
| }) |
There was a problem hiding this comment.
These async error tests can pass even when no error is thrown.
The try/catch blocks have no guaranteed failure on the non-throw path (except one case at Line 213). Use await expect(...).rejects with shape assertions to make failures deterministic.
As per coding guidelines, **/*.{spec,test}.{ts,tsx,js,jsx}: Test behavior, not implementation details.
Also applies to: 116-128, 130-142, 225-235
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/handlers/queueHandler.spec.ts` around lines 102 - 114, The
tests in queueHandler.spec.ts use try/catch blocks that allow silent success
(tests pass if no error is thrown); replace those with deterministic assertions
using Jest's async reject matcher—call createQueue({ client, interaction })
inside await expect(...).rejects and assert the error shape (e.g.,
expect.objectContaining({ name: 'ValidationError', details:
expect.objectContaining({ userId: 'user-1', channelId: 'channel-1' }) })) so
failures are caught; apply the same change to the other similar test blocks
referenced (around the ranges that include lines 116-128, 130-142, and 225-235)
and reference the createQueue invocation and error.details checks when updating
each test.
| const refreshButtons = document.querySelectorAll('button') | ||
| const refreshButton = Array.from(refreshButtons).find((btn) => | ||
| btn.querySelector('svg'), | ||
| ) | ||
|
|
||
| if (refreshButton) { | ||
| await user.click(refreshButton) | ||
|
|
||
| await waitFor(() => { | ||
| expect(api.automation.getStatus).toHaveBeenCalled() | ||
| expect(api.automation.getManifest).toHaveBeenCalled() | ||
| }) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Fragile refresh button query may silently pass if button is missing.
The DOM query document.querySelectorAll('button') with SVG filtering is implementation-coupled, and the if (refreshButton) conditional allows the test to pass silently if the button isn't found. Use an accessible query with aria-label instead.
🛠️ Proposed fix using accessible query
- const refreshButtons = document.querySelectorAll('button')
- const refreshButton = Array.from(refreshButtons).find((btn) =>
- btn.querySelector('svg'),
- )
-
- if (refreshButton) {
- await user.click(refreshButton)
-
- await waitFor(() => {
- expect(api.automation.getStatus).toHaveBeenCalled()
- expect(api.automation.getManifest).toHaveBeenCalled()
- })
- }
+ const refreshButton = screen.getByRole('button', { name: /refresh/i })
+ await user.click(refreshButton)
+
+ await waitFor(() => {
+ expect(api.automation.getStatus).toHaveBeenCalled()
+ expect(api.automation.getManifest).toHaveBeenCalled()
+ })Note: If the component's refresh button lacks an accessible label, consider adding aria-label="Refresh" to the button in the component itself for both testability and accessibility.
📝 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.
| const refreshButtons = document.querySelectorAll('button') | |
| const refreshButton = Array.from(refreshButtons).find((btn) => | |
| btn.querySelector('svg'), | |
| ) | |
| if (refreshButton) { | |
| await user.click(refreshButton) | |
| await waitFor(() => { | |
| expect(api.automation.getStatus).toHaveBeenCalled() | |
| expect(api.automation.getManifest).toHaveBeenCalled() | |
| }) | |
| } | |
| }) | |
| const refreshButton = screen.getByRole('button', { name: /refresh/i }) | |
| await user.click(refreshButton) | |
| await waitFor(() => { | |
| expect(api.automation.getStatus).toHaveBeenCalled() | |
| expect(api.automation.getManifest).toHaveBeenCalled() | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/frontend/src/pages/GuildAutomation.test.tsx` around lines 563 - 576,
The test currently finds the refresh button using a fragile DOM traversal
(document.querySelectorAll('button') + svg) and then guards with an if, which
can let the test pass silently; replace that with an accessible query (e.g., use
getByRole('button', { name: /refresh/i }) or getByLabelText('Refresh') from your
testing library) to explicitly locate the button and remove the conditional so
the test fails if not found, then call user.click on that retrieved element and
keep the existing waitFor assertions (api.automation.getStatus /
api.automation.getManifest); if the component lacks an accessible name, add
aria-label="Refresh" to the refresh button in the component to make it testable
and accessible.
This branch was successfully deployed
1 participant
Summary
Test Coverage Details
EmbedBuilder.test.tsx (16 tests)
GuildAutomation.test.tsx (21 tests)
Test Strategy
🤖 Generated with Claude Code
Summary by CodeRabbit