test: improve coverage for transaction_controller.go #238
Conversation
antoinegelloz
force-pushed
the
num-569-transaction_controllergo
branch
from
caae113 to
405aed6
Compare
Codecov Report
@@ Coverage Diff @@
## main #238 +/- ##
==========================================
+ Coverage 67.16% 69.25% +2.09%
==========================================
Files 72 73 +1
Lines 3816 3926 +110
==========================================
+ Hits 2563 2719 +156
+ Misses 1001 966 -35
+ Partials 252 241 -11
Continue to review full report at Codecov.
|
pkg/ledger/ledger.go
Outdated
| if err != nil { | ||
| return core.Transaction{}, err | ||
| } | ||
| if len(tx.Postings) == 0 { |
There was a problem hiding this comment.
I think we should modify the storage implementation to return an error if the transaction is not found.
There was a problem hiding this comment.
I changed the signature of the store method, it now returns *core.Transaction, with a nil response if the transaction isn't found. Is it better ?
| return | ||
| } | ||
|
|
||
| if err := l.(*ledger.Ledger).SaveMeta(c.Request.Context(), core.MetaTargetTypeTransaction, txId, m); err != nil { | ||
| _, err = l.(*ledger.Ledger).GetTransaction(c.Request.Context(), txId) |
There was a problem hiding this comment.
It is a breaking change. We need to validate this.
There was a problem hiding this comment.
what are we doing here exactly ? i'm not sure i understand
There was a problem hiding this comment.
Before this changes, we can add a metadata on a transaction while it is not existings.
It is ok on accounts, but not on a transaction. It is more an unexpected behavior than a breaking changes finally.
There was a problem hiding this comment.
I agree :) It is clear now @jdupas22?
No description provided.