-
Notifications
You must be signed in to change notification settings - Fork 344
[WIP]Add coverlet smoke test #2330
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| namespace Microsoft.TestPlatform.SmokeTests | ||
| { | ||
| using Microsoft.TestPlatform.TestUtilities; | ||
| using Microsoft.VisualStudio.TestTools.UnitTesting; | ||
| using System; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using System.Reflection; | ||
|
|
||
| [TestClass] | ||
| public class DataCollectorTestsCoverlets : IntegrationTestBase | ||
| { | ||
| [TestMethod] | ||
| public void RunCoverletCoverage() | ||
| { | ||
| string projectFileName = this.GetProjectAssetFullPath("CoverletCoverageTestProject.csproj", "CoverletCoverageTestProject.csproj"); | ||
MarcoRossignoli marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| string logId = Guid.NewGuid().ToString("N"); | ||
| this.InvokeDotnetTest($"--collect:\"XPlat Code Coverage\" {projectFileName} --diag:coverletcoverage.{logId}.log"); | ||
| var currentDir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); | ||
|
|
||
| // Verify vstest.console.dll CollectArgumentProcessor fix codeBase for coverlet package | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what this comment means, could you rephrase it to be more concrete, or explain to me what it means in a comment here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With this PR #2288 we removed custom code to allow coverlet lib to load in-proc collector. |
||
| var log = Directory.GetFiles(currentDir, $"coverletcoverage.{logId}.log").Single(); | ||
| Assert.IsTrue(File.ReadAllText(log).Contains("CoverletDataCollector in-process codeBase path")); | ||
|
|
||
| // Verify out-of-proc coverlet collector load | ||
| var dataCollectorLog = Directory.GetFiles(currentDir, $"coverletcoverage.{logId}.datacollector*log").Single(); | ||
| Assert.IsTrue(File.ReadAllText(dataCollectorLog).Contains("[coverlet]Initializing CoverletCoverageDataCollector")); | ||
|
|
||
| // Verify in-proc coverlet collector load | ||
| var hostLog = Directory.GetFiles(currentDir, $"coverletcoverage.{logId}.host*log").Single(); | ||
| Assert.IsTrue(File.ReadAllText(hostLog).Contains("[coverlet]Initialize CoverletInProcDataCollector")); | ||
|
|
||
| // Verify default coverage file is generated | ||
| this.StdOutputContains("coverage.cobertura.xml"); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <!-- Imports Common TestAssets props. --> | ||
| <Import Project="..\..\..\scripts\build\TestAssets.props" /> | ||
| <Import Project="..\..\..\scripts\build\TestPlatform.Dependencies.props" /> | ||
| <!-- Package dependency versions --> | ||
|
|
||
| <PropertyGroup> | ||
| <TargetFramework>netcoreapp2.1</TargetFramework> | ||
| <IsPackable>false</IsPackable> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0" /> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should use the variables from the TestPlatform.Dependencies to keep the versions up to date. I am currently making changes to run the acceptance tests against the version that is built in the repo, not against the previous version. We will need to rebase this on top of the other change to allow this test to pass, othewise it would use a way old version of the dependencies.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok I'll rebase when update will be in place...or do you want that I create a new prop
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's not necessary I would have to go and remove that variable afterwards. I am fighting with the acceptance tests to pass on the server, but hopefully it won't take more than a day more to get it to pass... 🤞 (or at least I hope because it is boring and exhausting 😁 )
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😆 🤞
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #2340 is now in master
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great rebasing!Any news on strange error #2330 (comment)? |
||
| <PackageReference Include="xunit" Version="2.4.0" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" Version="2.4.0" /> | ||
| <PackageReference Include="coverlet.collector" Version="1.2.0" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| using System; | ||
| using Xunit; | ||
|
|
||
| namespace CoverletCoverageTestProject | ||
| { | ||
| public class UnitTest1 | ||
| { | ||
| [Fact] | ||
| public void Test1() | ||
| { | ||
|
|
||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During my current work on the acceptance tests I found that this is falling short, because I am patching the task definition and the build but not the console and testhost. Would this be able to run via the InvokeVstest (or the other alike methods that run vsconsole but not dotnet test)? I tried taking the contents of the package that we ultimately insert into
dotnet test(the CLI package), and it broke the runner, and I would rather avoid copying "random" dlls into the local copy of dotnet because then we can't really be sure about what we are testing against and if it reflects the real world.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot use InvokeVsTest...because the base path of coverlet directory(nuget package) is injected through msbuild prop https://github.com/tonerdo/coverlet/blob/master/src/coverlet.collector/build/netstandard1.0/coverlet.collector.targets
I should pass
/testadapterpath:with path #2278 (comment) but it depends on where we run tests and on which plat(this is a less important issue, coverlet pack at the moment has got only one tfm netstandard1.0)Do you know a way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's figure that out later, imho finding the adapter should not be that hard, there will be some tests already doing it. But I would definitely love to get the dotnet test working with the currently built taks + console + testhost, because testing dotnet test more extensively would be nice. Maybe I was just doing something wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take a look if
InvokeVstestis used by other tests.