Fix exclude by files #524

MarcoRossignoli · 2019-08-20T10:58:08Z

Fixes #122

At the moment the issue is that files to exclude are searched on current directory and this is the problem.
I moved source file exclusion evaluation inside instrumentation for every instrumented dll/source.

NB. At the moment we allow rule with root drive defined...but this doesn't work well with globbing pattern matching(d:\file and c:\file will be matched like same file), btw this implementation allow the use, in future we could think about forbid. I don't think make a lot of sense use full path on rules.

I run this test
win

dotnet test /p:CollectCoverage=true /p:Include=\"[coverlet.core*]*\" /bl  /p:ExcludeByFile=\"**\Instrumenter.cs,**\ModuleTrackerTemplate.cs\,**\InstrumenterResult.cs"

with this result

ubuntu

 dotnet test /p:CollectCoverage=true /p:Include=\"[coverlet.core*]*\" /bl  /p:ExcludeByFile=\"**\\Instrumenter.cs,**\\ModuleTrackerTemplate.cs\,**\\InstrumenterResult.cs\"

Report without Instrumenter.cs, InstrumenterResult.cs coverage.json.txt
Normal report without filter Instrumenter.cs, InstrumenterResult.cs are present coverage.json.txt

cc: @tonerdo @petli
@ido-namely you did first implementation, if you've some time let me know what do you think.
@SlowLogicBoy @orkenstein @hack2root if you find some time could clone this branch and do tests, not obligated, you can use your local clone following this guide https://github.com/tonerdo/coverlet/blob/master/Documentation/Troubleshooting.md#use-local-build

MarcoRossignoli · 2019-08-20T11:11:18Z

Oh missed unix fs behaviour...I'll fix tests.

petli · 2019-08-21T12:28:04Z

src/coverlet.core/coverlet.core.csproj

@@ -5,6 +5,7 @@
    <TargetFramework>netstandard2.0</TargetFramework>
    <AssemblyVersion>5.1.1</AssemblyVersion>
    <IsPackable>false</IsPackable>
+    <LangVersion>preview</LangVersion>


Should master use preview version of C#?

Good question!
I did it for test project https://github.com/tonerdo/coverlet/blob/master/test/coverlet.core.tests/coverlet.core.tests.csproj
At the moment the only feature I used is null coalesce ??= because other features(i.e. IAsyncEnumerable) needs new netcoreapp3.0/standard2.1 object.
Also corefx seem allow preview https://github.com/dotnet/corefx/blob/master/Directory.Build.props#L248
@ViktorHofer do you see any issue with this config related to emitted IL?
cc: @tonerdo

No concerns from my side.

MarcoRossignoli · 2019-09-03T18:02:33Z

@tonerdo @petli what do you think about this PR?I'd like to have at least one more approval.

MarcoRossignoli · 2019-09-06T10:28:43Z

I'll merge after Viktor opinion on preview and to avoid stale...hope my test above were enough.

adrianhara · 2019-09-18T11:20:50Z

