Simplify handling traits/categories and include categories in test context properties

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestExecutionManager.cs

@@ -486,7 +486,7 @@ private async Task ExecuteTestsWithTestRunnerAsync(
 
             // Run single test passing test context properties to it.
             IDictionary<TestProperty, object?>? tcmProperties = TcmTestPropertiesProvider.GetTcmProperties(currentTest);
-            Dictionary<string, object?> testContextProperties = GetTestContextProperties(tcmProperties, sourceLevelParameters);
+            Dictionary<string, object?> testContextProperties = GetTestContextProperties(tcmProperties, sourceLevelParameters, unitTestElement);
 
             TestTools.UnitTesting.TestResult[] unitTestResult;
             if (usesAppDomains || Thread.CurrentThread.GetApartmentState() == ApartmentState.STA)
@@ -522,24 +522,31 @@ private async Task ExecuteTestsWithTestRunnerAsync(
     /// </summary>
     /// <param name="tcmProperties">Tcm properties.</param>
     /// <param name="sourceLevelParameters">Source level parameters.</param>
+    /// <param name="unitTestElement">The unit test element to get properties from.</param>
     /// <returns>Test context properties.</returns>
     private static Dictionary<string, object?> GetTestContextProperties(
         IDictionary<TestProperty, object?>? tcmProperties,
-        IDictionary<string, object> sourceLevelParameters)
+        IDictionary<string, object> sourceLevelParameters,
+        UnitTestElement unitTestElement)
     {
-        if (tcmProperties is null)
+        // If we only have sourceLevelParameters, we create a new dictionary with just those.
+        if (tcmProperties is null &&
+            unitTestElement.Traits is null or { Length: 0 } &&
+            unitTestElement.TestCategory is null or { Length: 0 })
         {
             return [with(sourceLevelParameters!)];
         }
 
-        // This dictionary will have *at least* 8 entries. Those are the sourceLevelParameters
-        // which were originally calculated from TestDeployment.GetDeploymentInformation.
-        var testContextProperties = new Dictionary<string, object?>(capacity: 8);
+        // To avoid any resizes and additional overhead, we calculate the capacity beforehand.
+        var testContextProperties = new Dictionary<string, object?>(capacity: sourceLevelParameters.Count + (tcmProperties?.Count ?? 0) + (unitTestElement.Traits?.Length ?? 0) + (unitTestElement.TestCategory?.Length ?? 0));
 
         // Add tcm properties.
-        foreach ((TestProperty key, object? value) in tcmProperties)
+        if (tcmProperties is not null)
         {
-            testContextProperties[key.Id] = value;
+            foreach ((TestProperty key, object? value) in tcmProperties)
+            {
+                testContextProperties[key.Id] = value;
+            }
         }
 
         // Add source level parameters.
@@ -548,9 +555,54 @@ private async Task ExecuteTestsWithTestRunnerAsync(
             testContextProperties[key] = value;
         }
 
+        if (unitTestElement.Traits is { Length: > 0 })
+        {
+            foreach (Trait trait in unitTestElement.Traits)
+            {
+                ValidateAndAssignTestProperty(testContextProperties, trait.Name, trait.Value);
+            }
+        }
+
+        if (unitTestElement.TestCategory is { Length: > 0 })
+        {
+            foreach (string category in unitTestElement.TestCategory)
+            {
+                ValidateAndAssignTestProperty(testContextProperties, category, string.Empty);
+            }
+        }
+
         return testContextProperties;
     }
 
+    /// <summary>
+    /// Validates If a Custom test property is valid and then adds it to the TestContext property list.
+    /// </summary>
+    /// <param name="testContextProperties"> The test context properties. </param>
+    /// <param name="propertyName"> The property name. </param>
+    /// <param name="propertyValue"> The property value. </param>
+    private static void ValidateAndAssignTestProperty(
+        Dictionary<string, object?> testContextProperties,
+        string propertyName,
+        string propertyValue)
+    {
+        if (StringEx.IsNullOrEmpty(propertyName))
+        {
+            return;
+        }
+
+        if (testContextProperties.ContainsKey(propertyName))
+        {
+            // Do not add to the test context because it would conflict with an already existing value.
+            // We were at one point reporting a warning here. However with extensibility centered around TestProperty where
+            // users can have multiple WorkItemAttributes(say) we cannot throw a warning here. Users would have multiple of these attributes
+            // so that it shows up in reporting rather than seeing them in TestContext properties.
+        }
+        else
+        {
+            testContextProperties.Add(propertyName, propertyValue);
+        }
+    }
+
     [SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Requirement is to handle errors in user specified run parameters")]
     private void CacheSessionParameters(IRunContext? runContext, ITestExecutionRecorder testExecutionRecorder)
     {

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.cs

@@ -67,16 +67,6 @@ internal TestMethodInfo(
     /// </summary>
     public bool IsTimeoutSet => TimeoutInfo.Timeout != TimeoutWhenNotSet;
 
-    /// <summary>
-    /// Gets or sets the reason why the test is not runnable.
-    /// </summary>
-    public string? NotRunnableReason { get; internal set; }
-
-    /// <summary>
-    /// Gets a value indicating whether test is runnable.
-    /// </summary>
-    public bool IsRunnable => StringEx.IsNullOrEmpty(NotRunnableReason);
-
     /// <summary>
     /// Gets the parameter types of the test method.
     /// </summary>

src/Adapter/MSTestAdapter.PlatformServices/Execution/TypeCache.cs

@@ -18,11 +18,6 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 /// </summary>
 internal sealed class TypeCache : MarshalByRefObject
 {
-    /// <summary>
-    /// Predefined test Attribute names.
-    /// </summary>
-    private static readonly string[] PredefinedNames = ["Priority", "TestCategory", "Owner"];
-
     /// <summary>
     /// Helper for reflection API's.
     /// </summary>
@@ -644,11 +639,7 @@ private TestMethodInfo ResolveTestMethodInfo(TestMethod testMethod, TestClassInf
 
         MethodInfo methodInfo = GetMethodInfoForTestMethod(testMethod, testClassInfo);
 
-        var testMethodInfo = new TestMethodInfo(methodInfo, testClassInfo, testContext);
-
-        SetCustomProperties(testMethodInfo, testContext);
-
-        return testMethodInfo;
+        return new TestMethodInfo(methodInfo, testClassInfo, testContext);
     }
 
     private DiscoveryTestMethodInfo ResolveTestMethodInfoForDiscovery(TestMethod testMethod, TestClassInfo testClassInfo)
@@ -719,91 +710,4 @@ private MethodInfo GetMethodInfoForTestMethod(TestMethod testMethod, TestClassIn
             ? null
             : testMethodInfo;
     }
-
-    /// <summary>
-    /// Set custom properties.
-    /// </summary>
-    /// <param name="testMethodInfo"> The test Method Info. </param>
-    /// <param name="testContext"> The test Context. </param>
-    private void SetCustomProperties(TestMethodInfo testMethodInfo, ITestContext testContext)
-    {
-        DebugEx.Assert(testMethodInfo != null, "testMethodInfo is Null");
-        DebugEx.Assert(testMethodInfo.MethodInfo != null, "testMethodInfo.TestMethod is Null");
-
-        // Avoid calling GetAttributes<T> to prevent iterator state machine allocations.
-        _ = ValidateAttributes(_reflectionHelper.GetCustomAttributesCached(testMethodInfo.MethodInfo), testMethodInfo, testContext) &&
-            ValidateAttributes(_reflectionHelper.GetCustomAttributesCached(testMethodInfo.Parent.ClassType), testMethodInfo, testContext);
-
-        static bool ValidateAttributes(Attribute[] attributes, TestMethodInfo testMethodInfo, ITestContext testContext)
-        {
-            foreach (Attribute attribute in attributes)
-            {
-                if (attribute is not TestPropertyAttribute testPropertyAttribute)
-                {
-                    continue;
-                }
-
-                if (!ValidateAndAssignTestProperty(testMethodInfo, testContext, testPropertyAttribute.Name, testPropertyAttribute.Value, isPredefinedAttribute: attribute is OwnerAttribute or PriorityAttribute))
-                {
-                    return false;
-                }
-            }
-
-            return true;
-        }
-    }
-
-    /// <summary>
-    /// Validates If a Custom test property is valid and then adds it to the TestContext property list.
-    /// </summary>
-    /// <param name="testMethodInfo"> The test method info. </param>
-    /// <param name="testContext"> The test context. </param>
-    /// <param name="propertyName"> The property name. </param>
-    /// <param name="propertyValue"> The property value. </param>
-    /// <param name="isPredefinedAttribute"> If the property originates from a predefined attribute. </param>
-    /// <returns> True if its a valid Test Property. </returns>
-    private static bool ValidateAndAssignTestProperty(
-        TestMethodInfo testMethodInfo,
-        ITestContext testContext,
-        string propertyName,
-        string propertyValue,
-        bool isPredefinedAttribute)
-    {
-        if (!isPredefinedAttribute && PredefinedNames.Any(predefinedProp => predefinedProp == propertyName))
-        {
-            testMethodInfo.NotRunnableReason = string.Format(
-                CultureInfo.CurrentCulture,
-                Resource.UTA_ErrorPredefinedTestProperty,
-                testMethodInfo.MethodInfo.DeclaringType!.FullName,
-                testMethodInfo.MethodInfo.Name,
-                propertyName);
-
-            return false;
-        }
-
-        if (StringEx.IsNullOrEmpty(propertyName))
-        {
-            testMethodInfo.NotRunnableReason = string.Format(
-                CultureInfo.CurrentCulture,
-                Resource.UTA_ErrorTestPropertyNullOrEmpty,
-                testMethodInfo.MethodInfo.DeclaringType!.FullName,
-                testMethodInfo.MethodInfo.Name);
-
-            return false;
-        }
-
-        if (testContext.TryGetPropertyValue(propertyName, out object? existingValue))
-        {
-            // Do not add to the test context because it would conflict with an already existing value.
-            // We were at one point reporting a warning here. However with extensibility centered around TestProperty where
-            // users can have multiple WorkItemAttributes(say) we cannot throw a warning here. Users would have multiple of these attributes
-            // so that it shows up in reporting rather than seeing them in TestContext properties.
-        }
-        else
-        {
-            testContext.AddProperty(propertyName, propertyValue);
-        }
-
-        return true;
-    }
 }

src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs

@@ -299,22 +299,6 @@ private static bool IsTestMethodRunnable(
             }
         }
 
-        // If test cannot be executed, then bail out.
-        if (!testMethodInfo.IsRunnable)
-        {
-            {
-                notRunnableResult =
-                [
-                    new TestResult
-                    {
-                        Outcome = UnitTestOutcome.NotRunnable,
-                        IgnoreReason = testMethodInfo.NotRunnableReason,
-                    },
-                ];
-                return false;
-            }
-        }
-
         bool shouldIgnoreClass = testMethodInfo.Parent.ClassType.IsIgnored(out string? ignoreMessageOnClass);
         bool shouldIgnoreMethod = testMethodInfo.MethodInfo.IsIgnored(out string? ignoreMessageOnMethod);
 

src/Adapter/MSTestAdapter.PlatformServices/Resources/Resource.resx

@@ -380,15 +380,9 @@ but received {4} argument(s), with types '{5}'.</value>
   <data name="UTA_ErrorNonPublicTestClass" xml:space="preserve">
     <value>UTA001: TestClass attribute defined on non-public class {0}</value>
   </data>
-  <data name="UTA_ErrorPredefinedTestProperty" xml:space="preserve">
-    <value>UTA023: {0}: Cannot define predefined property {2} on method {1}.</value>
-  </data>
   <data name="UTA_ErrorTestClassIsGenericNonAbstract" xml:space="preserve">
     <value>TestClass attribute defined on generic non-abstract class {0}</value>
   </data>
-  <data name="UTA_ErrorTestPropertyNullOrEmpty" xml:space="preserve">
-    <value>UTA021: {0}: Null or empty custom property defined on method {1}. The custom property must have a valid name.</value>
-  </data>
   <data name="UTA_ExecuteThrewException" xml:space="preserve">
     <value>An unhandled exception was thrown by the 'Execute' method. Please report this error to the author of the attribute '{0}'.
 {1}</value>

src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.cs.xlf

@@ -435,21 +435,11 @@ byl však přijat tento počet argumentů: {4} s typy {5}.</target>
         <target state="translated">UTA001: Atribut TestClass se definoval v neveřejné třídě {0}.</target>
         <note />
       </trans-unit>
-      <trans-unit id="UTA_ErrorPredefinedTestProperty">
-        <source>UTA023: {0}: Cannot define predefined property {2} on method {1}.</source>
-        <target state="translated">UTA023: {0}: V metodě {1} nejde definovat předdefinovanou vlastnost {2}.</target>
-        <note />
-      </trans-unit>
       <trans-unit id="UTA_ErrorTestClassIsGenericNonAbstract">
         <source>TestClass attribute defined on generic non-abstract class {0}</source>
         <target state="translated">Atribut TestClass definovaný u obecné neabstraktní třídy {0}</target>
         <note />
       </trans-unit>
-      <trans-unit id="UTA_ErrorTestPropertyNullOrEmpty">
-        <source>UTA021: {0}: Null or empty custom property defined on method {1}. The custom property must have a valid name.</source>
-        <target state="translated">UTA021: {0}: V metodě {1} je definovaná vlastní vlastnost, která je null nebo je prázdná. Vlastní vlastnost musí mít platný název.</target>
-        <note />
-      </trans-unit>
       <trans-unit id="UTA_ExecuteThrewException">
         <source>An unhandled exception was thrown by the 'Execute' method. Please report this error to the author of the attribute '{0}'.
 {1}</source>

src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.de.xlf

@@ -435,21 +435,11 @@ aber empfing {4} Argument(e) mit den Typen „{5}“.</target>
         <target state="translated">UTA001: Für die nicht öffentliche Klasse '{0}' definiertes Attribut 'TestClass'.</target>
         <note />
       </trans-unit>
-      <trans-unit id="UTA_ErrorPredefinedTestProperty">
-        <source>UTA023: {0}: Cannot define predefined property {2} on method {1}.</source>
-        <target state="translated">UTA023: {0}: Die vordefinierte Eigenschaft "{2}" kann nicht für die Methode "{1}" definiert werden.</target>
-        <note />
-      </trans-unit>
       <trans-unit id="UTA_ErrorTestClassIsGenericNonAbstract">
         <source>TestClass attribute defined on generic non-abstract class {0}</source>
         <target state="translated">TestClass-Attribut für generische nicht abstrakte Klasse {0} definiert</target>
         <note />
       </trans-unit>
-      <trans-unit id="UTA_ErrorTestPropertyNullOrEmpty">
-        <source>UTA021: {0}: Null or empty custom property defined on method {1}. The custom property must have a valid name.</source>
-        <target state="translated">UTA021: {0}: Für die Methode "{1}" wurde eine benutzerdefinierte Eigenschaft mit dem Wert NULL oder eine benutzerdefinierte leere Eigenschaft definiert. Die benutzerdefinierte Eigenschaft muss einen gültigen Namen aufweisen.</target>
-        <note />
-      </trans-unit>
       <trans-unit id="UTA_ExecuteThrewException">
         <source>An unhandled exception was thrown by the 'Execute' method. Please report this error to the author of the attribute '{0}'.
 {1}</source>

src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.es.xlf

@@ -435,21 +435,11 @@ pero recibió {4} argumentos, con los tipos '{5}'.</target>
         <target state="translated">UTA001: se ha definido el atributo TestClass en la clase no pública {0}</target>
         <note />
       </trans-unit>
-      <trans-unit id="UTA_ErrorPredefinedTestProperty">
-        <source>UTA023: {0}: Cannot define predefined property {2} on method {1}.</source>
-        <target state="translated">UTA023: {0}: no se puede definir la propiedad predefinida {2} en el método {1}.</target>
-        <note />
-      </trans-unit>
       <trans-unit id="UTA_ErrorTestClassIsGenericNonAbstract">
         <source>TestClass attribute defined on generic non-abstract class {0}</source>
         <target state="translated">Atributo TestClass definido en una clase genérica no abstracta {0}</target>
         <note />
       </trans-unit>
-      <trans-unit id="UTA_ErrorTestPropertyNullOrEmpty">
-        <source>UTA021: {0}: Null or empty custom property defined on method {1}. The custom property must have a valid name.</source>
-        <target state="translated">UTA021: {0}: se ha definido una propiedad personalizada nula o vacía en el método {1}. La propiedad personalizada debe tener un nombre válido.</target>
-        <note />
-      </trans-unit>
       <trans-unit id="UTA_ExecuteThrewException">
         <source>An unhandled exception was thrown by the 'Execute' method. Please report this error to the author of the attribute '{0}'.
 {1}</source>

src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.fr.xlf

@@ -435,21 +435,11 @@ mais a reçu {4} argument(s), avec les types « {5} ».</target>
         <target state="translated">UTA001 : attribut TestClass défini sur la classe non publique {0}</target>
         <note />
       </trans-unit>
-      <trans-unit id="UTA_ErrorPredefinedTestProperty">
-        <source>UTA023: {0}: Cannot define predefined property {2} on method {1}.</source>
-        <target state="translated">UTA023 : {0} : Impossible de définir la propriété prédéfinie {2} sur la méthode {1}.</target>
-        <note />
-      </trans-unit>
       <trans-unit id="UTA_ErrorTestClassIsGenericNonAbstract">
         <source>TestClass attribute defined on generic non-abstract class {0}</source>
         <target state="translated">Attribut TestClass défini sur une classe non abstraite générique {0}</target>
         <note />
       </trans-unit>
-      <trans-unit id="UTA_ErrorTestPropertyNullOrEmpty">
-        <source>UTA021: {0}: Null or empty custom property defined on method {1}. The custom property must have a valid name.</source>
-        <target state="translated">UTA021 : {0} : Une propriété null ou vide personnalisée est définie sur la méthode {1}. La propriété personnalisée doit posséder un nom valide.</target>
-        <note />
-      </trans-unit>
       <trans-unit id="UTA_ExecuteThrewException">
         <source>An unhandled exception was thrown by the 'Execute' method. Please report this error to the author of the attribute '{0}'.
 {1}</source>

src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.it.xlf

@@ -435,21 +435,11 @@ ma ha ricevuto {4} argomenti, con tipi '{5}'.</target>
         <target state="translated">UTA001: è stato definito l'attributo TestClass per la classe non pubblica {0}</target>
         <note />
       </trans-unit>
-      <trans-unit id="UTA_ErrorPredefinedTestProperty">
-        <source>UTA023: {0}: Cannot define predefined property {2} on method {1}.</source>
-        <target state="translated">UTA023: {0}: non è possibile definire la proprietà predefinita {2} per il metodo {1}.</target>
-        <note />
-      </trans-unit>
       <trans-unit id="UTA_ErrorTestClassIsGenericNonAbstract">
         <source>TestClass attribute defined on generic non-abstract class {0}</source>
         <target state="translated">Attributo TestClass definito nella classe generica non astratta {0}</target>
         <note />
       </trans-unit>
-      <trans-unit id="UTA_ErrorTestPropertyNullOrEmpty">
-        <source>UTA021: {0}: Null or empty custom property defined on method {1}. The custom property must have a valid name.</source>
-        <target state="translated">UTA021: {0}: per il metodo {1} è stata definita una proprietà personalizzata Null o vuota. Specificare un nome valido per la proprietà personalizzata.</target>
-        <note />
-      </trans-unit>
       <trans-unit id="UTA_ExecuteThrewException">
         <source>An unhandled exception was thrown by the 'Execute' method. Please report this error to the author of the attribute '{0}'.
 {1}</source>