-
Notifications
You must be signed in to change notification settings - Fork 128
Add unit tests for utility functions (#199) #700
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
base: main
Are you sure you want to change the base?
Conversation
- Add 8 new test cases for simple utility functions - Improve test coverage for core string and number utilities Closes lnp2pBot#199 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
@abcb1122 can you fix the failing checks? |
|
Yes, will check |
- Add eslint-disable no-unused-expressions for Chai property assertions - Apply Prettier auto-formatting (multiline imports, whitespace) - All 107 tests passing
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughA new test suite is added for utility functions covering Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
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 |
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: 0
🧹 Nitpick comments (6)
tests/util/index.spec.ts (6)
11-11: Use consistent ES6 import syntax.The file uses ES6 imports for utility functions (lines 2-9) but CommonJS
requirefor chai. For consistency and modern TypeScript conventions, use ES6 import syntax.Apply this diff:
-const { expect } = require('chai'); +import { expect } from 'chai';
15-22: Consider stronger assertions for currency properties.The test verifies that
resulthassymbolandlocaleproperties but doesn't validate their actual values. For known currencies like USD, consider asserting specific expected values (e.g.,expect(result.symbol).to.equal('$')).
14-41: Consider additional edge cases.The test suite covers basic scenarios but could benefit from testing:
nullorundefinedinputs- Lowercase currency codes (e.g.,
'usd')- Non-string inputs if the function signature allows
49-52: Strengthen assertion to verify actual formatting behavior.The test description states "should return original number if no locale found" but the assertion only checks
to.not.be.false. Consider verifying the actual return value matches the expected format or original number.Example:
- it('should return original number if no locale found', () => { + it('should format number with USD currency', () => { const result = numberFormat('USD', 1234.56); - expect(result).to.not.be.false; + expect(result).to.be.a('string'); + expect(result).to.include('1234'); });
116-126: Consider additional edge cases.The tests cover basic float vs integer distinction. Consider adding tests for:
- Negative floats (e.g.,
-3.14)- Edge case:
1.0(technically a float in JavaScript but represents an integer value)- Special values:
NaN,Infinity,-Infinity
128-138: Consider additional edge cases.The tests cover basic transformations. Consider adding tests for:
- Empty string (
'')- Already kebab-case input (
'kebab-case')- PascalCase (
'PascalCaseString')- Mixed delimiters (
'camelCase_with spaces')- Multiple consecutive delimiters (
'hello world','hello__world')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/util/index.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant
Wrap risky operations in try-catch with meaningful error context
Files:
tests/util/index.spec.ts
tests/**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Write tests in TypeScript using Mocha and Chai with the *.spec.ts naming under tests/
tests/**/*.spec.ts: Write Mocha + Chai specs in tests/** with .spec.ts suffix, mirroring source layout
Prefer unit isolation using proxyquire/sinon in tests
Files:
tests/util/index.spec.ts
tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Mirror runtime code structure under tests/ for quick navigation
Files:
tests/util/index.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Apply Prettier style (2-space indentation, semicolons, single quotes) before committing
Address ESLint Standard + TypeScript rule warnings instead of disabling them
Use camelCase for functions and variables
Use PascalCase for classes
Files:
tests/util/index.spec.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to tests/**/*.spec.ts : Write tests in TypeScript using Mocha and Chai with the *.spec.ts naming under tests/
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to tests/**/*.spec.ts : Write tests in TypeScript using Mocha and Chai with the *.spec.ts naming under tests/
Applied to files:
tests/util/index.spec.ts
📚 Learning: 2025-10-22T14:03:55.114Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-22T14:03:55.114Z
Learning: Applies to tests/**/*.spec.ts : Write Mocha + Chai specs in tests/** with .spec.ts suffix, mirroring source layout
Applied to files:
tests/util/index.spec.ts
📚 Learning: 2025-10-22T14:03:55.114Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-22T14:03:55.114Z
Learning: Applies to tests/**/*.spec.ts : Prefer unit isolation using proxyquire/sinon in tests
Applied to files:
tests/util/index.spec.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to util/**/*.ts : Place shared utilities and helpers under util/
Applied to files:
tests/util/index.spec.ts
⏰ 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). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: ci_to_main
🔇 Additional comments (3)
tests/util/index.spec.ts (3)
54-57: Verify NaN handling is intentional.The test expects
numberFormat('USD', NaN)to returnNaN. Typically, formatting functions handle invalid inputs by returning error indicators or default values rather than propagatingNaN. Confirm this behavior aligns with the function's intended error handling.
103-114: LGTM - Simple and sufficient coverage.The tests appropriately cover the singular and plural cases for this utility function.
76-80: No issues found—test assertions are correct.The
decimalRound(10.995, -2)function correctly returns the numeric value11. The algorithm shifts the decimal, rounds usingMath.round(), then shifts back:1099.5 → 1100 → 11. In JavaScript, this numeric result equals11, making the test assertion valid.
|
@grunch All checks are now passing ✅. I fixed the previous issue with the PR. It's ready for review. |
Summary
plural,isFloat, andtoKebabCaseutility functionsChanges
tests/util/index.spec.tsto include additional utility function testsTest Results
Closes #199
Summary by CodeRabbit