Skip to content

Conversation

@MarcoRossignoli
Copy link
Contributor

@MarcoRossignoli MarcoRossignoli commented Jan 25, 2020

Test PR follow up coverlet-coverage/coverlet#700

This is a sample, it's not intended for merge.

I did some debugging and maybe found the issue(we'll see on this PR)

@antonfirsov PR add some out of process tests that slow down a bit the shutdown of test process, you're using msbuild that is affected by know issue https://github.com/tonerdo/coverlet/blob/master/Documentation/KnowIssues.md#1-vstest-stops-process-execution-earlydotnet-test

So new out of process tests create/update hits file(file where we put line/branch hit) on process exits but all other tests fail to log hits because vs test kill test process(the parent process of all other out of ones) before flush microsoft/vstest#1900 (comment)

This PR change the driver to collectors

BTW vstest has got a bug that I've fixed in microsoft/vstest#2221 but not yet released so you need for now to use the preview test sdk.

My local result
image

cc: @JimBobSquarePants

@claassistantio
Copy link

claassistantio commented Jan 25, 2020

CLA assistant check
All committers have signed the CLA.

@MarcoRossignoli
Copy link
Contributor Author

@JimBobSquarePants got and error on codecov upload

2020-01-25T14:11:49.1313263Z ==> Scanning for reports
2020-01-25T14:11:49.5779598Z     + d:\a\ImageSharp\ImageSharp\tests\ImageSharp.Tests\TestResults\6d6166d5-6a42-46eb-93c3-6f4b736c89aa\coverage.info
2020-01-25T14:11:49.5787040Z     + d:\a\ImageSharp\ImageSharp\artifacts\bin\tests\ImageSharp.Tests.ProfilingSandbox\Release\netcoreapp3.1\win7-x64\Microsoft.VisualStudio.CodeCoverage.Shim.dll
2020-01-25T14:11:49.5793378Z     + d:\a\ImageSharp\ImageSharp\artifacts\bin\tests\ImageSharp.Tests\Release\netcoreapp3.1\Microsoft.VisualStudio.CodeCoverage.Shim.dll
2020-01-25T14:11:49.5793911Z ==> Uploading reports
2020-01-25T14:11:49.7850673Z     HTTP 400
2020-01-25T14:11:49.7851344Z Please provide the repository token to upload reports via `-t :repository-token`

@MarcoRossignoli MarcoRossignoli requested review from JimBobSquarePants and antonfirsov and removed request for antonfirsov January 25, 2020 14:19
@codecov
Copy link

codecov bot commented Jan 26, 2020

Codecov Report

Merging #1094 into master will increase coverage by 16.22%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1094       +/-   ##
===========================================
+ Coverage   65.21%   81.43%   +16.22%     
===========================================
  Files         704      704               
  Lines       29311    29311               
  Branches     3290     3290               
===========================================
+ Hits        19115    23870     +4755     
+ Misses       9582     4748     -4834     
- Partials      614      693       +79
Flag Coverage Δ
#unittests 81.43% <ø> (+16.22%) ⬆️
Impacted Files Coverage Δ
...c/ImageSharp/ColorSpaces/Companding/LCompanding.cs 0% <0%> (ø) ⬆️
...geSharp/ColorSpaces/Companding/Rec709Companding.cs 0% <0%> (ø) ⬆️
...eSharp/ColorSpaces/Companding/Rec2020Companding.cs 0% <0%> (ø) ⬆️
...mats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs 95.85% <0%> (+2.54%) ⬆️
...Sharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs 95.83% <0%> (+8.33%) ⬆️
src/ImageSharp/Formats/Jpeg/JpegFormat.cs 87.5% <0%> (+12.5%) ⬆️
...p/Formats/Jpeg/Components/Decoder/JpegComponent.cs 100% <0%> (+16.66%) ⬆️
...Formats/Jpeg/Components/Decoder/ProfileResolver.cs 100% <0%> (+16.66%) ⬆️
...arp/Formats/Jpeg/Components/Decoder/AdobeMarker.cs 96.42% <0%> (+17.85%) ⬆️
...lorConverters/JpegColorConverter.FromYCbCrBasic.cs 100% <0%> (+21.05%) ⬆️
... and 114 more

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 c50eb15...21440b0. Read the comment docs.

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Jan 26, 2020

@MarcoRossignoli That's fantastic thank you!

This is a sample, it's not intended for merge.

I can't see any reason why we cannot merge this directly. Is there more you intend to add?

I'm actually going to go ahead and merge this. I'm keen to push on and the fix looks great. Thanks again!

@JimBobSquarePants JimBobSquarePants merged commit 37bf5bc into SixLabors:master Jan 26, 2020
@MarcoRossignoli MarcoRossignoli deleted the fixcoverage branch January 26, 2020 08:17
@MarcoRossignoli
Copy link
Contributor Author

I can't see any reason why we cannot merge this directly. Is there more you intend to add?

Didn't know if was your workflow idea. BTW good!

This issue is the reason for us to move most user as possible to collectors, this bug is hard to discover and is related to very "small" behaviour changes.

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