Fix EndOfClass cleanup for DataRow

[nohwnd]

Fix cleanup for DataRow, to remove tests from the dictionary by using the same name (DisplayName) as what they were added. This affects DataRow tests and probably any test that uses DisplayName. Because then the entry in the hashmap and the key used to remove it (MethodName) are not the same.

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1448552

• Fix it
• Add tests

[nohwnd]

This seems to fix the issue, but I won't manage to add tests. Please add some @Haplois.

This was my repro, the Display name change in the attribute is not necessary.:

<!-- file TestProject3\TestProject3.csproj -->
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>netcoreapp3.1</TargetFramework>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0" />
    <PackageReference Include="MSTest.TestAdapter" Version="99.99.99-dev" />
    <PackageReference Include="MSTest.TestFramework" Version="99.99.99-dev" />
	  <!--<PackageReference Include="MSTest.TestAdapter" Version="2.2.8" />
	  <PackageReference Include="MSTest.TestFramework" Version="2.2.8" />-->
    <PackageReference Include="coverlet.collector" Version="1.2.0" />
  </ItemGroup>

</Project>

// file TestProject3\UnitTest1.cs
using Microsoft.VisualStudio.TestTools.UnitTesting;
using System;
using System.Diagnostics;

[assembly: ClassCleanupExecution(cleanupBehavior: ClassCleanupBehavior.EndOfClass)]

namespace TestProject3
{
    [TestClass]
    public class UnitTest1
    {
        static long Start;

        static UnitTest1()
        {
            Start = Stopwatch.GetTimestamp();
        }

        [ClassCleanup]
        public static void ClassCleanup() { 
            Console.WriteLine("cleanup");
        }

        [TestMethod(displayName: "TestMethod1")]
        [DataRow(1)]
        [DataRow(2)]
        public void TestMethod1(int _)
        {
            // the cleanup output should be on the test with the bigger number
            // because it needs to run on the last test
            Console.WriteLine(Stopwatch.GetTimestamp() - Start);
        }
    }

    [TestClass]
    public class UnitTest2
    {
        static long Start;
        static UnitTest2()
        {
            Start = Stopwatch.GetTimestamp();
        }


        [ClassCleanup]
        public static void ClassCleanup()
        {
            Console.WriteLine("cleanup");
        }

        [TestMethod(displayName: "TestMethod1")]
        public void TestMethod1()
        {
            Console.WriteLine(Stopwatch.GetTimestamp() - Start);
        }

        [TestMethod(displayName: "TestMethod2")]
        public void TestMethod2()
        {
            Console.WriteLine(Stopwatch.GetTimestamp() - Start);
        }
    }
}

[nohwnd]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[volleyms]

Hi, I wonder why you would use the display name which could be any string, even as duplicate within a test class, instead of the unique method name... just my 2 cents...

[Haplois]

Tests to be added later on...

[Haplois]

@volleyms, DisplayName should be unique; but a method could be executed multiple times if it's bound to a test data source and each execution considered as a test case.