-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: adding test configuration and test coverage for binance plugin #2482
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/plugin-binance/__tests__/account.test.tsOops! Something went wrong! :( ESLint: 9.18.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by 📝 WalkthroughWalkthroughThe pull request introduces comprehensive test suites for the Binance plugin, focusing on three key services: Changes
Possibly related issues
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/plugin-binance/vitest.config.ts (1)
4-16
: Add coverage configuration to track test coverage metrics.Consider adding coverage configuration to track and enforce test coverage thresholds:
export default defineConfig({ test: { globals: true, environment: 'node', - testTimeout: 10000 + testTimeout: 10000, + coverage: { + provider: 'v8', + reporter: ['text', 'json', 'html'], + branches: 80, + functions: 80, + lines: 80, + statements: 80 + } }, resolve: {packages/plugin-binance/__tests__/price.test.ts (1)
69-87
: Add edge cases to price formatting tests.Consider adding tests for:
- Zero values
- Negative numbers
- Scientific notation
- Invalid input handling
describe('formatPrice', () => { + it('should handle edge cases', () => { + expect(PriceService.formatPrice('0')).toBe('0.00'); + expect(PriceService.formatPrice('-42150.25')).toBe('-42,150.25'); + expect(PriceService.formatPrice('1e-8')).toBe('0.00000001'); + expect(() => PriceService.formatPrice('invalid')).toThrow(); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/plugin-binance/__tests__/account.test.ts
(1 hunks)packages/plugin-binance/__tests__/price.test.ts
(1 hunks)packages/plugin-binance/__tests__/trade.test.ts
(1 hunks)packages/plugin-binance/package.json
(1 hunks)packages/plugin-binance/vitest.config.ts
(1 hunks)
🔇 Additional comments (3)
packages/plugin-binance/package.json (3)
23-23
: LGTM! Workspace dependency is correctly configured.The
@elizaos/core
dependency is properly set up as a workspace dependency, which is ideal for monorepo setups.
34-36
: Test scripts follow standard conventions.The scripts are well-organized with both single-run and watch mode options for testing.
27-30
: Test framework dependencies are well-chosen.The combination of Vitest with vite-tsconfig-paths provides a robust testing setup with proper path resolution support.
Let's verify the Vitest configuration:
✅ Verification successful
Test framework setup is properly configured and utilized
- Test files exist in
__tests__
directory with proper naming- Vitest config includes correct environment, timeout, and path resolution settings
- Configuration extends core tsconfig with appropriate compiler options
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Vitest configuration exists and includes necessary settings # Check if vitest config exists fd -t f "vitest.config" packages/plugin-binance/ # Check if tsconfig includes proper test paths fd -t f "tsconfig.json" packages/plugin-binance/ --exec cat {} | jq -r 'select(.compilerOptions.paths != null)'Length of output: 203
Script:
#!/bin/bash # Check configuration files content cat packages/plugin-binance/vitest.config.ts echo "---" cat packages/plugin-binance/tsconfig.json echo "---" # Look for test files fd -e test.ts -e spec.ts . packages/plugin-binance/Length of output: 928
Relates to
#2481
Risks
Low: adding test coverage and configuration
Background
What does this PR do?
Test Structure:
Account Service Tests (account.test.ts)
Trade Service Tests (trade.test.ts)
-Limit order execution with price and time-in-force parameters
-Symbol validation
Comprehensive error handling:
Price Service Tests (price.test.ts)
What kind of change is this?
Feature/tests
#2481
Documentation changes needed?
None
Testing
Where should a reviewer start?
packages/plugin-binance
Detailed testing steps
pnpm install & pnpm build
navigate to desired directory to see tests
run pnpm test