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.

Sign up for GitHub

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

Jump to bottom

Working with Expecto Test Runner #11

Closed
TheAngryByrd opened this issue Apr 16, 2018 · 15 comments
Closed

Working with Expecto Test Runner #11

TheAngryByrd opened this issue Apr 16, 2018 · 15 comments
Labels
enhancement

Comments

@TheAngryByrd
Copy link

I'm just opening here as initial starting point. We can move to other repo's based on if they need PR's.

So I was trying to mimic coverlet's msbuild targets as much as I could like https://github.com/TheAngryByrd/ExpectoAltCover-Test/blob/master/tests/Directory.Build.targets. However when I go to run dotnet test I'm getting an issue with the Expecto Test adapter

An exception occurred while invoking executor 'executor://yolodev/expecto': Symbols were found but are not matching the assembly

This error seems to come from inside Mono.Cecil. https://github.com/jbevain/cecil/blob/master/Mono.Cecil/ModuleDefinition.cs#L1077

Repo: https://github.com/TheAngryByrd/ExpectoAltCover-Test

just run dotnet test in the tests folder to see the behavior.

Do you have any input as to what needs to change to make this work?

Thanks!

cc: @Alxandr @haf @AnthonyLloyd
@haf
Copy link

haf commented Apr 16, 2018

I think @MNie has been active with the .net core templates and might have an opinion on this.

I personally don't know, to be honest.

@Alxandr
Copy link

Alxandr commented Apr 16, 2018

My (somewhat unqualified) guess is that this is not (really) related to expecto, nor the test runner, but coverlet doing something weird in post-build. To test this theory, I would take the test runner out of the equation (makes things much harder to test ironically). Can you try the following?

  1. Add an entrypoint to the test project that calls the standard expecto "run tests" method (if you don't already have one).
  2. Build the project, and run the MSBuild task that does the DLL rewriting.
  3. Manually run the rewritten test dll (using dotnet run).

See if it crashes and if you get more information. Unfortunately I won't be able to look too much at this for the next few days.

@TheAngryByrd
Copy link
Author

I’ve tried it with dotnet run and it seems to work fine.

@Alxandr
Copy link

Alxandr commented Apr 16, 2018

And you get instrumentation?

@TheAngryByrd
Copy link
Author

TheAngryByrd commented Apr 16, 2018 •
edited
Loading

Yes


<Target Name="MyTesting" DependsOnTargets="Restore;BuildProject">
      <!-- Altcover fails if _Reports folder doesn't exist -->
     <MakeDir  
            Directories="$(OutputPath)/netcoreapp2.0/_Reports"/>   
      <AltCover.Prepare 
        InputDirectory="$(OutputPath)/netcoreapp2.0"  
        SymbolDirectories="$(OutputPath)/netcoreapp2.0"  
        OutputDirectory="$(OutputPath)/netcoreapp2.0/__Saved$([System.DateTime]::UtcNow.ToString().Replace(':','-').Replace('/','-').Replace(' ','+'))" 
        XmlReport="$(OutputPath)/netcoreapp2.0/_Reports/MSBuildTest.xml"
        CallContext="@(CallContext)"
        />
      <Exec Command="dotnet run -f netcoreapp2.0 --no-build" ContinueOnError="WarnAndContinue"/>
      <AltCover.Collect 
        RecorderDirectory="$(OutputPath)/netcoreapp2.0"   
        LcovReport="./lcov.info"
        />  
  </Target>

dotnet msbuild /t:MyTesting
Microsoft (R) Build Engine version 15.6.82.30579 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

/usr/local/share/dotnet/sdk/2.1.101/Sdks/Microsoft.NET.Sdk/build/Microsoft.NET.Sdk.DefaultItems.targets(198,5): warning : A PackageReference for 'Microsoft.NETCore.App' was included in your project. This package is implicitly referenced by the .NET SDK and you do not typically need to reference it from your project. For more information, see https://aka.ms/sdkimplicitrefs [/Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/tests.fsproj]
  Restore completed in 39.14 ms for /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/tests.fsproj.
  Restore completed in 17.43 ms for /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/tests.fsproj.
Build started, please wait...
  tests -> /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/bin/Debug/netcoreapp2.0/tests.dll
Build completed.

  Creating folder /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/bin/Debug/netcoreapp2.0/__Saved4-16-18+1-28-56+PM
  Saving files to /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/bin/Debug/netcoreapp2.0/__Saved4-16-18+1-28-56+PM
  Instrumenting files in /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/bin/Debug/netcoreapp2.0
     => /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/bin/Debug/netcoreapp2.0/__Saved4-16-18+1-28-56+PM/AltCover.Recorder.g.dll
     => /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/bin/Debug/netcoreapp2.0/__Saved4-16-18+1-28-56+PM/tests.dll

  Coverage Report: /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/bin/Debug/netcoreapp2.0/_Reports/MSBuildTest.xml


      /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/bin/Debug/netcoreapp2.0/tests.dll
                  <=  tests, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null
      /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/bin/Debug/netcoreapp2.0/AltCover.Recorder.g.dll
                  <=  AltCover.Recorder.g, Version=3.0.0.0, Culture=neutral, PublicKeyToken=c02b1a9f5b7cade8
  [09:28:58 INF] EXPECTO? Running tests... <Expecto>
  [09:28:58 INF] EXPECTO! 2 tests run in 00:00:00.0408560 for samples – 1 passed, 1 ignored, 0 failed, 0 errored. ᕙ໒( ˵ ಠ ╭͜ʖ╮ ಠೃ ˵ )७ᕗ <Expecto>
  ... /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/bin/Debug/netcoreapp2.0/_Reports/MSBuildTest.xml.0.acv
  ... /Users/myuser/Documents/GitHub/altcoverIssues/expectoTestRunnerIssue/tests/bin/Debug/netcoreapp2.0/_Reports/MSBuildTest.xml.1.acv
  8 visits recorded
  Coverage statistics flushing took 0.068 seconds
  Visited Classes 4 of 57 (7.02)
  Visited Methods 4 of 95 (4.21)
  Visited Points 5 of 217 (2.3)
  Visited Branches 0 of 145 (0)

  ==== Alternative Results (includes all methods including those without corresponding source) ====
  Alternative Visited Classes 4 of 59 (6.78)
  Alternative Visited Methods 4 of 122 (3.28)

@SteveGilham
Copy link
Owner

I recall at some point finding that Cecil was being unreliable in writing out symbols for the instrumented assemblies, but I can't remember where I left things -- I'll have to check through history to find the details. At the time, I didn't see a use-case for them, other than completeness.

@SteveGilham
Copy link
Owner

Relevant commentary from the code that I made as a note-to-self

    // Assembly with symbols pdb writing fails on .net core on Windows when writing with
    // System.NullReferenceException : Object reference not set to an instance of an object.
    // from deep inside Cecil -- but this works!!

where "this" is writing out symbols as .mdb; and

    // Assembly with pdb writing fails on mono on Windows when writing with
    // System.NullReferenceException : Object reference not set to an instance of an object.
    // from deep inside Cecil
    // Pdb writing fails on mono on non-Windows with
    // System.DllNotFoundException : ole32.dll
    //  at (wrapper managed-to-native) Mono.Cecil.Pdb.SymWriter:CoCreateInstance
    // Mdb writing now fails in .net framework, it throws
    // Mono.CompilerServices.SymbolWriter.MonoSymbolFileException :
    // Exception of type 'Mono.CompilerServices.SymbolWriter.MonoSymbolFileException' was thrown.
    // If there are portable .pdbs on mono, those fail to write, too with
    // Mono.CompilerServices.SymbolWriter.MonoSymbolFileException :
    // Exception of type 'Mono.CompilerServices.SymbolWriter.MonoSymbolFileException' was thrown.

These comments were made in January, apart from the last bit about portable .pdbs on mono, at the start of March; which means I've not re-tested since Cecil 0.10 came out.

@SteveGilham
Copy link
Owner

I just made the experiment of setting the .net core build to write .pdb rather than .mdb, and Cecil 0.10 final still gives the same issue

 System.NullReferenceException : Object reference not set to an instance of an object.
Stack Trace:
   at Mono.Cecil.MetadataBuilder.AddLocalConstants(ScopeDebugInformation scope)
   at Mono.Cecil.MetadataBuilder.AddLocalScope(MethodDebugInformation method_info, ScopeDebugInformation scope)
   at Mono.Cecil.MetadataBuilder.AddMethodDebugInformation(MethodDebugInformation method_info)
   at Mono.Cecil.Cil.CodeWriter.WriteResolvedMethodBody(MethodDefinition method)
   at Mono.Cecil.Cil.CodeWriter.WriteMethodBody(MethodDefinition method)
   at Mono.Cecil.MetadataBuilder.AddMethod(MethodDefinition method)
   at Mono.Cecil.MetadataBuilder.AddMethods(TypeDefinition type)
   at Mono.Cecil.MetadataBuilder.AddType(TypeDefinition type)
   at Mono.Cecil.MetadataBuilder.AddNestedTypes(TypeDefinition type)
   at Mono.Cecil.MetadataBuilder.AddType(TypeDefinition type)
   at Mono.Cecil.MetadataBuilder.AddTypes()
   at Mono.Cecil.MetadataBuilder.BuildTypes()
   at Mono.Cecil.MetadataBuilder.BuildModule()
   at Mono.Cecil.MetadataBuilder.BuildMetadata()
   at Mono.Cecil.ModuleWriter.<>c.<BuildMetadata>b__2_0(MetadataBuilder builder, MetadataReader _)
   at Mono.Cecil.ModuleDefinition.Read[TItem,TRet](TItem item, Func`3 read)
   at Mono.Cecil.ModuleWriter.BuildMetadata(ModuleDefinition module, MetadataBuilder metadata)
   at Mono.Cecil.ModuleWriter.Write(ModuleDefinition module, Disposable`1 stream, WriterParameters parameters)
   at Mono.Cecil.ModuleWriter.WriteModule(ModuleDefinition module, Disposable`1 stream, WriterParameters parameters)
   at Mono.Cecil.ModuleDefinition.Write(String fileName, WriterParameters parameters)
   at AltCover.Instrument.WriteAssembly(AssemblyDefinition assembly, String path)

@SteveGilham
Copy link
Owner

Having had reason to look more deeply into this, it's starting to look like maybe this is another manifestation of this issue -- a bug in Cecil 0.10 beta6, that seems possibly fixed in the final build, but which gets masked by the unit test framework.

@SteveGilham
Copy link
Owner

Having moved the related tests to xUnit, it now works with pdb in => pdb out, mdb in => mdb out for the .net core build (full fat framework isn't yet touched). Prerelease build 430 has this change in.

@TheAngryByrd
Copy link
Author

Seems to be working for me! Thanks!

@Alxandr
Copy link

Alxandr commented Apr 18, 2018

So it works with XUnit, but not the Expecto runner?

@SteveGilham
Copy link
Owner

Any of the unit tests involving symbol output were being skewed in .net core by the fact that NUnit drags in Cecil too, and the latest release of that is still on 0.10 beta6, and we don't have AppDomain isolation to protect from different assemblies with the same version. At root, it's a Cecil versioning bug -- the assembly contracts changed, but the version has stayed the same.

By moving those tests to XUnit, I'm actually able to test what Cecil 0.10 final is doing; and I can once more (as was the state of play at the turn of the year) make AltCover write out .pdb files for .pdb in, and .mdb files for .mdb in and see it working.

@SteveGilham
Copy link
Owner

The functional change (in amongst all the test and build furniture) is this one.

@SteveGilham SteveGilham added the ready to close Believed addressed, waiting to hear to the contrary label Apr 18, 2018
@SteveGilham
Copy link
Owner

In release 3.0.433

@SteveGilham SteveGilham removed the ready to close Believed addressed, waiting to hear to the contrary label Jul 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Alxandr @haf @TheAngryByrd @SteveGilham