Code coverage based on concrete tests only

[smol-ninja]

Currently, code coverage includes both fuzz and concrete tests. The fuzz tests require that functions are not reverting. That's why we use assume and bound in them. Thus, they only test for non reverting situations.

On the other hand, concrete tests is to test for each and every line and all possible branches, which is what coverage should be telling us. Currently, even if some of the branches are missed in concrete but called in fuzz, the coverage will not be able to detect that.

An example is missing notNull modifier in isTransferrable (cantina finding).

Thus, I propose to run coverage only using concrete tests.

RFC @sablier-labs/solidity.

Here is a comparison. As you can see the coverage for SablierFlow.sol went down when calculating it based on concrete tests, demonstrating that there are missing concrete tests for some lines / branches.

--mp "tests/{integration}/**/*.sol"	--mp "tests/integration/concrete/**/*.sol"

PS: similar suggestion for lockup.

[PaulRBerg]

IIRC, there were cases in Lockup circa 2023 when only the fuzz tests covered some branches because it was very, very difficult to get a concrete test to cover them.

But anyway, I will let you two decide and lead the way.

[andreivladbrg]

Thus, they only test for non reverting situations

that's correct, but since we have all reverting scenarios in concrete tests, how is the coverage affected?

concrete tests is to test for each and every line and all possible branches

this is not quiet possible as we have lines like this one:

https://github.com/sablier-labs/flow/blob/04f3ed65b4c633d514ee64e2ec4022d821919382/src/SablierFlow.sol#L668-L673

---

btw, one of the reasons for having a lower coverage on concrete compared to concrete + fuzz is due to missing getter tests, which would be addressed in https://github.com/sablier-labs/flow/issues/299 (in fuzz we use the getters often so the lines are covered) let's see how will these tests change this

[smol-ninja]

@andreivladbrg you were right. https://github.com/sablier-labs/flow/issues/299 increased coverage for SablierFlowBase, with concrete tests, to 100% on all accounts from (96%, 97%, 100%, 92%) before. However, it only increased coverage for SablierFlow by 0.2%-0.4% (just a note and no call to action at this time).

[andreivladbrg]

increased coverage for SablierFlowBase, with concrete tests, to 100% on all

great news

it only increased coverage for SablierFlow by 0.2%-0.4% (just a note and no call to action at this time).

oh, that's less that i would've estimated, interesting, thanks for the update