Blame for vstest by vagisha-nidhi · Pull Request #915 · microsoft/vstest

vagisha-nidhi · 2017-07-06T06:14:35Z

This feature gets the faulty test case name in event of a test host crash of reason unknown.
There are various scenarios in which the test host crashes due to a faulty test case and the reason cannot be diagnosed.
This pull request gets the name of the test case which caused the crash and displays it on the console.
To enable the blame mode, use --Blame|/Blame

The usage is as follows -

'''
"vstest.console.exe" /Testadapterpath:"C:\TestProject\TestProject\bin\Debug\net46" C:\path\to\your\assembly.dll /Blame
'''

This feature gets the faulty test case name in event of a test host crash of reason unknown. There are various scenarios in which the test host crashes due to a faulty test case and the reason cannot be diagnosed. This pull request gets the name of the test case which caused the crash and displays it on the console. To enable the blame mode, use --Blame|/Blame

msftclas · 2017-07-06T06:14:39Z

@vagisha-nidhi,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

codito

Suggest a few changes.

codito · 2017-07-06T11:38:03Z

scripts/build.ps1

        Move-Item $coreCLR20PackageDir\$file $coreCLRExtensionsDir -Force
    }
+
+    # Publish Datacollector


Require change in build.sh

Should this assembly be added for signing?

codito · 2017-07-06T11:39:53Z

src/Microsoft.TestPlatform.BlameDataCollector/BlameCollector.cs

@@ -0,0 +1,147 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.


nit: is there a naming convention for extensions? TrxLogger is named as Microsoft.TestPlatform.Extensions.TrxLogger, should we follow M.TP.E.Blame?

codito · 2017-07-06T11:40:36Z

src/Microsoft.TestPlatform.BlameDataCollector/Microsoft.TestPlatform.BlameDataCollector.csproj

@@ -0,0 +1,58 @@
+<?xml version="1.0" encoding="utf-8"?>


Enable stylecop.

codito · 2017-07-06T11:41:00Z

src/Microsoft.TestPlatform.BlameDataCollector/Microsoft.TestPlatform.BlameDataCollector.csproj

+  </PropertyGroup>
+  <PropertyGroup>
+    <AssemblyName>Microsoft.TestPlatform.BlameDataCollector</AssemblyName>
+    <TargetFrameworks>netcoreapp1.0;netcoreapp2.0;net46</TargetFrameworks>


Why not build for netstandard only?

codito · 2017-07-06T11:41:14Z

src/Microsoft.TestPlatform.BlameDataCollector/Microsoft.TestPlatform.BlameDataCollector.csproj

+  <PropertyGroup>
+    <AssemblyName>Microsoft.TestPlatform.BlameDataCollector</AssemblyName>
+    <TargetFrameworks>netcoreapp1.0;netcoreapp2.0;net46</TargetFrameworks>
+    <WarningsAsErrors>true</WarningsAsErrors>


Already used in testplatform.settings.targets?

codito · 2017-07-06T12:02:18Z

src/Microsoft.TestPlatform.BlameDataCollector/BlameLogger.cs

+            ValidateArg.NotNull<object>(sender, "sender");
+            ValidateArg.NotNull<TestRunCompleteEventArgs>(e, "e");
+
+            if (!e.IsAborted) return;


nit: please wrap if clauses in parenthesis.

codito · 2017-07-06T12:03:42Z

src/Microsoft.TestPlatform.BlameDataCollector/BlameLogger.cs

+                if (attachmentSet.DisplayName.Equals(Constants.BlameDataCollectorName))
+                {
+                    var filepath = attachmentSet.Attachments[0].Uri.LocalPath;
+                    List<TestCase> testCaseList = this.blameReaderWriter.ReadTestSequence(filepath);


Can we use Enumerable.LastOrDefault?

codito · 2017-07-06T12:04:14Z

src/Microsoft.TestPlatform.BlameDataCollector/Constants.cs

+        /// <summary>
+        /// Test run Abort 
+        /// </summary>
+        public const string TestRunAbortReason = "The test run was aborted because the host process crashed unexpectedly while executing test ";


codito · 2017-07-06T12:04:29Z

src/Microsoft.TestPlatform.BlameDataCollector/Friends.cs

+
+#region Test Assemblies
+
+[assembly: InternalsVisibleTo("Microsoft.TestPlatform.BlameDataCollector.UnitTests, PublicKey=002400000480000094000000060200000024000052534131000400000100010007d1fa57c4aed9f0a32e84aa0faefd0de9e8fd6aec8f87fb03766c834c99921eb23be79ad9d5dcc1dd9ad236132102900b723cf980957fc4e177108fc607774f29e8320e92ea05ece4e821c0a5efe8f1645c4c0c93c1ab99285d622caa652c1dfad63d745d6f2de5f17e5eaf0fc4963d261c8a12436518206dc093344d5ad293")]


Not sure if this required.

codito · 2017-07-06T12:05:26Z

test/Microsoft.TestPlatform.BlameDataCollector.UnitTests/BlameCollectorTests.cs

+        }
+
+        [TestMethod]
+        public void TriggerTestCaseStartedHandlerShouldIncreaseTestStartCount()


