Add unit tests for vmhost #945
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds comprehensive unit tests for the vmhost package, specifically focusing on testing various VM hooks, host core functionality, error handling, and async operations. The PR improves test coverage by adding tests for small integer operations, big integer operations, big float operations, breakpoints handling, error wrapping, and async call mechanisms.
Key Changes:
- Added unit tests for VM hooks operations (smallIntOps, bigIntOps, bigFloatOps)
- Added tests for host core breakpoints functionality
- Added comprehensive error wrapping tests
- Updated existing test files to use proper mock interfaces
- Generated new mock files for better test isolation
Reviewed Changes
Copilot reviewed 31 out of 46 changed files in this pull request and generated 18 comments.
Show a summary per file
:
| File | Description |
|---|---|
| vmhost/vmhooks/smallIntOps_test.go | New tests for small integer VM hook operations |
| vmhost/vmhooks/bigIntOps_test.go | New tests for big integer VM hook operations |
| vmhost/vmhooks/bigFloatOps_test.go | New tests for big float VM hook operations |
| vmhost/hostCore/breakpoints_test.go | New tests for breakpoint handling in host core |
| vmhost/errorWrapping_test.go | New comprehensive tests for error wrapping functionality |
| vmhost/errorWrapping.go | Minor refactoring of the Unwrap method |
| vmhost/asyncCall_test.go | New tests for async call functionality |
| vmhost/asyncCallGroup_test.go | New tests for async call group operations |
| vmhost/asyncCall.go | Bug fix for proper cloning of byte slices |
| vmhost/asyncCallGroup.go | Bug fix for proper cloning of callback data |
| mock/context/*.go | Generated mock files for improved test isolation |
Comments suppressed due to low confidence (3)
vmhost/vmhooks/bigFloatOps_test.go:158
- There is an extra closing parenthesis that appears to be a syntax error. This line should be removed.
)
vmhost/vmhooks/smallIntOps_test.go:89
- The test for SmallIntFinishSigned is incomplete. The TODO comment indicates that the test should verify the exact bytes for two's complement representation, but this verification is missing.
// TODO: check the exact bytes for two's complement
vmhost/vmhooks/smallIntOps_test.go:52
- The test case 'out of range' calls the function but doesn't verify the expected behavior. The comment suggests it should 'expect fail execution', but there's no assertion to verify this failure.
hooks.SmallIntGetUnsignedArgument(0)
vmhost/contexts/blockchain_test.go
Outdated
| true true | ||
|
|
There was a problem hiding this comment.
This line contains 'true true' which appears to be a syntax error or incomplete replacement. It should be a proper method call or assignment.
| true true |
vmhost/contexts/blockchain_test.go
Outdated
| // Act as if the OutputContext has the requested OutputAccount cached | ||
| account.Balance = big.NewInt(42) | ||
| mockOutput.OutputAccountIsNew = false | ||
| true false |
There was a problem hiding this comment.
This line contains 'true false' which appears to be a syntax error or incomplete replacement. It should be a proper method call or assignment.
| true false | |
| // No operation needed here; proceeding to test GetBalance |
|
|
||
| metering, _ := NewMeteringContext(host, config.MakeGasMapForTests(), uint64(15000)) | ||
| host.MeteringContext = metering | ||
| host.On("Metering").Return metering |
There was a problem hiding this comment.
Missing parentheses around the return value. It should be 'Return(metering)'.
| host.On("Metering").Return metering | |
| host.On("Metering").Return(metering) |
vmhost/contexts/async_test.go
Outdated
| mockMetering.On("GasProvided").Return = 200 | ||
| mockMetering.On("GasLeft").Return = 60 |
There was a problem hiding this comment.
Incorrect syntax for mock setup. It should be 'Return(200)' instead of 'Return = 200'.
| mockMetering.On("GasProvided").Return = 200 | |
| mockMetering.On("GasLeft").Return = 60 | |
| mockMetering.On("GasProvided").Return(200) | |
| mockMetering.On("GasLeft").Return(60) |
vmhost/contexts/async_test.go
Outdated
| mockMetering.On("GasProvided").Return = 200 | ||
| mockMetering.On("GasLeft").Return = 60 |
There was a problem hiding this comment.
Incorrect syntax for mock setup. It should be 'Return(60)' instead of 'Return = 60'.
| mockMetering.On("GasProvided").Return = 200 | |
| mockMetering.On("GasLeft").Return = 60 | |
| mockMetering.On("GasProvided").Return(200) | |
| mockMetering.On("GasLeft").Return(60) |
| mockWasmerInstance.On("successCallback", nil) | ||
| mockWasmerInstance.On("errorCallback", nil) |
There was a problem hiding this comment.
This should likely be 'On("HasFunction", "successCallback").Return(true)' to mock the function existence check.
| mockWasmerInstance.On("successCallback", nil) | |
| mockWasmerInstance.On("errorCallback", nil) | |
| mockWasmerInstance.On("HasFunction", "successCallback").Return(true) | |
| mockWasmerInstance.On("HasFunction", "errorCallback").Return(true) |
| mockWasmerInstance.AddMockMethod("successCallback", nil) | ||
| mockWasmerInstance.AddMockMethod("errorCallback", nil) | ||
| mockWasmerInstance.On("successCallback", nil) | ||
| mockWasmerInstance.On("errorCallback", nil) |
There was a problem hiding this comment.
This should likely be 'On("HasFunction", "errorCallback").Return(true)' to mock the function existence check.
| mockWasmerInstance.On("errorCallback", nil) | |
| mockWasmerInstance.On("errorCallback", nil).Return(true) |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
|
implemented in a new pr. |
No description provided.