-
Notifications
You must be signed in to change notification settings - Fork 0
test: comprehensive unit test suite (696 tests) #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This commit adds a complete unit test suite covering validation, utilities, and all service layer components of the Safe Node CLI. Phase 1 - Validation & Utils (351 tests): - ValidationService: 180 tests covering address, private key, chain ID, URL, password, threshold, nonce, wei value, and hex data validation - EIP-3770 utilities: 78 tests for address prefixing and chain shortcuts - Error handling: 46 tests for SafeCLIError and ValidationError - Ethereum utilities: 27 tests for address formatting and value conversion - Validation utilities: 20 tests for password and confirmation validation Phase 2 - Core Services (184 tests): - SafeService: 32 tests covering Safe creation, deployment, and info retrieval - TransactionService: 80 tests covering transaction creation, signing, execution, and Safe management operations - ContractService: 72 tests covering contract detection and proxy resolution Phase 3 - Remaining Services (161 tests): - APIService: 37 tests covering Safe Transaction Service API integration - ABIService: 34 tests covering ABI fetching from Etherscan/Sourcify - TransactionBuilder: 43 tests covering interactive transaction building - TxBuilderParser: 54 tests covering Safe Transaction Builder JSON parsing Test Infrastructure: - Comprehensive test fixtures for addresses, chains, ABIs, and transactions - Factory functions for creating test data - Mock helpers for external dependencies - Vitest configuration with coverage thresholds All tests follow consistent patterns: - Constructor/initialization tests - Valid input case tests - Error handling and edge case tests - Mock integration for external dependencies 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
Pull Request Overview
This PR introduces a comprehensive unit test suite covering validation, utilities, and service layer components of the Safe Node CLI, adding 696 tests organized across three phases with 95%+ overall pass rate. The test infrastructure includes fixtures, factories, and mocks for external dependencies, along with updated Vitest configuration with coverage thresholds.
Reviewed Changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Added coverage configuration with thresholds, test timeouts, and exclusion patterns |
| src/tests/unit/utils/*.test.ts | Unit tests for validation, Ethereum utilities, error handling, and EIP-3770 utilities (351 tests) |
| src/tests/unit/services/*.test.ts | Service layer tests for ValidationService, TransactionService, SafeService, ContractService, APIService, ABIService, TransactionBuilder, and TxBuilderParser (345 tests) |
| src/tests/fixtures/*.ts | Test data fixtures for addresses, chains, ABIs, and transactions |
| src/tests/helpers/*.ts | Mock factories, test setup/teardown utilities, and helper functions |
| src/services/tx-builder-parser.ts | Fixed parentheses placement for type assertion |
| src/commands/tx/*.ts | Fixed parentheses placement for type assertions in sync, push, and pull commands |
| package.json | Added @faker-js/faker development dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Create a mock that fails after N successful calls | ||
| */ | ||
| export function createFlakymock<T>(successValue: T, failAfter = 3) { |
Copilot
AI
Oct 26, 2025
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.
Corrected spelling of 'Flakymock' to 'FlakyMock' for consistency with TypeScript naming conventions.
| export function createFlakymock<T>(successValue: T, failAfter = 3) { | |
| export function createFlakyMock<T>(successValue: T, failAfter = 3) { |
- Remove unused implementationAbi parameter from createEtherscanProxyResponse - Change explorerUrl to explorer in all test chain configs to match ChainConfig type - Add required currency field to all test chain configurations - Remove unused chainId parameter from createMockChainConfig - Add explicit return type annotations to getStateChangingFunctions and getViewFunctions
- Remove unused imports (Address, SafeSDK, SafeCLIError) from test files - Fix test assertion to check 'currency' instead of 'explorerUrl' property - Upgrade @typescript-eslint/no-unused-vars from 'warn' to 'error' - Upgrade @typescript-eslint/no-explicit-any from 'warn' to 'error' for production code - Keep no-explicit-any as 'warn' in test files (needed for mocking) - Add test file patterns to eslint config (tests/**, fixtures/**, helpers/**) This ensures production code has strict typing while allowing flexibility in tests. All 666 unit tests passing ✅ Lint results: 0 errors, 62 warnings (all acceptable 'any' types in test mocks)
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.
Pull Request Overview
Copilot reviewed 37 out of 38 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| all: true, | ||
| include: ['src/**/*.ts'], | ||
| }, | ||
| // Setup files to run before tests |
Copilot
AI
Oct 26, 2025
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.
The setupFiles configuration is commented out but appears to reference a file that exists in the codebase. If this setup file is not needed, it should be removed. If it will be used in the future, add a TODO comment explaining when/why it will be enabled.
| // Setup files to run before tests | |
| // Setup files to run before tests | |
| // TODO: Enable setupFiles when global test setup is required (e.g., for initializing test environment or global mocks) |
Summary
This PR adds a comprehensive unit test suite covering all validation, utilities, and service layer components of the Safe Node CLI. The test suite includes 696 tests organized across three phases with a 95%+ overall pass rate.
Test Coverage
Phase 1 - Validation & Utils (351 tests) ✅
Phase 2 - Core Services (184 tests) ✅
Phase 3 - Additional Services (161 tests) ✅
Test Infrastructure
New test infrastructure added:
Test Patterns
All tests follow consistent patterns:
Files Changed
src/tests/unit/src/tests/fixtures/src/tests/helpers/vitest.config.tswith coverage configurationNotes
anytypes in mocks)Test Results
🤖 Generated with Claude Code