Path is empty for VB My namespace #1073

daveMueller · 2021-01-28T01:11:35Z

closes #775

daveMueller · 2021-01-28T01:33:29Z

The issue is the My namespace in Visual Basic (My). Usually a sequence point has a reference to a source file like this.

In Visual Basic the compiler injects the My classes into the assembly without a reference to a source file.

Maybe there are also other scenarios where a sequence point doesn't have a reference to a source file that we haven't encountered yet. So I created a PR that excludes code like this within an assembly.

But I could also think of a solution that really just filters out the My namespace in a Visual Basic assembly. Just let me know what you think.

MarcoRossignoli · 2021-01-28T08:15:37Z

Thx Dave, need to go deeper on it to understand if as you said make sense something for that injected namespace.

Can we add a new VB project and create a test?Is it too complex?
Something like coverlet.tests.projectsample.vbmynamespace here https://github.com/coverlet-coverage/coverlet/tree/master/test

daveMueller · 2021-01-29T09:02:11Z

Can we add a new VB project and create a test?Is it too complex?
Something like coverlet.tests.projectsample.vbmynamespace here https://github.com/coverlet-coverage/coverlet/tree/master/test

I will give it a try.
But nevertheless I think instrumentation of IL code that doesn't have a reference to a source file doesn't make much sense for the coverage.

MarcoRossignoli · 2021-01-29T17:47:47Z

But nevertheless I think instrumentation of IL code that doesn't have a reference to a source file doesn't make much sense for the coverage.

I agree...but we need to be pretty sure to not break something, so I'm ok with the update but I'd like to have a test in place for future issue on it.

src/coverlet.core/Instrumentation/Instrumenter.cs

MarcoRossignoli · 2021-01-30T14:05:07Z

test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs

