Repository navigation
Tricky Revert Test With Missing Coverage Branch #9007
|
Hello Guys, Does anyone have an idea how can I test revert from this function: /// @dev Override Vote Function
/// @notice Changes block.number into block.timestamp for snapshot
function clock() public view override returns (uint48) {
return Time.timestamp();
}
/// @dev Override Vote Function
/// @notice Changes block.number into block.timestamp for snapshot
function CLOCK_MODE() public view override returns (string memory) {
// Check that the clock was not modified
if (clock() != Time.timestamp()) revert ERC6372InconsistentClock();
return "mode=timestamp";
}The problem is that mocking revert doesnt count as proper test and I still have missing branch in my coverage report... /// @dev COVERAGE DOES NOT ACCEPT 'MockCallRevert', SO THER IS MISSING BRANCH FOR THIS ERROR In COVERAGE
function testClockMode() public {
uint256 currentClock = asset.clock();
uint256 currentTimestamp = block.timestamp;
assertEq(currentClock, currentTimestamp);
assertEq(asset.CLOCK_MODE(), "mode=timestamp");
vm.expectRevert(Votes.ERC6372InconsistentClock.selector);
vm.mockCallRevert(address(asset), abi.encodeWithSignature("CLOCK_MODE()"), abi.encodeWithSelector(Votes.ERC6372InconsistentClock.selector));
asset.CLOCK_MODE();
}Would much appreciate any tips/ideas how can I properly test this revert I believe that foundry somehow allows that but I just have no idea how... Thanks in advance! |
Replies: 3 comments
|
Update, I was trying to use |
|
The issue is that The core problem: The trick is to mock function testClockModeRevert() public {
// Mock clock() to return a value that differs from block.timestamp
// This causes the check inside CLOCK_MODE() to fail
vm.mockCall(
address(asset),
abi.encodeWithSignature("clock()"),
abi.encode(uint48(block.timestamp + 1))
);
vm.expectRevert(Votes.ERC6372InconsistentClock.selector);
asset.CLOCK_MODE();
}This works because:
Important note: // In your test file
contract BrokenClockHarness is YourToken {
bool public breakClock;
function setBreakClock(bool _break) external {
breakClock = _break;
}
function clock() public view override returns (uint48) {
if (breakClock) return uint48(block.timestamp + 1);
return super.clock();
}
}
// In your test
function testClockModeRevertWithHarness() public {
BrokenClockHarness harness = new BrokenClockHarness();
// ... initialize harness same as asset ...
harness.setBreakClock(true);
vm.expectRevert(Votes.ERC6372InconsistentClock.selector);
harness.CLOCK_MODE();
}The harness approach is the most reliable because it guarantees the actual revert path executes inside the real |
|
The reason nothing works: that branch is unreachable on the contract under test. The standard pattern for defensive-but-dead branches: deploy a test harness that inherits the contract under test and overrides only import {ERC20Votes} from "openzeppelin/token/ERC20/extensions/ERC20Votes.sol";
contract InconsistentClock is ERC20Votes {
function clock() public view override returns (uint48) {
return uint48(block.number); // always != block.timestamp -> branch taken
}
}
function test_clockMode_revertsOnInconsistentClock() public {
InconsistentClock token = new InconsistentClock();
vm.expectRevert(Votes.ERC6372InconsistentClock.selector);
token.CLOCK_MODE(); // real execution -> branch counted in forge coverage
}Because Keep your original happy-path test as-is; together the two tests cover both branches of the check. This harness-override trick is generally the cleanest way to cover "this should never happen" guards that are provably unreachable on a correct implementation. |
The reason nothing works: that branch is unreachable on the contract under test.
clock()returnsTime.timestamp()andCLOCK_MODE()compares it againstTime.timestamp()— the two are always equal, so no state manipulation can make them diverge, andmockCallRevertcan't cover it because it never executes the function body (the branch is never evaluated by the EVM).The standard pattern for defensive-but-dead branches: deploy a test harness that inherits the contract under test and overrides only
clock(), so the comparison genuinely fails and the real code path executes: