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

Merging should not fail if file doesn't exist #220

Merged
merged 1 commit into from Oct 22, 2018
Merged

Merging should not fail if file doesn't exist #220

merged 1 commit into from Oct 22, 2018

Conversation

ghost
Copy link

@ghost ghost commented Oct 18, 2018

Changed merging behavior: merges the result of the current test coverage with the specified file only if the file exists.

@codecov
Copy link

codecov bot commented Oct 18, 2018

Codecov Report

Merging #220 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #220   +/-   ##
=======================================
  Coverage   94.91%   94.91%           
=======================================
  Files          15       15           
  Lines        1633     1633           
=======================================
  Hits         1550     1550           
  Misses         83       83
Impacted Files Coverage Δ
src/coverlet.core/Coverage.cs 99.49% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34c4e50...69fecaf. Read the comment docs.

Copy link
Collaborator

@tonerdo tonerdo left a comment

Choose a reason for hiding this comment

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

LGTM! Please update your commit to link to your GitHub profile. I prefer all my contributors' commits to be properly attributed

@ghost
Copy link
Author

ghost commented Oct 18, 2018

As discussed on #203

This tiny change should enable the use of a single "dotnet test" command to generate coverage for multiple test projects, aggregate all the results in one file, available in other formats (if requested).

Using VSTS/Azure Devops, this would be the way to have it work properly:

dotnet test **/*[Tt]ests/*.csproj
/p:CollectCoverage=true
/p:MergeWith=myfolder/coverage.json
/p:CoverletOutput=myfolder/
/p:CoverletOutputFormat=json,opencover,cobertura

Caveats:

  1. When requesting multiple formats in addition to JSON, those other files are generated and overwritten multiples times, for no good reason (other than the execution context not "knowing" which merge is the last one).

  2. In my scenario, my build definition depends on coverlet's default naming convention (coverage.json)

Correct me if I'm wrong in my analysis. I'll check to see if I can figure out how to test this, but I probably won't have enough time. If I'm right, this should be a way to really enhance coverlet's usage from a user's standpoint.

@pape77
Copy link
Contributor

pape77 commented Oct 19, 2018

I tested it and although it does create a new file if not exists, it creates a new file in each run without merging...

What i run:

dotnet test .\ProjectName.sln -l trx -r /results /p:CollectCoverage=true /p:Cove
rletOutputFormat=opencover /p:CoverletOutput=/results/coverage /p:MergeWith='/results/coverage.json'

For my example in #203

@tonerdo @vlef


Update:
It does work. What does not work is the generation of the opencover output alltogether in one single line.

dotnet test .\ProjectName.sln -l trx -r /results /p:CollectCoverage=true /p:CoverletOutput=/results/coverage /p:MergeWith='/results/coverage.json'

So this works great. But if I add /p:CoverletOutputFormat=opencover then it won't merge correctly again. But I think this was the case anyway before. Because the merging is done over .json files and apparently cannot be done (yet) in one run.
Good addition!

@ghost
Copy link
Author

ghost commented Oct 19, 2018

It should work, if you request both json and your other formats (those you actually want), separated by a coma.

/p:CoverletOutputFormat=json,opencover,cobertura

It's gonna merge with the json file, and also generate/overwrite the other formats in the same folder as the json. You really have to target a folder as the ouput though, can't pick a specific file or it's not gonna work either.

Edit: I had to use quotes and backslashes in my build when requesting multiple formats, maybe this can save you a headache:

/p:CollectCoverage=true "/p:CoverletOutputFormat=\"cobertura,opencover\"" /p:CoverletOutput=$(Build.SourcesDirectory)\TestResults\Coverage\

Thanks a lot for testing it!

@pape77
Copy link
Contributor

pape77 commented Oct 19, 2018

@vlef ah good point! Haven't thought about adding both output types as param.
Will try it next time next week. Hope it works.
Thanks!

@pape77
Copy link
Contributor

pape77 commented Oct 19, 2018

@vlef couldn't resist and I just tested it. Works! :D
In my case i had to add the %2c as to be interpreted as a comma, as stated in the project's wiki

dotnet test .\ProjectName.sln -l trx -r /results /p:CollectCoverage=true /p:CoverletOutputFormat='json%2copencover' /p:CoverletOutput=/results/coverage /p:MergeWith='/results/coverage.json'

And I got the file nicely merged in both formats :D

@tonerdo hope after this merge comes some testing on your side followed by a release!
As we are quite a few asking for this.

Cheers!

@tonerdo tonerdo merged commit b54c93c into coverlet-coverage:master Oct 22, 2018
@ghost ghost deleted the fix-merge-results-creates-file branch October 25, 2018 17:32
NorekZ pushed a commit to NorekZ/coverlet that referenced this pull request Nov 8, 2018
…creates-file

Merging should not fail if file doesn't exist
ViktorHofer pushed a commit to ViktorHofer/coverlet that referenced this pull request Nov 26, 2018
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.

None yet

2 participants