This test is testing an internal implementation detail. We should care about the user behavior.

vagisha-nidhi · 2017-07-11T05:49:35Z

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs

+            // In case of crash testStartCount will be greater than testEndCount and we need to send write the sequence
+            if (this.testStartCount > this.testEndCount)
+            {
+                var filepath = Path.Combine(AppContext.BaseDirectory, Constants.AttachmentFileName);


Check filepath

vagisha-nidhi · 2017-07-11T05:50:11Z

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameLogger.cs

+        /// <summary>
+        /// Uri used to uniquely identify the Blame logger.
+        /// </summary>
+        public const string ExtensionUri = "logger://Microsoft/TestPlatform/Extensions/Blame";


vagisha-nidhi · 2017-07-11T05:51:06Z

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameLogger.cs

+                        if (testCaseList.Count > 0)
+                        {
+                            var testcase = testCaseList[testCaseList.Count - 1];
+                            return testcase.FullyQualifiedName;


LastOrDefault

vagisha-nidhi · 2017-07-11T05:51:54Z

...rm.Extensions.BlameDataCollector/Microsoft.TestPlatform.Extensions.BlameDataCollector.csproj

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


Enable stylecop

vagisha-nidhi · 2017-07-11T05:54:47Z

src/package/sign/sign.proj

      <!-- NetCoreExtensions -->
      <CoreAssembliesToSign Include="$(ArtifactsCoreDirectory)Extensions\Microsoft.VisualStudio.TestPlatform.Extensions.Trx.TestLogger.dll" />
 	  <CoreAssembliesToSign Include="$(ArtifactsCoreDirectory)Extensions\Microsoft.TestPlatform.TestHostRuntimeProvider.dll" />
+	  <CoreAssembliesToSign Include="$(ArtifactsCoreDirectory)Extensions\Microsoft.TestPlatform.Extensions.BlameDataCollector.dll" />


Add for net46

vagisha-nidhi added 2 commits July 5, 2017 01:51

Unit tests added and minor changes

af00f67

msftclas added the cla-already-signed label Jul 6, 2017

codito suggested changes Jul 6, 2017

View reviewed changes

vagisha-nidhi added 3 commits July 8, 2017 18:43

Suggested changes made

46f87dd

Spaces removed

6922cb9

Build updated

1e9b6ca

vagisha-nidhi commented Jul 11, 2017

View reviewed changes

vagisha-nidhi added 4 commits July 12, 2017 21:54

Integration test added

1d352f3

Blame Processor priority changed in test

5663e13

BlameUnitTestProject updated

35e5cf8

SDK Previous version updated to 15.3.0-preview-20170628-02

8599465

codito approved these changes Jul 14, 2017

View reviewed changes

Merge branch 'master' into newBlameBranch

57c24b5

codito merged commit c62f4d3 into microsoft:master Jul 14, 2017

		@@ -0,0 +1,147 @@
		// Copyright (c) Microsoft Corporation. All rights reserved.


		#region Test Assemblies

		[assembly: InternalsVisibleTo("Microsoft.TestPlatform.BlameDataCollector.UnitTests, PublicKey=002400000480000094000000060200000024000052534131000400000100010007d1fa57c4aed9f0a32e84aa0faefd0de9e8fd6aec8f87fb03766c834c99921eb23be79ad9d5dcc1dd9ad236132102900b723cf980957fc4e177108fc607774f29e8320e92ea05ece4e821c0a5efe8f1645c4c0c93c1ab99285d622caa652c1dfad63d745d6f2de5f17e5eaf0fc4963d261c8a12436518206dc093344d5ad293")]

Conversation

vagisha-nidhi commented Jul 6, 2017

Uh oh!

msftclas commented Jul 6, 2017

Uh oh!

codito left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants