Skip to content

Fix coverage#3155

Merged
shargon merged 23 commits intomasterfrom
core-fix-coverarals
Feb 23, 2024
Merged

Fix coverage#3155
shargon merged 23 commits intomasterfrom
core-fix-coverarals

Conversation

@shargon
Copy link
Member

@shargon shargon commented Feb 22, 2024

The coverage is sometimes random, this pr aims to fix it

@vncoelho
Copy link
Member

this will be good if fixed

@vncoelho
Copy link
Member

Option path-to-lcov is now deprecated, use file instead.

@vncoelho
Copy link
Member

maybe version v2.2.3

@vncoelho
Copy link
Member

vncoelho commented Feb 22, 2024

"You can also skip using the file option and coveralls will try to find all supported coverage files and combine their data."
"If coveralls fails to determine your coverage report format, use explicit format option to specify it. See supported values."

@shargon
Copy link
Member Author

shargon commented Feb 22, 2024

I think that now it's fixed!

@vncoelho vncoelho marked this pull request as ready for review February 22, 2024 23:31
vncoelho
vncoelho previously approved these changes Feb 22, 2024
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

I think it is fixed, it was good to check their release info.

Maybe we can also leave the file config empty and delete it.

@shargon
Copy link
Member Author

shargon commented Feb 22, 2024

It only take cryptography https://coveralls.io/builds/65868879

@vncoelho vncoelho dismissed their stale review February 22, 2024 23:36

not ready

@vncoelho
Copy link
Member

I just tried something to be sure, let's see the result.

Recently I got some problems on devcontainer if these commands were not executed before

@vncoelho
Copy link
Member

Since the last command is just used for ubuntu

maybe remove the rest of the parameter for the other matrix.os

    - name: Test
      run: |
        find tests -name *.csproj | xargs -I % dotnet add % package coverlet.msbuild
        dotnet test /p:CollectCoverage=true /p:CoverletOutputFormat=lcov /p:CoverletOutput=${GITHUB_WORKSPACE}/coverage/lcov /p:MergeWith="./coverage/lcov.net7.0.info"
    - name: Coveralls
      if: matrix.os == 'ubuntu-latest'
      uses: coverallsapp/github-action@v2.2.3
      with:
        github-token: ${{ secrets.GITHUB_TOKEN }}
        file: ./coverage/lcov.net7.0.info
        ```

@vncoelho
Copy link
Member

vncoelho commented Feb 22, 2024

why we need merge? Which additional reports should be added?

@shargon
Copy link
Member Author

shargon commented Feb 22, 2024

Please @vncoelho wait for my wait for review
Screenshot_20240223_005512_Chrome

@shargon
Copy link
Member Author

shargon commented Feb 23, 2024

The doc said that it's required to merge the test one by one, and convert to the desired format in the last one:

https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/Examples/MSBuild/MergeWith/HowTo.md

@vncoelho
Copy link
Member

The doc said that it's required to merge the test one by one, and convert to the desired format in the last one:

https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/Examples/MSBuild/MergeWith/HowTo.md

I saw this as well later

@vncoelho
Copy link
Member

I think you should use the default as well without specifying, it will be json

/p:CoverletOutput=../CoverageResults/ /p:MergeWith="../CoverageResults/coverage.json"

@shargon shargon marked this pull request as ready for review February 23, 2024 00:37
@shargon
Copy link
Member Author

shargon commented Feb 23, 2024

It works now :)

@shargon shargon requested a review from Jim8y February 23, 2024 00:48
@shargon shargon merged commit de39a36 into master Feb 23, 2024
@shargon shargon deleted the core-fix-coverarals branch February 23, 2024 00:50
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

perfect, I think you can change to

    - name: Test for Win and Mac
      if: matrix.os == 'windows-latest' || matrix.os == 'macos-latest'   
      run: |
        dotnet test     
    - name: Test for Coveralls Only
      if: matrix.os == 'ubuntu-latest'  

@shargon
Copy link
Member Author

shargon commented Feb 23, 2024

This should also be done in modules and devpack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants