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.

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

Coverlet in-process collector is not loaded for version > 1.0.0 #2221

Merged
merged 8 commits into from
Nov 13, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions TestPlatform.sln
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio 15
VisualStudioVersion = 15.0.28307.779
# Visual Studio Version 16
VisualStudioVersion = 16.0.29319.158
MinimumVisualStudioVersion = 10.0.40219.1
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{ED0C35EB-7F31-4841-A24F-8EB708FFA959}"
EndProject
Expand Down Expand Up @@ -179,6 +179,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.TestPlatform.Exte
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests", "test\Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests\Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests.csproj", "{41248B96-6E15-4E5E-A78F-859897676814}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "coverlet.collector", "test\TestAssets\coverlet.collector\coverlet.collector.csproj", "{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -897,6 +899,18 @@ Global
{41248B96-6E15-4E5E-A78F-859897676814}.Release|x64.Build.0 = Release|Any CPU
{41248B96-6E15-4E5E-A78F-859897676814}.Release|x86.ActiveCfg = Release|Any CPU
{41248B96-6E15-4E5E-A78F-859897676814}.Release|x86.Build.0 = Release|Any CPU
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}.Debug|Any CPU.Build.0 = Debug|Any CPU
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}.Debug|x64.ActiveCfg = Debug|Any CPU
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}.Debug|x64.Build.0 = Debug|Any CPU
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}.Debug|x86.ActiveCfg = Debug|Any CPU
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}.Debug|x86.Build.0 = Debug|Any CPU
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}.Release|Any CPU.ActiveCfg = Release|Any CPU
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}.Release|Any CPU.Build.0 = Release|Any CPU
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}.Release|x64.ActiveCfg = Release|Any CPU
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}.Release|x64.Build.0 = Release|Any CPU
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}.Release|x86.ActiveCfg = Release|Any CPU
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3}.Release|x86.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -973,6 +987,7 @@ Global
{D16ACC60-52F8-4912-8870-5733A9F6852D} = {8DA7CBD9-F17E-41B6-90C4-CFF55848A25A}
{236A71E3-01DA-4679-9DFF-16A8E079ACFF} = {5E7F18A8-F843-4C8A-AB02-4C7D9205C6CF}
{41248B96-6E15-4E5E-A78F-859897676814} = {020E15EA-731F-4667-95AF-226671E0C3AE}
{F1D8630D-97D5-4CD7-BC18-A5E1779FA6E3} = {8DA7CBD9-F17E-41B6-90C4-CFF55848A25A}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {0541B30C-FF51-4E28-B172-83F5F3934BCD}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Common/Resources/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@
<value>Incorrect format for TestCaseFilter {0}. Specify the correct format and try again. Note that the incorrect format can lead to no test getting executed.</value>
</data>
<data name="UnableToFetchUriString" xml:space="preserve">
<value>Unable to find a datacollector with friendly name '[0}'.</value>
<value>Unable to find a datacollector with friendly name '{0}'.</value>
</data>
<data name="VSInstallationNotFound" xml:space="preserve">
<value>This option works only with vstest.console.exe installed as part of Visual Studio.</value>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@
<note></note>
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<note />
</trans-unit>
<trans-unit id="UnableToFetchUriString">
<source>Unable to find a datacollector with friendly name '[0}'.</source>
<source>Unable to find a datacollector with friendly name '{0}'.</source>
<target state="new">Unable to find a datacollector with friendly name '[0}'.</target>
<note></note>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,5 +329,15 @@ internal static class Constants
/// Test sources property name
/// </summary>
public const string TestSourcesPropertyName = "TestSources";

/// <summary>
/// Coverlet inproc data collector codebase
/// </summary>
public const string CoverletDataCollectorCodebase = "coverlet.collector.dll";

/// <summary>
/// Coverlet inproc data collector type name
/// </summary>
public const string CoverletDataCollectorTypeName = "Coverlet.Collector.DataCollection.CoverletInProcDataCollector";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection
{
using System;
using System.IO;
using System.Linq;
using System.Reflection;

Expand Down Expand Up @@ -67,10 +68,20 @@ internal InProcDataCollector(string codeBase, string assemblyQualifiedName, Type
this.assemblyLoadContext = assemblyLoadContext;

var assembly = this.LoadInProcDataCollectorExtension(codeBase);
this.dataCollectorType =
assembly?.GetTypes()
.FirstOrDefault(x => x.AssemblyQualifiedName.Equals(assemblyQualifiedName) && interfaceTypeInfo.IsAssignableFrom(x.GetTypeInfo()));

Func<Type, bool> filterPredicate;
if (Path.GetFileName(codeBase) == Constants.CoverletDataCollectorCodebase)
{
// If we're loading coverlet collector we skip to check the version of assembly
// to allow upgrade throught nuget package
filterPredicate = (x) => x.FullName.Equals(Constants.CoverletDataCollectorTypeName) && interfaceTypeInfo.IsAssignableFrom(x.GetTypeInfo());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mayankbansal018 Should we make this default ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think no, If I'm not mistaken that aqn is read from possible runsetting file so a user should have that assembly in local folder. The issue with coverlet is that we(you) inject hardcoded aqn if user provide --collect:"XPlat Code Coverage".
It works well for user provided collectors because there is always a runsettings file provided, but with coverlet don't, so we need a way to make it works for every version without runsettings file.
So my idea is to check only "trusted" type/lib name only for coverlet.
BTW I could miss something in the big picture.

}
else
{
filterPredicate = (x) => x.AssemblyQualifiedName.Equals(assemblyQualifiedName) && interfaceTypeInfo.IsAssignableFrom(x.GetTypeInfo());
}

this.dataCollectorType = assembly?.GetTypes().FirstOrDefault(filterPredicate);
this.AssemblyQualifiedName = this.dataCollectorType?.AssemblyQualifiedName;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace TestPlatform.CrossPlatEngine.UnitTests.DataCollection
{
using System.IO;
using System.Reflection;

using Coverlet.Collector.DataCollection;
using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection;
using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection.Interfaces;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection;
Expand Down Expand Up @@ -64,7 +64,7 @@ public void InProcDataCollectorShouldNotThrowExceptionIfAssemblyDoesNotContainAn
public void InProcDataCollectorShouldInitializeIfAssemblyContainsAnyInProcDataCollector()
{
var typeInfo = typeof(TestableInProcDataCollector).GetTypeInfo();

this.assemblyLoadContext.Setup(alc => alc.LoadAssemblyFromPath(It.IsAny<string>()))
.Returns(typeInfo.Assembly);

Expand All @@ -79,6 +79,27 @@ public void InProcDataCollectorShouldInitializeIfAssemblyContainsAnyInProcDataCo
Assert.AreEqual(this.inProcDataCollector.AssemblyQualifiedName, typeInfo.AssemblyQualifiedName);
}

[TestMethod]
public void InProcDataCollectorLoadCoverlet()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test verify that also if aqn shows version 1.0.0.0 we correctly load a version 9.9.9.9 of lib

{
var typeInfo = typeof(CoverletInProcDataCollector).GetTypeInfo();

Assert.AreEqual("9.9.9.9", typeInfo.Assembly.GetName().Version.ToString());

this.assemblyLoadContext.Setup(alc => alc.LoadAssemblyFromPath(It.IsAny<string>()))
.Returns(typeInfo.Assembly);

this.inProcDataCollector = new InProcDataCollector(
typeInfo.Assembly.Location,
"Coverlet.Collector.DataCollection.CoverletInProcDataCollector, coverlet.collector, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null",
typeof(InProcDataCollection).GetTypeInfo(),
string.Empty,
this.assemblyLoadContext.Object);

Assert.IsNotNull(this.inProcDataCollector.AssemblyQualifiedName);
Assert.AreEqual(this.inProcDataCollector.AssemblyQualifiedName, typeInfo.AssemblyQualifiedName);
}

private class TestableInProcDataCollector : InProcDataCollection
{
public void Initialize(IDataCollectionSink dataCollectionSink)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
<ProjectReference Include="..\..\src\Microsoft.TestPlatform.CoreUtilities\Microsoft.TestPlatform.CoreUtilities.csproj">
<FromP2P>true</FromP2P>
</ProjectReference>
<ProjectReference Include="..\TestAssets\coverlet.collector\coverlet.collector.csproj" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'net451' ">
<Reference Include="System.Runtime" />
Expand Down
46 changes: 46 additions & 0 deletions test/TestAssets/coverlet.collector/CoverletInProcDataCollector.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Reflection;
using System;

using Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollector.InProcDataCollector;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.InProcDataCollector;

[assembly: AssemblyKeyFile("key.snk")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this key file for?

Copy link
Contributor Author

@MarcoRossignoli MarcoRossignoli Oct 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly 2 reason

  1. Without signed assembly I cannot compile I get an error that force me to have a strong name asm.
    image

  2. After this merge we have on backlog this Sign collector and update visibility coverlet-coverage/coverlet#566 so I'll update this key here and will amend test and alg to check also "public key token".

[assembly: AssemblyVersion("9.9.9.9")]

namespace Coverlet.Collector.DataCollection
{
// This class MUST have the same full name as
// https://github.com/tonerdo/coverlet/blob/master/src/coverlet.collector/InProcDataCollection/CoverletInProcDataCollector.cs
// to mimic real behaviour
public class CoverletInProcDataCollector : InProcDataCollection
{
public void Initialize(IDataCollectionSink dataCollectionSink)
{
throw new NotImplementedException();
}

public void TestCaseEnd(TestCaseEndArgs testCaseEndArgs)
{
throw new NotImplementedException();
}

public void TestCaseStart(TestCaseStartArgs testCaseStartArgs)
{
throw new NotImplementedException();
}

public void TestSessionEnd(TestSessionEndArgs testSessionEndArgs)
{
throw new NotImplementedException();
}

public void TestSessionStart(TestSessionStartArgs testSessionStartArgs)
{
throw new NotImplementedException();
}
}
}
12 changes: 12 additions & 0 deletions test/TestAssets/coverlet.collector/coverlet.collector.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>netcoreapp2.1;net451</TargetFrameworks>
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\..\..\src\Microsoft.TestPlatform.ObjectModel\Microsoft.TestPlatform.ObjectModel.csproj" />
</ItemGroup>

</Project>
Binary file added test/TestAssets/coverlet.collector/key.snk
Binary file not shown.