Add unit tests for utility functions (#199) #700
Conversation
|
@abcb1122 can you fix the failing checks? |
|
Yes, will check |
WalkthroughAdded a new test suite Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (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. |
|
Hi @abcb1122, can you please sign your commits so we can review and merge your changes? |
- 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 <noreply@anthropic.com>
- Add eslint-disable no-unused-expressions for Chai property assertions - Apply Prettier auto-formatting (multiline imports, whitespace) - All 107 tests passing
abcb1122
force-pushed
the
feature/issue-preparation
branch
from
dc9d01f to
f7d1740
Compare
|
@Luquitasjeffrey Hello, done. |
3 participants
Summary
plural,isFloat, andtoKebabCaseutility functionsChanges
tests/util/index.spec.tsto include additional utility function testsTest Results
Closes #199
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.