-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: request modules integration tests #66
Conversation
_target = new MockCallee(); | ||
} | ||
|
||
function test_createRequest_finalizeEmptyResponse(bytes4 _selector, bytes calldata _data) public { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From line 30 to 47 could be move it to the setup
_target = new MockCallee(); | ||
} | ||
|
||
function test_createRequest_finalizeEmptyResponse(bytes4 _selector, bytes calldata _data) public { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also as the previous pr, if I were me I would delete the params and create a selector and a data for the test
import {MockCallee} from '../mocks/MockCallee.sol'; | ||
import './IntegrationBase.sol'; | ||
|
||
contract Integration_ContractCallRequest is IntegrationBase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also probably when you finalize the request , in the oracle the status will change, would be awesome to have asserts for this status, and expectCall to check that pay al release have been called with the correct params.
import {IHttpRequestModule} from '../../contracts/modules/request/HttpRequestModule.sol'; | ||
import './IntegrationBase.sol'; | ||
|
||
contract Integration_HttpRequest is IntegrationBase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similar comments that I put in the other test
- move common logic to internal function - avoid using fuzzing in integration tests - use startPrank, get rid of repeated lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥇
🤖 Linear
Closes OPO-658