feat: Improving test coverage of SCs #240
Conversation
Raid5594
requested review from
vladbochok and
StanislavBreadless
as code owners
| } | ||
|
|
||
| function getTotalPriorityTxs() external view returns (uint256) { | ||
| return priorityQueue.getTotalPriorityTxs(); | ||
| return PriorityQueue.getTotalPriorityTxs(priorityQueue); |
There was a problem hiding this comment.
using for patterns are ignored by coverage
There was a problem hiding this comment.
I also wanted to ask why the PriorityQueue folder is missing from tests in this branch, but is available in the main.
Shall I add it back or maybe the base is not correct?
There was a problem hiding this comment.
You should add back, thanks!
There was a problem hiding this comment.
It moved to state-transition folder, so all good, missed it :)
l1-contracts/test/foundry/unit/concrete/AddressAliasHelper/_AddressAliasHelper_Shared.t.sol
Show resolved
Hide resolved
...tracts/test/foundry/unit/concrete/common/libraries/UncheckedMath/_UncheckedMath_Shared.t.sol
Show resolved
Hide resolved
| // add this to be excluded from coverage report | ||
| function test() internal virtual {} |
There was a problem hiding this comment.
This should be removed in order to PriorityQueue.sol to be in coverage
| // add this to be excluded from coverage report | ||
| function test() internal virtual {} |
| uint256 chainId = block.chainid; | ||
| Diamond.DiamondCutData memory initialDiamondCutData = getDiamondCutData(diamondInit); | ||
|
|
||
| vm.expectRevert(bytes("StateTransition: only bridgehub")); |
There was a problem hiding this comment.
It doesn't seem that coverage captured modifier's revert, researching how to make it work
|
|
||
| vm.expectRevert(bytes("StateTransition: only bridgehub")); | ||
|
|
||
| chainContractAddress.createNewChain(chainId, baseToken, sharedBridge, admin, abi.encode(initialDiamondCutData)); |
There was a problem hiding this comment.
Will probably move new chain creation to a function, as it's been reused in multiple places
| vm.startPrank(governor); | ||
| } | ||
|
|
||
| function getAdminSelectors() private view returns (bytes4[] memory) { |
There was a problem hiding this comment.
Will move selector getters to Utils
| import {DiamondProxy} from "solpp/state-transition/chain-deps/DiamondProxy.sol"; | ||
|
|
||
| contract createNewChainTest is StateTransitionManagerTest { | ||
| function testRevertWhenInitialDiamondCutHashMismatch() public { |
There was a problem hiding this comment.
Nit: I think convention is test_RevertWhen_
|
|
||
| // Call fails as storage is frozen | ||
| vm.expectRevert(bytes.concat("q1")); | ||
| isChainFrozen = gettersFacet.isDiamondStorageFrozen(); |
There was a problem hiding this comment.
Is it the case that this getter can't return true? (as the call reverts with the error described above)
What ❔
Add tests for contracts or simply factor existing tests, so that they are included in coverage report.
Why ❔
Increase overall coverage of smart contracts in the repo
Checklist