Foundry coverage report is inaccurate #332
Comments
This was referenced Apr 3, 2023
2 tasks
|
The coverage report for the |
PaulRBerg
referenced
this issue
May 30, 2023
* feat: implement the NFT descriptor * feat: add status box in the SVG feat: add streamed amount abbreviation box feat: hash the chainid in the getColorAccent function feat: implement the half empty hourglass when the stream is depleted feat: add a bash script to extract the svg from tokenURI function fix: use the correct JSON string format refactor: remove the contract name box refactor: add the contract name in the rotating border refactor: reorder function parameters for improved clarity refactor: reorder struct parameters for improved clarity refactor: improve naming of the variables build: git ignore "tokenURI.svg" * chore: correct comment in extract_svg.sh * docs: correct getColorAccent natspec * fix: issues in the NFT descriptor (#454) * fix: typo in import of "Base64" in NFTSVG.sol * fix: incorrect SVG character escaping in "getStreamedAbbreviation" function * fix: use correct string representation of "sender" and "recipient" SVG params in "tokenURI" function * build: disable solhint "quotes" chore: disable "max-line-length" in SVGComponents * perf: optimize SVG components (#462) * fix: return value in abbreviation function when >999q * feat: use hsl colors instead of hex fix: getStreamedAbbreviation when decimals equals to 0 fix: limit the maximum number of days in the box to 9999 fix: update the new extended status perf: remove redundant spaces in the SVG components * fix: circle color in the progress box fix: decrease box offsets to prevent too wide boxes fix: use "<" and ">" operators instead of ">" and "<" fix: fractionalPart in getStreamedAbbreviation function refactor: remove support for quadrillions numbers in getStreamedAbbreviation function * test: add tokenURI tests for lockup linear test: add tokenURI tests for lockup dynamic test: add createDefaultStreamWithCliffTime helper function * test: fix streamed box tests in dynamic test: use cliff duration instead of total duration test: correct local variables names in streamed box tests in linear * refactor: move extract_svg script in shell dir * test: solhint disable in tokenURI test files * test: remove unnecessary space when logging the uri * fix: fix SVG attributes and comparison signs chore: disable Solhint rules selectively chore: improve extract SVG shell script chore: make Prettier format "*.svg" files chore: re-enable "quotes" Solhint rule globally chore: rename extract SVG shell script * refactor: remove unneeded `viewBox` in SVG refactor: lowercase ids in svg refactor: reorder attributes test: remove "createDefaultStreamWithCliffTime" test: rename "tokenURI.svg" to "token-uri.svg" * perf: polish and optimize SVG build: bump OpenZeppelin to v4.9 docs: add explanatory comments docs: improve writing and formatting in NatSpec refactor: alphabetical ordering in SVG files refactor: change order of ifs in "getSablierStatus" refactor: improve function names in SVG files refactor: remove leading zeroes refactor: remove unneeded "<g>" tags refactor: revert invalid contracts in "tokenURI" refactor: use "Strings.equal" instead of keccak256 comparison refactor: use "#fff" instead of "white" * perf: further polish and optimize SVG test: thoroughly test individual functions in "NFTDescriptor" feat: add "safeAssetDecimals" feat: add "safeAssetSymbol" feat: add "stringifyPercentage" perf: use fewer variables perf: reduce number of params for "card" function refactor: refactor streamed percentage calculations refactor: get rid of "get" prefix in SVG helpers refactor: improve function and variable names in SVG files refactor: refactor NFT descriptor error test: add "ERC20Bytes32" mock test: add "NFTDescriptorMock" test: deploy "Noop" in base test test: refactor "Empty" mock to "Noop" * fix: fix SVG attribute spacing in "progressCircle" * test: simplify "generateAccentColor" test * refactor: remove global style in SVG * fix: fix StackTooDeep in "generateDefs" refactor: modulo 360 for "hue" in HSL algorithm refactor: remove named return args in SVG files refactor: rename "rowWidth" to "cardsWidth" * feat: implement NFT metadata chore: include "svg" extension in Prettier scripts docs: polish NatSpec comments feat: add "external_url" in SVG metadata refactor: base64 encode in "NFTDescriptor" refactor: define "&" character as string instead of hex refactor: generate JSON in "NFTDescriptor" refactor: improve variable and function names refactor: rename "nft" to "sablier" test: test NFT metadata functions test: upgrade NFT descriptor test contract names * refactor: remove nested dirs in "script" refactor: rename "Base" to "Init" refactor: use named params in "Init" * feat: add "GenerateSVG" script * refactor: update gas snapshot chore: include "SablierV2NFTDescriptor" in "gas_reports" * refactor: rename DeployProtocol to DeployCore * ci: polish workflow input descriptions * build: correct test function paths test: use days instead of big numbers * ci: polish github step summary * ci: add workflow for "GenerateSVG" script * feat: shell scripts to extract pure svg code from solidity script feat: shell script to generate common set of svgs * feat: dedicate out-svg folder for generated nfts * fix: fix "duration" in "GenerateSVG" * feat: svg panoply chore: delete "out-svg" in "clean" script chore: prettier ignore all "*.sol" files chore: unignore "out-svg" docs: add explanatory comments in "generate-svg.sh" docs: polish and simplify comments in Shell scripts refactor: concat GE sign in "GenerateSVG" refactor: polish "generate-svg.sh" Shell script refactor: rename "generate-svg-set" to "generate-svg-panoply" * feat: add "stringifyFractionalAmount" fix: fix fractional parts with leading zero in "abbreviateAmount" refactor: use "stringifyFractionalAmount" in "stringifyPercentage" test: add more test cases for "abbreviateAmount" test: test "stringifyFractionalAmount" * build: bump "@openzeppelin/contracts" Node.js package * test: use "Strings.equal" instead of "eqString" * test: simplify nesting structure in NFT descriptor tests test: add more tests for "calculateDurationInDays" * test: test hard-coded SVG * test: bring back "unit" directory chore: use "--mt" and "--nmt" shorthands cI: sync workflows with new test directory structure test: add "AdminableMock" test: correct errors in "Adminable" tests test: get rid of superfluous comptroller test contract test: move NFT descriptor tests to "unit" test: remove superfluous "setUp" declarations * feat: provide precompile for NFTDescriptor feat: add "DeployDeterministicNFTDescriptor" refactor: deploy NFT descriptor in "DeployCore" refactor: define bytecodes as constants in "Precompiles" refactor: hard code bytecodes in "Precompiles" refactor: revert with reason string in "Precompiles" test: expand "NFTDescriptorMock" test: run tests against precompiled "AdminableMock" test: run tests against precompiled "NFTdescriptorMock" test: various misc improvements * refactor: update warning in NFT JSON metadata * refactor: remove circle when progress is "0%" perf: adjust average character width for small font refactor: fix streamed amounts in svg panoply refactor: switch order of params in "progressCircle" * fix: use numerical escape character docs: add NatSpec comments for "SIGN_" constants docs: polish NatSpec comments refactor: rename "SABLIER" to "LOCKUP_LINEAR" refactor: rename return parameters in "Precompiles" test: fix variable names in "generateDescription" tests test: define "sender" config option in "foundry.toml" test: remove named arguments in "Base_Test" test: test "tokenURI" * refactor: remove "extract-svg" script * refactor: handle zero amounts in "abbreviateAmount" chore: fix streamed amounts in SVG panoply feat: add more configurations in SVG panoply refactor: handle zero amounts in "GenerateSVG" test: test pending status in "generateSVG" test: remove superfluous "@title" tag test: reorder functions in "Base_Test" * test: various improvements test: declare private "vm" in "Utils" test: move "isTestOptimizedProfile" to "Utils" test: remove named return params in test utils test: remove superfluous "@title" tags * chore: prettier ignore "out-svg" * refactor: update precompiles * docs: polish NatSpec comments * build: add one more case in the svg panoply refactor: use "abi.encodeCall" for low level calls chore: remove unused imports * chore: fix streamed amounts in svg panoply * test: cross-profile deterministic addresses test: fix "tokenURI" tests in CI * ci: fix path in "test-utils" job * test: skip on address mismatch * test: assume no precompiles in "transferAdmin" * refactor: bring back "viewBox" in SVG * chore: codecov ignore SVG libraries ref https://github.com/sablier-labs/v2-core/issues/332\#issuecomment-1568411246 * refactor: update precompiles * refactor: update gas snapshot --------- Co-authored-by: Paul Razvan Berg <paul.razvan.berg@gmail.com> Co-authored-by: Ahmed Ihsan Tawfeeq <ahmed.i.tawfeeq@protonmail.com> Co-authored-by: Razvan Gabriel <razvan.gabriel.apostu@gmail.com>
|
Closing as there's nothing we can do this about and this issue has become a historical FYI. |
|
I've looked at the most recent coverage reports, and they are more accurate now. It looks like foundry-rs/foundry#4305 (fixed now) was the culprit behind the many false positives reported in the OP. There's just one false positive now: https://app.codecov.io/gh/sablier-labs/v2-core/blob/main/src%2FSablierV2LockupDynamic.sol#L357 Three other cases appear as uncovered:
But these are expected - those uncovered ifs are meant to detect potentially unknown bugs in the calculation functions. For more details, see #420. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem
The coverage report we get from Foundry is mostly incorrect, and this messes up the branch coverage % reported on Codecov.
I spent days investigating what's going on, and based on my findings, I think that the real test coverage we have is 100%.
Incorrect Reports
Assertions appear as partially covered
See foundry-rs/foundry#4294 for context.
Other incorrect partials
>,==, and<)isFlashLoanablereturnstrue, and when it returnsfalse.ACTIVEin the for loop inwithdrawMultiple.stream.amounts.deposit == stream.amounts.withdrawnand when they are not equal (!=).index > 1and whenindex <= 1.Related Foundry Issues
forge coverageconsiders branches partially covered if they have CALL foundry-rs/foundry#3497forge coveragedoes not work for if statements within for loops foundry-rs/foundry#3600The text was updated successfully, but these errors were encountered: