VM: New test for runTX return values #1213
Conversation
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Have updated this branch. |
|
@emersonmacro thanks for the PR, this looks actually pretty good already, great! 😄 Will leave this open for now for some discussion around the |
| res.receipt.gasUsed, | ||
| res.gasUsed.toArrayLike(Buffer), | ||
| `runTx result -> receipt.gasUsed -> result.gasUsed as Buffer (${txType.name})` | ||
| ) |
There was a problem hiding this comment.
There was a problem hiding this comment.
ah yes I think we should update it to return a BN.
in general here I'd say that instead of using t.deepEqual we should use BN's eq method, so for example t.ok(res.gasUsed.eq(new BN(res.receipt.gasUsed)))
I also like to do this with Buffers as well: t.ok(buffer1.equals(buffer2))
There was a problem hiding this comment.
Is this encompassing more checks or stricter or what's the reasoning for this?
Added a new test for default return values in
runTx.spec.ts(testing the non-optional fields inRunTxResult).I'm still getting my head wrapped around the code and testing practices for this project, so feedback is most definitely welcome.