Add Foundry tests to coverage

[cairoeth]

Adds Foundry tests to coverage and allows Codecov to combine both reports. The coverage script command removes zero hits in the Foundry report so that both reports can be merged exclusively, otherwise Codecov will mark lines as not covered, when in reality they are covered by Hardhat.

It increases coverage by 0.07% overall, 0.98% in Governor and 0.65% in Clones. Time increase for workflow is very small.

PR Checklist

• Tests
• Documentation
• Changeset entry (run npx changeset add)

[changeset-bot]

⚠️ No Changeset found

Latest commit: 76c9351

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[Amxx]

I tried running this, I'm getting an error

lcov: ERROR: (inconsistent) "contracts/token/ERC721/extensions/ERC721Royalty.sol":24: function ERC721Royalty.supportsInterface found on line but no corresponding 'line' coverage data point. Cannot derive function end line. See lcovrc man entry for 'derive_function_end_line'.
(use "lcov --ignore-errors inconsistent ..." to bypass this error)

I'm using LCOV version 2.1-1

Also, I'm missing how this is working. Should the forge coverage and "lcov processing" be done BEFORE the hardhat coverage so that both reports can be merged ?

[cairoeth]

I tried running this, I'm getting an error

lcov: ERROR: (inconsistent) "contracts/token/ERC721/extensions/ERC721Royalty.sol":24: function ERC721Royalty.supportsInterface found on line but no corresponding 'line' coverage data point. Cannot derive function end line. See lcovrc man entry for 'derive_function_end_line'.
(use "lcov --ignore-errors inconsistent ..." to bypass this error)

I'm using LCOV version 2.1-1

Also, I'm missing how this is working. Should the forge coverage and "lcov processing" be done BEFORE the hardhat coverage so that both reports can be merged ?

the lcov processing is only needed for the foundry report, so it has to be done after that. i also don't like that the script command in package.json is very long and complicated. maybe we can move it to a sh script and call the script in package.json?

[Amxx]

maybe we can move it to a sh script and call the script in package.json?

yes

[Amxx]

the lcov processing is only needed for the foundry report, so it has to be done after that

That I inderstand, but after that processing nothing gets done with the lcov.info. I was thinking that for the hardhat task to merge the report produced by foundry (and post processed) ... then the harhdat coverage run should be done at the end.

[cairoeth]

nothing gets done with the lcov.info

the workflow uploads lcov.info and the other coverage reports to codecov automatically, which then merges it. solidity-coverage doesn't support merging Hardhat and Foundry reports sadly (sc-forks/solidity-coverage#734).

[Amxx]

nothing gets done with the lcov.info

the workflow uploads lcov.info and the other coverage reports to codecov automatically, which then merges it. solidity-coverage doesn't support merging Hardhat and Foundry reports sadly (sc-forks/solidity-coverage#734).

Hoooo ... then I don't think the foundry part should run when doing coverage locally on our machine (unless we find a way to report an aggregate).

If we have a script it could distinguish between CI and local, and only run the foundry + post processing is in CI.

[ernestognw]

LGTM, thanks @cairoeth

On a related topic, I'm intrigued by the increase in coverage for Clones.sol, it seems like it's hitting a case I never knew how to replicate in unit tests. See here