Add tests for partial account helper functions #3745
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Is this AI generated? If so can we please establish the convention (within the whole team) to always clearly mark/label this as such?
If so, we should clearly exchange on some best practices on how to review such code and what expectation we put on it.
If not: my apologies. 😂
Sure, we can block this for now. I used AI to generate the boilerplate and cases, then reviewed, corrected, and filled in missing pieces of the tests to get it to compile 🙂. |
…s/ethereumjs-monorepo into partial-account-function-tests
…to partial-account-function-tests
There was a problem hiding this comment.
The tests themselves look good! However, if you create a partial account from some data and then assert later it is the correct account (with the same, copied data) it is more clean to read these variables from data and then compare those (now the values are copied, so if you change one value you also have to change it on another place. If it's in a const var, only one place needs to be edited)
packages/util/test/account.spec.ts
Outdated
| const account = createPartialAccount({ | ||
| nonce: 1n, | ||
| balance: Units.ether(1), | ||
| storageRoot: KECCAK256_RLP, | ||
| codeHash: KECCAK256_RLP, | ||
| codeSize: 10, | ||
| version: 1, | ||
| }) | ||
|
|
||
| assert.deepEqual(account, new Account(1n, Units.ether(1), KECCAK256_RLP, KECCAK256_RLP, 10, 1)) |
There was a problem hiding this comment.
Imo this could be improved by assigning nonce, balance, etc. to variables, and then testing against that.
There was a problem hiding this comment.
I've pulled out the field data into a const variable and am using that to both create the account and check for equality.
This change adds unit tests testing a variety of cases for the partial account helper functions:
createPartialAccountcreatePartialAccountFromRLPserializeWithPartialInfo