@@ -597,7 +597,7 @@ public int SampleMethod()
            }
            else
            {
-                Assert.NotEmpty(result.Documents);
+                Assert.Empty(result.Documents);


mmm is this correct?(didn't tested on my own but seem we expect opposite behavior)

The compiled assembly in this test doesn't have a reference to a source file an thus the result is always empty. Before my change in the Instrumenter this test returned a result with a empty document source which is exactly what would create the bug in the report generation.

After my change such a InstrumenterResult isn't created anymore and that's why now result.Documents is also empty here.

To make this test the way it was intended the dynamically created assembly must contain sequence points with a reference to the source file. But what ever I tried here the assemblies sequence points never had source file information.

coverlet/test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs

Line 556 in 4bbcd0b

var emitResult = compilation.Emit(outputStream, pdbStream, options: isWindows ? emitOptions : emitOptions.WithDebugInformationFormat(DebugInformationFormat.PortablePdb));

The assemblies sequence points were always without source information.

got it...pls can you add a comment on top of that assert with this explanation for future reference?

daveMueller · 2021-02-05T09:28:43Z

@MarcoRossignoli It seems the My namespace in VB is only in .NET Framework projects. I think it isn't possible to add a project with .NET Framework as target to the coverlet solution? Or at least I couldn't do it without crashing the build jobs?

MarcoRossignoli · 2021-02-05T10:22:10Z

@MarcoRossignoli It seems the My namespace in VB is only in .NET Framework projects. I think it isn't possible to add a project with .NET Framework as target to the coverlet solution? Or at least I couldn't do it without crashing the build jobs?

I think it's possible, AFAIK .NET Framework project can be compiled on all plat through dotnet driver, I have some doubt on type resolution for Mac/Linux but in those case we can skip tests. Give it a try and let me know.

If it'll be too hard to do, we can merge and open a ticket to follow up this.

daveMueller · 2021-02-05T13:05:32Z

@MarcoRossignoli I'm still working on this but it seems that Linux and Mac build jobs can't build the project. Windows build job is green now. But for the others I'm getting (The reference assemblies for .NETFramework,Version=v4.7.2 were not found. To resolve this, install the Developer Pack (SDK/Targeting Pack) for this framework version or retarget your application.)

MarcoRossignoli · 2021-02-05T13:13:19Z

Try with this https://andrewlock.net/using-reference-assemblies-to-build-net-framework-libararies-on-linux-without-mono/

https://www.nuget.org/packages/Microsoft.NETFramework.ReferenceAssemblies/

daveMueller · 2021-02-06T01:56:23Z

Thanks, but now I get a vbc : error BC2017: could not find library 'Microsoft.VisualBasic.dll' which is weird because the ReferenceAssemblies package contains the dll. Somehow the build doesn't look in the right place? According to the documentation (docs) this should work. Any other ideas? Otherwise I'll open a issue in the dotnet repo.

MarcoRossignoli · 2021-02-06T08:13:01Z

Ok Dave open an issue on the dotnet repo, I never used that ref I only know about the existence

MarcoRossignoli · 2021-03-06T14:51:25Z

/azp run

azure-pipelines · 2021-03-06T14:51:34Z

Azure Pipelines successfully started running 1 pipeline(s).

nk111 · 2021-10-05T18:05:39Z

Hi, I've been trying to get a coverage report for a .Net Framework project displayed in Devops for 3 days. After updating all possible tools on our build agent, the test is finally running via DotNetCli. But unfortunately I also get the "The path is empty" error at the end. It is a VB project.
Can this PR please be completed?

MarcoRossignoli · 2021-10-05T20:14:12Z

src/coverlet.core/Instrumentation/Instrumenter.cs

@@ -510,7 +510,12 @@ private void InstrumentType(TypeDefinition type)

        private void InstrumentMethod(MethodDefinition method)
        {
+            //_logger.LogInformation($"Instrumenting method '{method.FullName}' in module '{_module}'.");


@daveMueller I see this line as commented, is it on purpose?

I'm pretty sure I just used it for debugging. So no need to keep this in the log.

MarcoRossignoli · 2021-10-05T20:16:51Z

@daveMueller do you plan to move on this one or can I take over? It's pretty old and I think we can exclude tests from Mac/Linux and run failing one only on Win.

daveMueller · 2021-10-05T23:55:18Z

@MarcoRossignoli I totally forgot about this one. I'm currently really busy so I would be glad if you could take over. I'm fine with excluding the test for Mac/Linux. Thank you.

MarcoRossignoli · 2021-10-06T06:02:26Z

Great, thanks a lot for the effort here!

# Conflicts: # coverlet.sln # src/coverlet.core/Instrumentation/Instrumenter.cs # test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs # test/coverlet.core.tests/coverlet.core.tests.csproj

# Conflicts: # Documentation/Changelog.md

MarcoRossignoli · 2023-01-22T07:44:20Z

Is this good to merge now?

daveMueller · 2023-01-24T00:51:49Z

@MarcoRossignoli yes I worked on it again and made the integration test run on all platforms now. So this is ready for review/merge. I asked in the issue (#775 (comment)) if someone can give it a try.

MarcoRossignoli · 2023-01-24T06:41:22Z

Thanks @daveMueller

MarcoRossignoli changed the title ~~775 Path is empty~~ Path is empty for VB My namespace Jan 28, 2021

MarcoRossignoli added bug Something isn't working tenet-reporters Issue related to coverage output files(reporters) labels Jan 28, 2021

MarcoRossignoli reviewed Jan 30, 2021

View reviewed changes

src/coverlet.core/Instrumentation/Instrumenter.cs Show resolved Hide resolved

MarcoRossignoli reviewed Jan 30, 2021

View reviewed changes

daveMueller added 4 commits February 5, 2021 11:32

exclude type that doesn't have a source file

2795674

adapted test

8c18caa

added log and test

883fc15

added nuget.build.tasks.pack

02c92d6

daveMueller force-pushed the 775_PathIsEmpty branch from 3f11164 to 02c92d6 Compare February 5, 2021 10:32

daveMueller added 3 commits February 5, 2021 11:42

test

8088969

fixed test

812aba8

nit

ff030c9

added reference assemblies

410d8d2

daveMueller mentioned this pull request Feb 6, 2021

dotnet build with Microsoft.NETFramework.ReferenceAssemblies can't find 'Microsoft.VisualBasic.dll' dotnet/sdk#15712

Closed

petli mentioned this pull request May 18, 2021

Skip fsharp construct with unknown source #1165

Merged

MarcoRossignoli reviewed Oct 5, 2021

View reviewed changes

daveMueller and others added 5 commits January 18, 2023 00:10

Merge branch 'upstream/master' into 775_PathIsEmpty

aed9be4

# Conflicts: # coverlet.sln # src/coverlet.core/Instrumentation/Instrumenter.cs # test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs # test/coverlet.core.tests/coverlet.core.tests.csproj

fix test

a004bea

update test project to use new sdk style project file

6197539

update changelog

d16002b

Merge branch 'upstream/master' into 775_PathIsEmpty

9d8dc85

# Conflicts: # Documentation/Changelog.md

David Mueller IWS added 2 commits January 24, 2023 01:19

adapted unit test

cf608b3

nit

a6a19fe

MarcoRossignoli approved these changes Jan 24, 2023

View reviewed changes

MarcoRossignoli merged commit a014bf0 into coverlet-coverage:master Jan 24, 2023

daveMueller deleted the 775_PathIsEmpty branch September 12, 2023 22:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Path is empty for VB My namespace #1073

Path is empty for VB My namespace #1073

daveMueller commented Jan 28, 2021

daveMueller commented Jan 28, 2021

MarcoRossignoli commented Jan 28, 2021 •

edited

daveMueller commented Jan 29, 2021

MarcoRossignoli commented Jan 29, 2021

MarcoRossignoli Jan 30, 2021 •

edited

daveMueller Feb 5, 2021

MarcoRossignoli Feb 7, 2021

daveMueller commented Feb 5, 2021 •

edited

MarcoRossignoli commented Feb 5, 2021 •

edited

daveMueller commented Feb 5, 2021

MarcoRossignoli commented Feb 5, 2021

daveMueller commented Feb 6, 2021

MarcoRossignoli commented Feb 6, 2021

MarcoRossignoli commented Mar 6, 2021

azure-pipelines bot commented Mar 6, 2021

nk111 commented Oct 5, 2021 •

edited

MarcoRossignoli Oct 5, 2021

daveMueller Oct 5, 2021

MarcoRossignoli commented Oct 5, 2021

daveMueller commented Oct 5, 2021

MarcoRossignoli commented Oct 6, 2021

MarcoRossignoli commented Jan 22, 2023

daveMueller commented Jan 24, 2023

MarcoRossignoli commented Jan 24, 2023

Path is empty for VB My namespace #1073

Path is empty for VB My namespace #1073

Conversation

daveMueller commented Jan 28, 2021

daveMueller commented Jan 28, 2021

MarcoRossignoli commented Jan 28, 2021 • edited

daveMueller commented Jan 29, 2021

MarcoRossignoli commented Jan 29, 2021

MarcoRossignoli Jan 30, 2021 • edited

Choose a reason for hiding this comment

daveMueller Feb 5, 2021

Choose a reason for hiding this comment

MarcoRossignoli Feb 7, 2021

Choose a reason for hiding this comment

daveMueller commented Feb 5, 2021 • edited

MarcoRossignoli commented Feb 5, 2021 • edited

daveMueller commented Feb 5, 2021

MarcoRossignoli commented Feb 5, 2021

daveMueller commented Feb 6, 2021

MarcoRossignoli commented Feb 6, 2021

MarcoRossignoli commented Mar 6, 2021

azure-pipelines bot commented Mar 6, 2021

nk111 commented Oct 5, 2021 • edited

MarcoRossignoli Oct 5, 2021

Choose a reason for hiding this comment

daveMueller Oct 5, 2021

Choose a reason for hiding this comment

MarcoRossignoli commented Oct 5, 2021

daveMueller commented Oct 5, 2021

MarcoRossignoli commented Oct 6, 2021

MarcoRossignoli commented Jan 22, 2023

daveMueller commented Jan 24, 2023

MarcoRossignoli commented Jan 24, 2023

MarcoRossignoli commented Jan 28, 2021 •

edited

MarcoRossignoli Jan 30, 2021 •

edited

daveMueller commented Feb 5, 2021 •

edited

MarcoRossignoli commented Feb 5, 2021 •

edited

nk111 commented Oct 5, 2021 •

edited