Closed gkaracha closed 3 years ago
Thanks for the review @dorranh! Indeed, I made two changes for coverage purposes:
Moved the right-hand side of this clause to the same line as the pattern:
| CheckerEntrypoint _ ->
(Ligo.failwith error_ContractNotDeployed: LigoOp.operation list lazy_function_map (string, Ligo.bytes) Ligo.big_map * deployment_state)
| CheckerEntrypoint _ -> (Ligo.failwith error_ContractNotDeployed: LigoOp.operation list lazy_function_map (string, Ligo.bytes) Ligo.big_map * deployment_state)
This at least makes the coverage report to mark the line as yellow (pattern: green, rhs: red due to https://github.com/aantron/bisect_ppx/issues/377), which I can read as green (if the pattern is matched, then the rhs is trivially accessed).
Switched the if-then-else
for a match-with
, as you pointed out. I've noticed that the else
is never marked red or green, so, to get the "yellow" behavior I mentioned in the previous point, I switched to pattern matching instead.
Overall, since I don't expect https://github.com/aantron/bisect_ppx/issues/377 to be changed any time soon I've been thinking that we can combine @aantron's advice with using pattern matches instead of if-then-else
to get accurate coverage. That is, if we convert
if <condition>
then e1
else (Ligo.failwith (...))
into
match <condition> with
| true -> e1
| false -> ((Ligo.failwith [@coverage off]) (...)) (* tricky *)
then the "tricky" line is colored green when covered by the tests and red when not covered by the tests, as we want.
These unit tests primarily go through different execution paths and ensure that calls to entrypoints fail/succeed, depending on whether checker is sealed/unsealed. However, the values themselves are not inspected.
At this high a level most of the actual code to be run is wrapped in
{BEGIN,END}_LIGO
, so I think e2e tests are much more appropriate for testing the behavior of main.Test coverage goes from 86.40% to 88.52%.