test: improve coverage for token/token package (#1348)#1478
test: improve coverage for token/token package (#1348)#1478nishantbkl3345-ship-it wants to merge 4 commits intohyperledger-labs:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR increases unit test coverage for the token/token package (targeting ~93% coverage) by adding missing tests for core token structs and quantity edge cases.
Changes:
- Added a new
token_test.gocoveringID,LedgerToken, and token collection helpers (Sum,ByType,Count,At). - Expanded
quantity_test.goto coverToBigInt, type-mismatch panics, non-64 precision paths, and negative decimal input handling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
token/token/token_test.go |
New unit tests for token identifiers, ledger token equality, and issued/unspent token collection helpers. |
token/token/quantity_test.go |
Additional unit tests covering ToBigInt, panic behavior on type mismatches, BigQuantity path in UInt64ToQuantity, and negative decimal parsing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
token/token/quantity_test.go
Outdated
| assert.Panics(t, func() { big128.(*token.BigQuantity).Cmp(u64) }) | ||
|
|
||
| // UInt64Quantity methods panic when given BigQuantity | ||
| big128 = token.NewZeroQuantity(128) // fresh instance after mutation |
There was a problem hiding this comment.
The comment "fresh instance after mutation" is misleading here: the preceding Add/Sub/Cmp calls are expected to panic before mutating big128, so there is no mutation to recover from. Consider rewording or removing the comment to reflect what’s actually happening (e.g., "fresh instance" without implying mutation).
| big128 = token.NewZeroQuantity(128) // fresh instance after mutation | |
| big128 = token.NewZeroQuantity(128) // fresh instance |
127e5f7 to
e60cfb3
Compare
|
Opened a PR for this here: #1478 |
|
Hi @nishantbkl3345-ship-it , please, rebase and check the linter errors. You can run the linters with Thanks 🙏 |
0b3e3ac to
b63824f
Compare
f178ac2 to
4651e85
Compare
|
Hi @nishantbkl3345-ship-it , thanks for submitting this. I'll review ASAP. |
b301925 to
91cbc64
Compare
|
Hi @nishantbkl3345-ship-it , please, check again the PR. There are linting issues. We have upgraded golangci-lint version. Thanks 🙏 |
8183729 to
0c2b88c
Compare
…verage Signed-off-by: Nishant <nishantbkl3345@gmail.com>
0c2b88c to
c6d00c1
Compare
|
I checked it again with the updated lint setup and pushed a follow-up fix. I cleaned up the issues in |
Signed-off-by: Nishant <nishantbkl3345@gmail.com>
c1a5eb7 to
42d0b33
Compare
|
Hi @nishantbkl3345-ship-it , please, fix the conflicts. Thanks 🙏 |
|
@nishantbkl3345-ship-it , there is something strange in the PR. There are way too many files changed. Please, have a second look. |
adecaro
left a comment
There was a problem hiding this comment.
Please, double check the PR.
Signed-off-by: Nishant <nishantbkl3345@gmail.com>
Summary
This PR improves unit test coverage for the
token/tokenpackage as part of #1348.What’s included
token_test.gofile to cover logic intoken.goquantity_test.gofor missing paths (e.g.ToBigInt, type mismatch cases, and edge scenarios)Coverage
Notes
I avoided duplicating existing tests and focused only on uncovered or weakly covered areas, mainly in
token.goand a few edge cases inquantity.go.Let me know if you’d like me to adjust or expand any tests.