When can we expect this to be released (I'd need to in the global tool)?

MarcoRossignoli · 2019-09-18T11:37:50Z

We don't have a strict plan for release cc: @tonerdo
BTW you can use our nightly and use the feature today https://github.com/tonerdo/coverlet/blob/master/Documentation/ConsumeNightlyBuild.md
Ah @adrianhara let us know if it works as expected!

adrianhara · 2019-09-18T13:05:19Z

@MarcoRossignoli nice! I installed the latest nightly version of the global tool, unfortunately i get this:

Unable to instrument module: '.\XUnitTest.Tests\bin\Debug\XUnitTest.exe' because : Invalid COR20 header signature.

My command line:
coverlet --target "c:\wrk\test\wpf\XUnitTest\packages\xunit.runner.console.2.4.1\tools\net461\xunit.console.exe" --targetargs ".\XUnitTest.Tests\bin\Debug\XUnitTest.Tests.dll" --exclude-by-file "**/*.g.cs" --output ".\xunittestcoverage" --format json --verbosity detailed .\XUnitTest.Tests\bin\Debug\XUnitTest.Tests.dll

I didn't get the error with the previous official version.

adrianhara · 2019-09-18T13:09:50Z

The version 1.5.69-gafe227b39e seems to work though:

no COR20 error
the files are excluded

MarcoRossignoli · 2019-09-18T13:19:38Z

can you cleanup files and retry with 1.5.79-g2aa8ed683c should work in the same way.
Enable logs please https://github.com/tonerdo/coverlet/blob/master/Documentation/Troubleshooting.md#coverlet-global-tool

adrianhara · 2019-09-18T13:30:04Z

@MarcoRossignoli can you be a bit more specific which files I should clean up?

MarcoRossignoli · 2019-09-18T13:30:55Z

can you be a bit more specific which files I should clean up?

something like git clean -fdx...to be sure to rebuild

adrianhara · 2019-09-18T13:34:06Z

Oh I see, you're right: I cleaned up my project (deleted bin+obj folders) and 1.5.79-g2aa8ed683c seems to work.

One difference I noticed from the previous official version is it gives some warning messages:
Unable to instrument module: .\XUnitTest.Tests\bin\Debug\xunit.assert.dll, embedded pdb without local source files
Unable to instrument module: .\XUnitTest.Tests\bin\Debug\xunit.core.dll, embedded pdb without local source files
Unable to instrument module: .\XUnitTest.Tests\bin\Debug\xunit.execution.desktop.dll, embedded pdb without local source files
Unable to instrument module: .\XUnitTest.Tests\bin\Debug\xunit.runner.reporters.net452.dll, embedded pdb without local source files
Unable to instrument module: .\XUnitTest.Tests\bin\Debug\xunit.runner.utility.net452.dll, embedded pdb without local source files
Unable to instrument module: .\XUnitTest.Tests\bin\Debug\xunit.runner.visualstudio.testadapter.dll, embedded pdb without local source files

These weren't there when running the official version.

MarcoRossignoli · 2019-09-18T13:44:39Z

One difference I noticed from the previous official version is it gives some warning messages:

Have you enabled logs --verbosity detailed?

And yes new version exclude in automatic way libs that are not instrumentable...a lib is instrumentable if has got pdb and if source(indicated in pdb) are present on current hard drive(source that builds dll+pdb).
That log should show only with verbosity enabled #548 I changed a pair of days ago.

adrianhara · 2019-09-18T13:50:11Z

It actually shows the same regardless of verbosity (not specified / detailed). It's a minor issue, just thought you wanted to know. Thanks for the quick help!

MarcoRossignoli · 2019-09-18T14:07:45Z

It actually shows the same regardless of verbosity (not specified / detailed).

Uh...I'll investigate...

adrianhara · 2019-09-18T14:21:53Z

😢 The problem with invalid COR20 header signature is now happening in my continuous integration environment:

Unable to instrument module: 'blabla.dll' because : Invalid COR20 header signature.

The CI build does:

Fresh git checkout (no files on disk prior)
Build project
Run unit tests (xunit)
Run coverlet -> error

The error is only reported on some DLLs, others are Instrumented module: ok (ran with verbosity detailed). It's a .net 461 project.

Ideas?

MarcoRossignoli · 2019-09-18T14:43:46Z

You could include only module you want to cover using filters --include "[modulename.*]*,..." to avoid to instrument infrastructure libs.

adrianhara · 2019-09-18T14:46:35Z

Well in my case these are modules I want to cover, they belong to the app. In the meantime I discovered that the error might be related to some permissions so I switched the CI user to an admin and the Invalid COR20 header signature. has gone away 🎉

However, I have a new issue 😢
Instrumented module: 'bla.dll' <- so that seems good, but then
Hits file:'bla_81db4e74-0db0-4459-a021-1a30d59b7020' not found for module: 'bla' <- which results in a 0% coverage being reported for that module

Any clue as to why the hits file can't be found?

MarcoRossignoli · 2019-09-18T14:50:40Z

Any clue as to why the hits file can't be found?

Could be related to 2 fact...one there is no test that call that code...the second one could be an well know issue with vs test that we solved with collectors #110
code ref https://github.com/tonerdo/coverlet/blob/master/src/coverlet.core/Coverage.cs#L231

adrianhara · 2019-09-18T16:06:47Z

@MarcoRossignoli you're right: the modules for which no hits file were generated didn't have any tests referencing them, so that's cool!

Unfortunately I'm sorry to say my previous statement that the COR20 header signature was related to permissions was incorrect, sorry about the confusion. Actually on the other account I tried there was still the version 1.5.69-gafe227b39e installed and that's why it worked. Once I updated that account to 1.5.79-g2aa8ed683c I got the same error. Reversely, downgrading the original account under which the CI user ran to 1.5.79-g2aa8ed683c fixed the COR20 header signature error. So empirically it looks like there's something wrong with 1.5.79-g2aa8ed683c.

I can also supply some repro steps:

I created a .Net 461 solution containing two projects: a WPF app and Class Library referencing the app and containing some tests.
Fresh checkout from git of the above
Build + run tests with the xunit console runner
Run coverlet coverlet --target "c:\wrk\test\wpf\XUnitTest\packages\xunit.runner.console.2.4.1\tools\net461\xunit.console.exe" --targetargs ".\XUnitTest.Tests\bin\Debug\XUnitTest.Tests.dll" --exclude-by-file "**/*.g.cs" --output ".\xunittestcoverage" --format json --verbosity detailed .\XUnitTest.Tests\bin\Debug\XUnitTest.Tests.dll
Error

Same sequence of steps with version 1.5.69-gafe227b39e works just fine.

I noticed that if after step 3 I delete the obj folder there is no COR20 header signature error, but also the coverage isn't calculated correctly, total shows 100% while average shows shows infinity.

MarcoRossignoli · 2019-09-18T17:44:15Z

@adrianhara thank's for infos...I'll try to repro...if you have already a repro please attach it here(zip solution) so I won't waste time(sometimes is not so easy perfectly repro, also a small difference changes the game)

adrianhara · 2019-09-18T18:24:50Z

@MarcoRossignoli sure, I uploaded my repro code here https://github.com/adrianhara/coverlet-issue-repro

MarcoRossignoli · 2019-09-19T13:24:17Z

@adrianhara to run on last version you should generate portable pdb from wpf project

fix exclude by files

758c5f1

MarcoRossignoli requested review from tonerdo and petli August 20, 2019 10:58

MarcoRossignoli added the bug Something isn't working label Aug 20, 2019

MarcoRossignoli added 2 commits August 20, 2019 13:00

merge master

b64c8d3

fix InstrumentMethod

c2584f4

MarcoRossignoli changed the title ~~Fix exclude by files~~ [WIP]Fix exclude by files Aug 20, 2019

updates

7d70560

MarcoRossignoli changed the title ~~[WIP]Fix exclude by files~~ Fix exclude by files Aug 20, 2019

improve logging

52e5901

petli reviewed Aug 21, 2019

View reviewed changes

MarcoRossignoli mentioned this pull request Sep 3, 2019

Improve instrumentation tests #527

Merged

MarcoRossignoli merged commit afe227b into coverlet-coverage:master Sep 6, 2019

MarcoRossignoli deleted the excludedbyfile2 branch September 6, 2019 10:23

This was referenced Sep 19, 2019

Fix source file check for non portable pdb #558

Merged

v5.2.0 was released without a ChangeLog #562

Closed

MarcoRossignoli mentioned this pull request Nov 8, 2019

Exclude argument is not working for dotnet test command #613

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix exclude by files #524

Fix exclude by files #524

MarcoRossignoli commented Aug 20, 2019 •

edited

Loading

MarcoRossignoli commented Aug 20, 2019

petli Aug 21, 2019

MarcoRossignoli Aug 21, 2019 •

edited

Loading

ViktorHofer Aug 21, 2019

MarcoRossignoli commented Sep 3, 2019

MarcoRossignoli commented Sep 6, 2019 •

edited

Loading

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 18, 2019 •

edited

Loading

adrianhara commented Sep 18, 2019 •

edited

Loading

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 18, 2019 •

edited

Loading

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 18, 2019

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 18, 2019

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 18, 2019

adrianhara commented Sep 18, 2019 •

edited

Loading

MarcoRossignoli commented Sep 18, 2019

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 18, 2019

adrianhara commented Sep 18, 2019 •

edited

Loading

MarcoRossignoli commented Sep 18, 2019 •

edited

Loading

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 19, 2019

Fix exclude by files #524

Fix exclude by files #524

Conversation

MarcoRossignoli commented Aug 20, 2019 • edited Loading

MarcoRossignoli commented Aug 20, 2019

petli Aug 21, 2019

Choose a reason for hiding this comment

MarcoRossignoli Aug 21, 2019 • edited Loading

Choose a reason for hiding this comment

ViktorHofer Aug 21, 2019

Choose a reason for hiding this comment

MarcoRossignoli commented Sep 3, 2019

MarcoRossignoli commented Sep 6, 2019 • edited Loading

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 18, 2019 • edited Loading

adrianhara commented Sep 18, 2019 • edited Loading

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 18, 2019 • edited Loading

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 18, 2019

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 18, 2019

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 18, 2019

adrianhara commented Sep 18, 2019 • edited Loading

MarcoRossignoli commented Sep 18, 2019

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 18, 2019

adrianhara commented Sep 18, 2019 • edited Loading

MarcoRossignoli commented Sep 18, 2019 • edited Loading

adrianhara commented Sep 18, 2019

MarcoRossignoli commented Sep 19, 2019

MarcoRossignoli commented Aug 20, 2019 •

edited

Loading

MarcoRossignoli Aug 21, 2019 •

edited

Loading

MarcoRossignoli commented Sep 6, 2019 •

edited

Loading

MarcoRossignoli commented Sep 18, 2019 •

edited

Loading

adrianhara commented Sep 18, 2019 •

edited

Loading

MarcoRossignoli commented Sep 18, 2019 •

edited

Loading

adrianhara commented Sep 18, 2019 •

edited

Loading

adrianhara commented Sep 18, 2019 •

edited

Loading

MarcoRossignoli commented Sep 18, 2019 •

edited

Loading