Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

635 remove unnecessary files from coverage #637

Conversation

eugenioclrc
Copy link
Contributor

Resolves #635

In forge currently is impossible to ignore files, but there is a wayaround to tackle this issue.
Please view foundry-rs/foundry#2988 based on this comment foundry-rs/foundry#2988 (comment) i have setup a coverage.sh script to ignore tests, mocks and scriptint solidity files.

@0x4007 0x4007 requested review from rndquu and zgorizzo69 and removed request for rndquu May 17, 2023 16:09
Copy link
Member

@0x4007 0x4007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to the DevPool from the DragonFly CTF leaderboard @eugenioclrc!

Some questions:

.github/workflows/coverage-check.yml Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Merge of ignores into the wildcard `*.info`

Co-authored-by: アレクサンダー.eth <4975670+pavlovcik@users.noreply.github.com>
eugenioclrc and others added 3 commits May 17, 2023 14:13
Add coverage task

Co-authored-by: アレクサンダー.eth <4975670+pavlovcik@users.noreply.github.com>
Co-authored-by: アレクサンダー.eth <4975670+pavlovcik@users.noreply.github.com>
Co-authored-by: アレクサンダー.eth <4975670+pavlovcik@users.noreply.github.com>
@zgorizzo69
Copy link
Contributor

nice work that is what I had in mind ;)

.github/workflows/coverage-check.yml Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
.github/workflows/coverage-check.yml Outdated Show resolved Hide resolved
@eugenioclrc eugenioclrc requested a review from rndquu May 18, 2023 00:34
@0x4007 0x4007 merged commit 29ff34c into ubiquity:development May 18, 2023
@eugenioclrc
Copy link
Contributor Author

Note for the future, it will be excellent to make this work;

diff --git a/.github/workflows/coverage-check.yml b/.github/workflows/coverage-check.yml
index 2b40567d..ef400b75 100644
--- a/.github/workflows/coverage-check.yml
+++ b/.github/workflows/coverage-check.yml
@@ -29,34 +29,7 @@ jobs:
       - name: Get development branch coverage
         id: coverage-development
         run: |
-          cd ./packages/contracts
-
-          # generates lcov.info
-          forge coverage --report lcov
-
-          # Foundry uses relative paths but Hardhat uses absolute paths.
-          # Convert absolute paths to relative paths for consistency.
-          sed -i -e 's/\/.*solidity.//g' lcov.info
-
-          # Merge lcov files
-          lcov \
-              --rc lcov_branch_coverage=1 \
-              --add-tracefile lcov.info \
-              --output-file merged-lcov.info
-
-          # Filter out node_modules, test, and mock files
-          lcov \
-              --rc lcov_branch_coverage=1 \
-              --remove merged-lcov.info \
-              --output-file filtered-lcov.info \
-              "*node_modules*" "*test*" "*mock*" "*scripts*"
-
-
-          # Generate summary
-          COVERAGE_DEVELOPMENT_OUTPUT=$(lcov \
-              --rc lcov_branch_coverage=1 \
-              --list filtered-lcov.info)
-
+          COVERAGE_DEVELOPMENT_OUTPUT=$(npm run coverage)
           echo COVERAGE=$(echo "${COVERAGE_DEVELOPMENT_OUTPUT}" | tail -n 1 | cut -d % -f 1 | cut -d \| -f 2) >> $GITHUB_OUTPUT
 
       - name: Checkout code in PR branch
@@ -65,33 +38,7 @@ jobs:
       - name: Get PR branch coverage
         id: coverage-pr
         run: |
-          cd ./packages/contracts
-
-          # generates lcov.info
-          forge coverage --report lcov
-
-          # Foundry uses relative paths but Hardhat uses absolute paths.
-          # Convert absolute paths to relative paths for consistency.
-          sed -i -e 's/\/.*solidity.//g' lcov.info
-
-          # Merge lcov files
-          lcov \
-              --rc lcov_branch_coverage=1 \
-              --add-tracefile lcov.info \
-              --output-file merged-lcov.info
-
-          # Filter out node_modules, test, and mock files
-          lcov \
-              --rc lcov_branch_coverage=1 \
-              --remove merged-lcov.info \
-              --output-file filtered-lcov.info \
-              "*node_modules*" "*test*" "*mock*" "*scripts*"
-
-
-          # Generate summary
-          COVERAGE_DEVELOPMENT_OUTPUT=$(lcov \
-              --rc lcov_branch_coverage=1 \
-              --list filtered-lcov.info)
+          COVERAGE_DEVELOPMENT_OUTPUT=$(npm run coverage)
           echo COVERAGE=$(echo "${COVERAGE_DEVELOPMENT_OUTPUT}" | tail -n 1 | cut -d % -f 1 | cut -d \| -f 2) >> $GITHUB_OUTPUT
 
       - name: Print coverages

@0x4007
Copy link
Member

0x4007 commented May 19, 2023

Is that pseudo code or why not just implement the suggestion as is?

@eugenioclrc
Copy link
Contributor Author

Is that pseudo code or why not just implement the suggestion as is?

Npm run coverage should run all those lines, not sure why is not working on gh action

@0x4007
Copy link
Member

0x4007 commented May 20, 2023

You can link some GitHub action logs here and we might be able to help debug it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove unecessary files from coverage
4 participants