SteveGilham / altcover

Cross-platform coverage gathering and processing tool set for dotnet/.Net Framework and Mono
MIT License
494 stars 17 forks source link

Microsoft.NET.Test.Sdk 17.8.0 support #198

Closed latop2604 closed 5 months ago

latop2604 commented 7 months ago

Hello, I try to migrate my app from net7 to net8, but it seem to fail. I'm using altcover.global v8.6.95.

I use the following command:

# running in docker dotnet/sdk:8.0.100
dotnet build
./tools/altcover -e xunit -o=__saved_myapp --inplace -r myapp.opencover.xml -i myapp/src/MyApp.Tests/bin/Debug/net8.0/ -a GeneratedCodeAttribute -e MyApp.Tests
./tools/altcover Runner -x dotnet -r myapp/src/MyApp.Tests/bin/Debug/net8.0 -- test myapp/src/MyApp.Tests --no-build --no-restore

And the I got the following error :

Testhost process for source(s) '/builds/veepee/offerdiscovery/products/marketplace/monorepo/myapp/src/MyApp.Tests/bin/Debug/net8.0/MyApp.Tests.dll' exited with error: Unhandled exception. System.MethodAccessException: Attempt by method 'Microsoft.VisualStudio.TestPlatform.Execution.UiLanguageOverride..ctor()' to access method 'Microsoft.VisualStudio.TestPlatform.CoreUtilities.Helpers.EnvironmentVariableHelper..ctor()' failed.
   at Microsoft.VisualStudio.TestPlatform.Execution.UiLanguageOverride..ctor() in /_/src/Microsoft.TestPlatform.Execution.Shared/UILanguageOverride.cs:line 22
   at Microsoft.VisualStudio.TestPlatform.TestHost.Program.Run(String[] args) in /_/src/testhost.x86/Program.cs:line 54
   at Microsoft.VisualStudio.TestPlatform.TestHost.Program.Main(String[] args) in /_/src/testhost.x86/Program.cs:line 37
. Please check the diagnostic logs for more information.
Test Run Aborted.

The coverage seem to work with coverlet.

Do you think it could be related to altcover it self and if not, advise on what could be wrong ?

Thx

SteveGilham commented 7 months ago

The quick work-round is to exclude the Microsoft assemblies from instrumentation. Since it is unlikely that coverage data from those assemblies will be of interest, rather than being clutter, I've not dug deeply into what is going on with the cross-assembly calls in the test platform assemblies. Rather, when I first encountered it on one of the preview releases, I just added that exclusion to my own tests.

latop2604 commented 7 months ago

I tried to add -e Microsoft -e testhost -e xunit to instrumentation line (./tools/altcover -e xunit ...), but It does not fix the issue.

Is it what you suggested by "exclude the Microsoft assemblies from instrumentation" ?

latop2604 commented 7 months ago

I downgraded Microsoft.NET.Test.Sdk from 17.8.0 to 17.6.2 and it seem to work again

SteveGilham commented 7 months ago

The executive summary:

As the Microsoft code has no references to your code, you want -s testhost -s Microsoft; the difference being that -s leaves the excluded assemblies completely alone, whereas -e is a (slightly obsolescing) work-round that does interfere with them.

Background information: Having repro'd the issue again and decompiled the affected assemblies, it turns out that it is, as I had guessed, to do with assembly strong-names -- the code in testhost.dll that throws is invoking a contructor in Microsoft.TestPlatform.CoreUtilities.dll that it has access to only via an InternalsVisibleToAttribute; that attribute of course implies the full assembly name for testhost.dll, including the strong-name.

The -e filter is itself a strong-naming workround, rewriting assembly references in the nominated files, at the cost of also rewriting their strong names, and is intended for libraries that link your code under test directly (e.g. for excluding unit test code from coverage when applying a non-production strong-name during test).

Obviously, not having the Microsoft strong-name key means not being able to rewrite the assembly with the original strong name, thus breaking the InternalsVisibleTo.

AltCover doesn't rewrite those attributes, though it could, because I've not yet found an appropriate use-case; here the correct thing to do is exclude those linked assemblies.

SteveGilham commented 7 months ago

Added work item to update [InternalsVisibleTo]

ctuesley commented 7 months ago

If you are building using the dotnet.exe test command adding the flag /p:AltCoverAssemblyFilter="testhost|Microsoft" seems to resolve it also as per SteveGilham comments but using the dotnet test syntax.

SteveGilham commented 6 months ago

The general issue of which this is a specific case should be resolved in release v8.6.125.